Skip to content

fix(button)!: default to type="button" - #144

Open
pratikbuilds wants to merge 1 commit into
corbitsdev:mainfrom
pratikbuilds:claude/i142-button-type-4ccef8
Open

pratikbuilds wants to merge 1 commit into
corbitsdev:mainfrom
pratikbuilds:claude/i142-button-type-4ccef8

Conversation

@pratikbuilds

Copy link
Copy Markdown
Collaborator

Summary

Button renders a native <button>, and a <button> with no type is a submit button. Inside a host form, every bare Button submitted the form, including the stalled "Load earlier messages" control in ConversationEarlier and Retry in SendStatusLine (findings 8 and 9).

Button now renders type="button" unless a type is passed. An explicit type still wins, and asChild gets no injected type, since the slotted child may not be a button.

Breaking: a bare Button no longer submits a form. Pass type="submit" for a submit button.

Verification

  • bun run check
  • New button.test.tsx: a bare Button in a form does not submit, type="submit" does, and asChild gets no type.

Closes #142

Button rendered a native <button> with no type, so HTML treated every
bare Button as a submit button: inside a host form, activating it
submitted the form. That is the stalled "Load earlier messages" control
in ConversationEarlier and the Retry control in SendStatusLine.

Button now renders type="button" unless a type is passed. asChild gets
no injected type, since the slotted child may not be a button.

BREAKING CHANGE: a bare Button no longer submits a form. Pass
type="submit" for a submit button.

Addresses corbitsdev#142 findings 8 and 9.

@TheGreatAxios TheGreatAxios left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review · Request changes

Defaults native Button elements to type="button" when asChild is false so bare buttons inside host forms do not implicitly submit.

Findings

  • src/ui/button.tsx:80 — type={asChild ? undefined : "button"} is placed before {...props}, so a caller or wrapper that forwards { type: undefined } overwrites type="button" with undefined and renders a submit button (<button data-slot="button" ...>). Could we destructure type = asChild ? undefined : "button" in Button's parameters and pass type={type} so type={undefined} still defaults to "button"?
  • src/ui/button.test.tsx:30 (informational) — container.querySelector("button") as HTMLButtonElement (and lines 44, 62) uses as to silence | null rather than narrowing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Follow-ups from the conversation PR review

2 participants