seb-oss / green

Green is an open-source design system built by SEB.
https://storybook.seb.io/
Apache License 2.0
33 stars 47 forks source link

ButtonGroups: Unwanted margin inside of forms #825

Closed syradar closed 1 year ago

syradar commented 1 year ago

Bug already reported ?

For which framework/library you are reporting the bug

React, Chlorophyll

Component name

ButtonGroup, Group, FormGroup

Description

image

image

Steps To Reproduce

  1. Use TypeScript and React.
  2. Add a ButtonGroup as an input inside of a FormGroup.

Current Behaviour

Expected Behaviour

id should be allowed on the ButtonGroup Component.

The margin should be between the Label and the Input (in this case a ButtonGroup)

Less magic for components, or clearer rules. By using the group I also got all of these CSS rules that I did not expect

image

Is there any plans of a more utility based CSS framework or would you be open for a SEB Green Config for something like Tailwind?

astrit commented 1 year ago

Is this the setup you are looking for ?

CleanShot 2023-04-03 at 15 31 21@2x

Example:

CleanShot 2023-04-03 at 15 33 42@2x
syradar commented 1 year ago

Yeah I guess. Right now I solved by adding flex-column to the label and buttongroup's parent and gap of 8px (Spacing3 right)

astrit commented 1 year ago

Yeah I guess. Right now I solved by adding flex-column to the label and buttongroup's parent and gap of 8px (Spacing3 right)

Yes with flex and gap things would be better but for now I pushed a little fix.

Also was wondering why this is not a radio button since it would be more accessible and the ButtonGroup is not really intended to have a label, if you don't mind to elaborate on that ✌️

astrit commented 1 year ago

@syradar

Me and @splashdust went through the design library and to me it seems like the ButtonGroup is not the best fit in this scenario since it can't really have a label as per se when you click on it it can't focus to all buttons.

Would strongly suggest to use Radio Button instead and that has a label, so you don't need to specify manually or even group the entire component. https://sebgroup.github.io/green/latest/react/?path=/docs/components-form-radiobutton--radio-button

splashdust commented 1 year ago

To add to @astrit's reponse, the ButtonGroup is actually the Segmented Control component from Design LIbrary, which is not meant to be used as a form input.

This is currently unclear in Green though, especially with the unaptly named ButtonGroup component, and I think we need to improve documentation and provide better examples to explain what the components were designed for.