Support toggled status in Button.Group

[victorfunes]

• [Yes] I have tested the latest version

Summary

Based on your Figma designs, it can be useful to support in Button.Group component the selection of one of the buttons. Imagine as an example a filter of time for your screen such as this one:

Context

Here you can see the implementation I have now to highligh the selected button, but it obliges to manage manually the className to toggle the selected button:

        <Button.Group>
          <Button size="sm" color="light" onPointerDown={filterLastYear}>
            <span className={internalFilterDate === FilterDate.LAST_YEAR ? selectedButtonClassname : undefined}>
              Last year
            </span>
          </Button>
          <Button size="sm" color="light" onPointerDown={filterLastMonth}>
            <span className={internalFilterDate === FilterDate.LAST_MONTH ? selectedButtonClassname : undefined}>
              Last 30 days
            </span>
          </Button>
          <Button size="sm" color="light" onPointerDown={filterLastWeek}>
            <span className={internalFilterDate === FilterDate.LAST_WEEK ? selectedButtonClassname : undefined}>
              Last 7 days
            </span>
          </Button>
          <Button size="sm" color="light" onPointerDown={filterAlways}>
            <span className={internalFilterDate === FilterDate.ALWAYS ? selectedButtonClassname : undefined}>
              Always
            </span>
          </Button>
        </Button.Group>

[tulup-conner]

Could you provide an example of an API modification to <Button> that would solve this for you?

To be honest, the code you provided doesn't seem like it's much of an effort, to be worth altering the API, and adding complexity to the component.

[rluders]

I think that it can be done without any new API for the Button component, TBH... and probably with a simpler code that doesn't requires that <span> tag... You can control states with useState and use the button onClick API to apply the filters, then you can change the button style by simply passing it a 'className' with the desired style.

[rluders]

const { filter, setFilter } = useState<Filter>(null);
// ...
<Button.Group>
  <Button 
    size="sm" 
    color="light" 
    className={filter === Filter.LastYear && 'bg-red-300'} 
    onClick={() => setFilter(Filter.LastYear)}
  >
    Last year
  </Button>
</Button.Group>

From the top of my head, written directly here, not tested at all... BTW, you could even write something to accept multiple buttons to be active, it is just a question of logic. If this is the case, I'm not sure if we want/should add this complexity to the component, since it seems much simpler to implement without any new API. 🤔

[victorfunes]

I am also using the pagination component and I thought it was based in the same code internally, so I watched into the code to see how does this component manage it, but I have seen it is a completely different component.

In pagination, when you click a button, it is marked as selected automatically and this behaviour would be the most useful in my opinion.

For me it would be interesting to have in Button.Group a boolean to activate this behaviour and manage it internally by the component itself like pagination does. Maybe this means you also have to expand the scope of the component theme to enable a new style for the toggled button. 🤷‍♂️

I have solved it in my case as you have seen, so I don’t need any change to implement this in my project, but as you can see, I think it could be useful in many cases such as filters, pagination and for sure many others…

You decide: for sure have much more experience and criteria than I have.

[rluders]

The reason Paginator and ButtonGroup are distinct is due to their distinct purposes. Paginator functions as a navigation component, while ButtonGroup serves as an "action element" with versatile applications.

Using the same component for both would likely complicate the logic, making maintenance more challenging. I am uncertain about utilizing ButtonGroup as a toggle state since it seems feasible to implement this functionality without introducing a new API to the component.

Nevertheless, it is plausible that my perspective is incorrect. There is a possibility that the community may desire this feature, or alternatively, it could be considered as a separate component. Ultimately, I do not possess the final decision-making authority. However, I believe it would be valuable to understand the advantages and disadvantages of adding this new API to ButtonGroup. Additionally, if the intention is to create a new component, it would be beneficial to identify the suitable scenarios in which it could be utilized.

[rluders]

I was thinking about this request, @victorfunes, and to enable the transformation of the ButtonGroup into a toggle state manager, the following steps can be taken:

1. ButtonGroup should offer a property that allows toggling.
2. ButtonGroup should inject a click handler into each of its child buttons, enabling them to change their state.
3. A new style for the active/enabled button should be introduced.
4. Additionally, an onChange property can be added to the ButtonGroup to trigger a function whenever there is a state change.

Do you think these suggestions effectively capture the desired modifications?

It would look like this - but I'm not 100% sure of this implementation:

<Button.Group toggle onChange={(value: ???) => doSomethigWhenChange(value}>
  <Button 
    size="sm" 
    color="light"
    value="????"   
  >
    Last year
  </Button>
</Button.Group>

[victorfunes]

Thank you @rluders ! It seems perfect to me. Only one minor topic to my mind, and maybe it was also thought in your proposal: the value defined in each button could support other types than string?

[rluders]

So, this is one of the problems with this implementation, IMHO. It would be complicated to define a type for it, 'cause, well, you can basically wish to use anything, right? You can create your own type, use string, use number... using any is out of the question, maybe generics? Is it possible?

I am still not convinced that this is a good idea. But, I want to brainstorm it before taking any decision, and I would also like to hear other opinions about this...

[victorfunes]

Maybe then it is simplier to keep the logic in each button using onClick or onPointerDown, isn’t it?

[rluders]

In this case, I still think that the best approach is to implement it outside the component, as I explained here

[victorfunes]

I agree: one or the other, because keeping toogle property but managing the state outside of the button group can lead to inconsistencies. Then if you ask my opinion, I would prefer to keep it simple with an string and go for the toggle solution like described here: https://github.com/themesberg/flowbite-react/issues/854#issuecomment-1635385393

You decide what is best

[rluders]

I'm closing this one since there is a possible implementation without increasing the component logic.