jpmorganchase / salt-ds

React UI components built with a focus on accessibility, customization and ease-of-use
https://www.saltdesignsystem.com
Apache License 2.0
125 stars 89 forks source link

Buttons used in Salt have a lot of overrides #2793

Open joshwooding opened 10 months ago

joshwooding commented 10 months ago

Buttons currently have 4 different sizes:

Size Description
Base Standalone Buttons
Base - spacing 100 Pill close button
Base - spacing 100 Input adornment
Base - spacing 50 Multiline Input adornment
Base - spacing 100 Toggle Button (in group)

This makes code and composition of components quite brittle e.g. #2759 #2740

joshwooding commented 10 months ago

cc @pseys

pseys commented 10 months ago

Buttons currently have 4 different sizes:

Size Description Base Standalone Buttons Selectable Pill close button Base - spacing 100 Input adornment Base - spacing 50 Multiline Input adornment This makes code and composition of components quite brittle e.g. #2759 #2740

Pill (next) close button looks like it's using --salt-size-base - --salt-spacing-100 similar to input adornment, not --selectable.

pseys commented 10 months ago

Toggle Button group is another example of --salt-size-base - --salt-spacing-100

joshwooding commented 10 months ago

Thanks, updated the description. Do you think we need a size prop for Button? I think it will be cleaner

pseys commented 10 months ago

Thanks, updated the description. Do you think we need a size prop for Button? I think it will be cleaner

Yes I think that might be beneficial. I think there are two things here, removing instancing of size and space being used to drive the height of a component/element and enabling buttons to be reduced in height based on context.

Switch also uses --salt-size-base - --salt-spacing-100 to control height.

pseys commented 9 months ago

Proposed changes have been identified, setting up time for review.