Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Toggle/ToggleGroup components should be fully controlled #581

Open
kotAPI opened this issue Nov 24, 2024 · 3 comments
Open

BUG: Toggle/ToggleGroup components should be fully controlled #581

kotAPI opened this issue Nov 24, 2024 · 3 comments

Comments

@kotAPI
Copy link
Collaborator

kotAPI commented Nov 24, 2024

Description

The Toggle and ToggleGroup components should be fully controlled, currently it is managed by having internal state

Ref-
#576 (comment)

@krau5
Copy link
Contributor

krau5 commented Dec 1, 2024

@kotAPI I would like to take care of this one. Could you assign it to me?

UPD. I did a bit of research, lots of questions are coming up. I think we should define component behavior and props at first and then proceed with this issue. I started refactoring the code, but the APIs between components are not even similar. We are passing property X and Y to the component, when it expects property Z. I will raise a discussion in the dev channel a bit later and I will propose a solution

@kotAPI
Copy link
Collaborator Author

kotAPI commented Dec 1, 2024

@kotAPI I would like to take care of this one. Could you assign it to me?

UPD. I did a bit of research, lots of questions are coming up. I think we should define component behavior and props at first and then proceed with this issue. I started refactoring the code, but the APIs between components are not even similar. We are passing property X and Y to the component, when it expects property Z. I will raise a discussion in the dev channel a bit later and I will propose a solution

Sure! Let's discuss and decide what to pick next!

@kotAPI kotAPI changed the title BUG: Toggle/ToggleGroup compoennts should be fully controlled BUG: Toggle/ToggleGroup components should be fully controlled Dec 2, 2024
@kotAPI
Copy link
Collaborator Author

kotAPI commented Dec 2, 2024

Hey @krau5, noting your inputs here for reference so we don't miss it in the threads, we'll have a detailed discussion on this and decide how to proceed, will update the issue with our decisions and directions so we leave a trail

I'll start with ToggleGroup as it is the one with more problems.
The component should support single/multiple selection (like a SelectBox in some ui libs).
The component is fully controlled, meaning that it shall not have any internal state and/or defaultValue-like props.
The component could accept an array of items to render, current value (or array of values for multiple selection) and onChange.
Currently we also have some sort of duplication as there is a Toggle and ToggleItem, even though it should've been one component.

Long story short, my proposal is:

  • ToggleGroup API consists of: value (string | string[]), onChange (accepts an argument of the same type as value), type (single | multiple)
  • ToggleItem is removed and replaced with Toggle
  • Toggle is being utilized by ToggleGroup
  • ToggleGroup is a controlled component

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants