add type="button" to all components using button element

[wysher]

Using SelectTrigger shadcn/svelte with forms causes form submit. This happens, because button by default has type="submit".

I think it's good idea to set type="button" instead, to prevent unwanted submits.
Here's implementation which allows to change it by developer, but maybe it should not be possible to changed at all?

[changeset-bot]

🦋 Changeset detected

Latest commit: 8efa3b1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
bits-ui	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
primitives	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 30, 2023 7:29am

[huntabyte]

Hey @wysher , thanks for this! I don't think it should be able to be changed at all tbh. If someone really wants to change it, they can use asChild and override the attribute.

[huntabyte]

Thanks for noticing this! We could also probably Omit the type prop from the TriggerProps as well!

[huntabyte]

Suggested change

export let type: $$Props["type"] = "button";

[huntabyte]

Suggested change

	{type}
	type="button"

[wysher]

@huntabyte all done. And also more... I searched for button elements within bits components and added more type=button. I'm pretty sure all of them needs it.

Especially checkbox or radio - you don't want form inputs to trigger submit at any time. Same with tabs, popovers, etc.

[huntabyte]

You're incredible, thank you!

[huntabyte]

For the rest of the buttons, I think we should do something like this:

		type="button"
		{...$$restProps}
		{...attrs}

That way, if for some reason or another someone wants to submit the form when they close the dialog, or toggle a switch, or whatever, they are able to override the type="button" we set by passing type="submit" since the ...$$restProps follows our hardcoded type.

[huntabyte]

Suggested change

type="button"

I think we leave this up to the dev to add here if they wish. Imagine you wanted to add a tooltip to a button that submits a form, with the tooltip saying "Submit form" or something. We don't want to prevent the user from doing something like this.

[wysher]

makes sense here. Done.

[huntabyte]

Suggested change

	type TriggerProps = AsChild & Omit<HTMLButtonAttributes, "type">;
	type TriggerProps = AsChild & HTMLButtonAttributes;

[huntabyte]

Thanks again @wysher!

[wysher]

One and only reason is that changed components here uses button element under the hood - without developer knowledge. Button component which will be deliberately used by developer was not changed by this PR. pon., 6 lis 2023 o 19:17 Wilson Hobbs ***@***.***> napisał(a):

…

What's the reasoning here? Default HTML behavior is no type attribute, and if it is within a form, the expectation is that the developer will add type="button" to prevent submit. Is there a good reason to go against the web standard? — Reply to this email directly, view it on GitHub <#150 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEA7U5HDJKWIL7WI5AP2VRTYDEZUNAVCNFSM6AAAAAA6UJX7EGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJWGA4DOOBSGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[wysher]

One and only reason is that changed components here uses button element under the hood - without developer knowledge. Button component which will be deliberately used by developer was not changed by this PR.

@huntabyte this one left me with one question. Maybe AlertDialogAction should not set type="button" as default, cause it looks like submit and AlertDialogContent may be wrapped with form?

[wbhob]

I realized and deleted my comments after. Great PR! :)