shoelace-style / shoelace

A collection of professionally designed, every day UI components built on Web standards. SHOELACE IS BECOMING WEB AWESOME πŸ‘‡πŸ‘‡πŸ‘‡
https://webawesome.com
MIT License
12.75k stars 816 forks source link

Button uses protected type attribute for variants #137

Closed whoisryosuke closed 4 years ago

whoisryosuke commented 4 years ago

Describe the bug Button component uses type attribute to define style variants (primary, secondary, etc), when type is a protected attribute for the HTML button element.

Using the submit boolean attribute allows you to change the underlying button's type from "button" to "submit" -- but it doesn't accommodate the use case of someone creating a "reset" button (to reset a form).

Expected behavior Button component should pass through the type attribute (and type check it against the platform options). This is easier for people to understand because it replicates the web platform API (rather than overriding the familiar, existing API).

For alternative component styles like "primary" or "secondary", another attribute/prop name should be assigned, like variant or button-style.

You can see an example of this with StencilJS component with the Ionic UI library's button component. The type attribute reflects the native button attribute, and they use a color attribute to set variant styles like "primary".

Additional context Add any other context about the problem here.

claviska commented 4 years ago

Hi, thanks for posting this!

Button component uses type attribute to define style variants (primary, secondary, etc), when type is a protected attribute for the HTML button element.

The type attribute is protected for <button>, but <sl-button> is a custom element with its own, independent API. I do understand where you're coming from and I thought about this decision a lot early on. I even added a note about it in the docs.

You might expect similarly named elements to share the same API as native HTML elements. This is not always the case. Shoelace components are not designed to be one-to-one replacements for their HTML counterparts.

That said, I'm intentionally not mimicking the <button> API because I feel that it was poorly designed. The default type is submit instead of button, which is almost never what users want. Neither submit nor reset feel like a type to me β€” they're really actions. And <button type="button"> is redundant.

If we're going to deviate from the norm to fix this, we might as well rethink the API as a whole to make life easier for consumers. If we stop expecting <sl-button> to be the same as <button> this makes a lot more sense.

You can see an example of this with StencilJS component with the Ionic UI library's button component. The type attribute reflects the native button attribute, and they use a color attribute to set variant styles like "primary".

IMO "color" doesn't reflect the semantic meaning of the property. "Variant" might be a better choice at the expense of a few extra characters, but I'm still not convinced using type was a bad decision.

At the end of the day, I'm not interested in rebuilding existing HTML elements. My goal is to build new ones that provide a better developer experience, so I'm OK if Shoelace elements don't align with the APIs of their native counterparts. πŸ˜„

As far as reset goes, I haven't implemented it yet but it would happen as a method in <sl-form>. A form can be reset at any time, but I think it's more common to happen after submission or when a route is entered/exited. It's less likely a button would trigger it without first submitting or sending an XHR, so I don't see much value in tying it directly into <sl-button>.

claviska commented 4 years ago

I'm going to close this for now, since it's not technically a bug.

Future readers: if you feel strongly about this, please vote with πŸ‘ or πŸ‘Ž on the original post so I can gauge feedback from the community. Additional feedback is also welcome, so long as it remains constructive.

claviska commented 2 years ago

I've renamed type to variant in alert, badge, button, and tag to disambiguate between variants and other "type" attributes, such as <sl-input type> and <sl-format-number type>.

https://github.com/shoelace-style/shoelace/commit/fb20155485a9b7098ef3f470807efeb47d677367