search

suankularb-wittayalai-school / sk-components

A design system for all Suankularb applications
https://sk-components-demo.mysk.school
0 stars 1 forks source link

New Button syntax #70

Closed SiravitPhokeed closed 1 year ago

SiravitPhokeed commented 2 years ago

Is your feature request related to a problem? Please describe. Whoever wrote the Button component is clearly drunk. name is for the description that is only visible to screenreaders (and not even the tooltip!), and label is for the text inside the Button. To put stuff inside the Button you have to write attributes, not children?

Describe the solution you’d like Consider this:

<Button title="Bring up Photo Gallery" onClick={() => toggleGallery()}>
  <MaterialIcon icon="photo_library" allowCustomSize />
  <span>See all</span>
</Button>

Doesn’t that make more sense? Button should adapt automatically based on what elements are present inside children as well, eliminating the need for iconOnly.

Additional context There are some changes that must be made in SKCom for this to work. Instead of changing modifiers based on whether Icon or text are present, we detect those elements directly in the CSS. This will make it easier to use Button for SKCom users as well.

SiravitPhokeed commented 1 year ago

As part of the 3.0 revamp, most of this feedback has been implemented with minor changes, the most major of which is how Icon is implemented. As is standard in other component libraries, an icon can be added to Button via the icon prop. I guess congratulations are in order for…past me…?