fix: ensure button renders custom tag

[michelheusschen]

Closes #1644

📑 Description

Fixes an issue where the Button component doesn't use the custom tag. This also affects the CheckboxButton and RadioButton since they use <Button tag="label">. These components produce invalid html as a result, according to the Nu Html Checker: Error: The element input must not appear as a descendant of the button element.

Other changes/fixes:

• Remove unnecessary click handler introduced in 5b0af61
• Remove wrong role="button" from anchor
• Add tests

Status

• Not Completed
• Completed

✅ Checks

• My pull request adheres to the code style of this project
• My code requires changes to the documentation
• I have updated the documentation as required
• I have checked the page with https://validator.unl.edu/
• All the tests have passed
• My pull request is based on the latest commit (not the npm version).

ℹ Additional Information

Summary by CodeRabbit

• Bug Fixes

  • Improved button component rendering logic to ensure anchor elements are only rendered when an href is present, and simplified overall branching for more predictable behavior.
  • Simplified radio button component by relying on native label/input interactions, removing unnecessary click handling.

• Tests

  • Added comprehensive test coverage for button and radio button components, including basic, submit, link, and label button scenarios, as well as radio button selection behavior.

[vercel]

@michelheusschen is attempting to deploy a commit to the Themesberg Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

Walkthrough

The updates refactor the button and radio button components to simplify their rendering logic, removing unnecessary manual event handling and redundant branches. Additionally, comprehensive test files are introduced for both button and radio button components, covering various rendering scenarios and user interactions using Svelte Testing Library and Vitest.

Changes

File(s)	Change Summary
src/lib/buttons/Button.svelte	Simplified conditional rendering: anchors render only if href is defined; removed redundant anchor branch.
src/lib/forms/radio/RadioButton.svelte	Removed manual input click handling and input element reference; relies on native label/input behavior.
src/tests/button/basic-button.test.svelte src/tests/button/submit-button.test.svelte src/tests/button/link-button.test.svelte src/tests/button/label-button.test.svelte	Added Svelte test files for basic, submit, link, and label button variants, each rendering the Button component.
src/tests/button/button.test.ts	Added unit tests for button variants, verifying rendering, attributes, and tag types using Vitest and Testing Library.
src/tests/forms/radio/check-radio-button.test.svelte	Added test file rendering two radio buttons with the same name but different values.
src/tests/forms/radio/radio-button.test.ts	Added test for radio button selection behavior, simulating user clicks and checking state.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ButtonComponent
    participant DOM

    User->>ButtonComponent: Render with props (href, tag, etc.)
    alt href is defined
        ButtonComponent->>DOM: Render <a> element with props
    else tag is "button" and href is undefined
        ButtonComponent->>DOM: Render <button> element
    else
        ButtonComponent->>DOM: Render element specified by tag
    end

Loading

sequenceDiagram
    participant User
    participant RadioButtonComponent
    participant DOM

    User->>RadioButtonComponent: Render with props (name, value, etc.)
    RadioButtonComponent->>DOM: Render <label> containing <input type="radio">
    User->>DOM: Click label or input
    DOM->>RadioButtonComponent: Native event triggers input selection
    RadioButtonComponent->>DOM: Updates checked state

Loading

Poem

A button’s fate now clearly shown,
As anchors link when href is known.
No more manual clicks for radios here,
Native labels make intent clear.
With tests that check each rendered state,
This code hops forward, feeling great!
🐇✨

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6d0f102 and 8270974.

📒 Files selected for processing (9)

• src/lib/buttons/Button.svelte (1 hunks)
• src/lib/forms/radio/RadioButton.svelte (1 hunks)
• src/tests/button/basic-button.test.svelte (1 hunks)
• src/tests/button/button.test.ts (1 hunks)
• src/tests/button/label-button.test.svelte (1 hunks)
• src/tests/button/link-button.test.svelte (1 hunks)
• src/tests/button/submit-button.test.svelte (1 hunks)
• src/tests/forms/radio/check-radio-button.test.svelte (1 hunks)
• src/tests/forms/radio/radio-button.test.ts (1 hunks)

🔇 Additional comments (12)

src/lib/buttons/Button.svelte (1)

18-22: Excellent fix for custom tag rendering!

This change correctly addresses the core issue described in the PR objectives. The updated conditional logic now:

1. Properly renders anchor elements when href is provided (without unnecessary role="button")
2. Ensures custom tags like tag="label" are actually rendered as <label> elements instead of <button> elements
3. Eliminates the invalid HTML structure that was causing Nu Html Checker errors

The logic flow is now clean and intuitive: href → anchor, button tag → button element, other tags → custom element.

src/tests/button/basic-button.test.svelte (1)

1-6: Good basic test component setup.

This test component correctly renders a basic Button with default props, which will be useful for testing the default tag="button" behavior.

src/tests/button/submit-button.test.svelte (1)

1-6: Good submit button test setup.

This test component correctly tests the submit button variant, ensuring that the type="submit" attribute is properly passed through to the rendered button element.

src/tests/button/label-button.test.svelte (1)

1-6: Critical test for the main fix!

This test component directly validates the core issue being fixed - ensuring that tag="label" actually renders a <label> element instead of a <button> element. This resolves the invalid HTML structure that was causing Nu Html Checker errors.

The data-testid attribute makes it easy to query and verify the correct element type in tests.

src/tests/button/link-button.test.svelte (1)

1-6: Good test for anchor rendering.

This test component validates that when href is provided, the Button component correctly renders as an anchor element. This tests the updated conditional logic and ensures proper link functionality.

src/tests/forms/radio/check-radio-button.test.svelte (1)

1-6: LGTM! Well-structured test component.

The test component correctly sets up two radio buttons with the same name attribute but different value attributes, which is the proper way to create a radio button group for testing.

src/tests/forms/radio/radio-button.test.ts (1)

1-22: Excellent test coverage for radio button functionality.

The test correctly validates the native label/input behavior by clicking on the parent element (label) and verifying the radio button selection state. This aligns well with the component simplification that now relies on native browser behavior instead of manual event handling.

src/lib/forms/radio/RadioButton.svelte (1)

11-12: Further inspecting the Button implementation to confirm it renders arbitrary tags (including label):

#!/bin/bash
# Find the Button.svelte file and show how `tag` is used
file=$(find . -type f -name Button.svelte | head -n1)
echo "Inspecting: $file"
grep -n "export let tag" "$file" || true
grep -n "svelte:element" "$file" || true
sed -n '1,200p' "$file"

src/tests/button/button.test.ts (4)

13-20: Well-structured basic button test.

The test correctly validates that a basic button renders with the proper role, text content, and type="button" attribute.

---

22-29: Proper submit button validation.

The test correctly ensures submit buttons render with type="submit" attribute, which is essential for form submission behavior.

---

31-37: Good link button test coverage.

The test properly validates that link buttons render as anchor elements without a type attribute, which is correct HTML behavior.

---

39-46: Excellent validation of label button rendering.

This test is particularly important as it validates the core fix mentioned in the PR objectives. By ensuring the label element doesn't have type or role attributes, it prevents the invalid HTML structure that was causing the "input must not appear as a descendant of button element" error.

✨ Finishing Touches

• 📝 Generate Docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Explain this complex logic.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai explain this code block.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and explain its main purpose.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[shinokada]

Why do you want to use Button for label? Can't you use Label component?

[michelheusschen]

Are you suggesting that the CheckboxButton and RadioButton components should use the Label component instead? While that could work, it would result in duplicating a lot of the button component's code. I think it's better to fix root cause: the button component doesn't correctly use the tag prop. Which is exactly what this PR tries to accomplish.