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

[material-next][Switch] Add Switch component #39886

Open
3 of 7 tasks
lhilgert9 opened this issue Nov 15, 2023 · 12 comments
Open
3 of 7 tasks

[material-next][Switch] Add Switch component #39886

lhilgert9 opened this issue Nov 15, 2023 · 12 comments
Assignees
Labels
component: switch This is the name of the generic UI component, not the React module! design: material you on hold There is a blocker, we need to wait

Comments

@lhilgert9
Copy link
Contributor

lhilgert9 commented Nov 15, 2023

This issue is to track the work for adding the Switch component to @mui/material-next, moving forward the Material You effort (#29345)

Material You Swicth specification: https://m3.material.io/components/switch/overview

Migration Guide steps

Note: This comment will be updated to show the progress and list possible issues.

@DiegoAndai

@github-actions github-actions bot added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 15, 2023
@zannager zannager added the component: switch This is the name of the generic UI component, not the React module! label Nov 15, 2023
@mnajdova mnajdova added v6.x design: material you and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 16, 2023
@DiegoAndai DiegoAndai added v7.x and removed v6.x labels Dec 26, 2023
@DiegoAndai DiegoAndai added this to the Material UI: v7 draft milestone Dec 26, 2023
@DiegoAndai DiegoAndai assigned DiegoAndai and unassigned mj12albert Dec 29, 2023
@lhilgert9
Copy link
Contributor Author

lhilgert9 commented Jan 3, 2024

@DiegoAndai I worked on the next steps and came to the point where I asked myself whether we wanted to keep the current structure of the switch component or align it with the structure of @mui/base. So there are two options:

  1. @mui/material:
<Root>
  <SwitchBase/> //has to be copied in @mui/mui-material-next
  <Track/>
</Root>
  1. @mui/base:
<Root>
  <Track/>
  <Thumb/>
  <Input/>
</Root>

I would prefer the second option, as it is easier to implement the useSwitch hook, but there would be a lot of breaking changes. However, I find this justifiable, as the structure of the components in mui is finally the same and developers do not have to differentiate.

@DiegoAndai
Copy link
Member

it is easier to implement the useSwitch hook, but there would be a lot of breaking changes

I would also prefer option 2. What are the breaking changes that would be required?

@lhilgert9
Copy link
Contributor Author

lhilgert9 commented Jan 4, 2024

What are the breaking changes that would be required?

  • The SwitchBase-component will be removed
  • The switchBase-class will be removed
  • The SwitchBaseProps will be replaced with the UseSwitchParameters in the SwitchProps
  • The value-prop can be removed
  • icon and checkedIcon could be implemented as slot components, so we can use a hook as default checkedIcon like in the MD3 docs

I think that must have been all of them. If nothing speaks against it, I would try to implement the whole thing and we can then make further adjustments in the PR.

@DiegoAndai
Copy link
Member

  • The SwitchBase-component will be removed
  • The switchBase-class will be removed
  • The SwitchBaseProps will be replaced with the UseSwitchParameters in the SwitchProps

I'm not so worried about these ones, the whole idea of the Base UI refactor is to remove code duplication, so removing SwitchBase makes sense.

  • icon and checkedIcon could be implemented as slot components, so we can use a hook as default checkedIcon like in the MD3 docs

This one makes sense as well, we'll soon deprecate these in favor of the slots pattern

  • The value-prop can be removed

Is there a use for this prop? I wouldn't remove it unless there's an advantage of doing so or if it's difficult to keep for some reason.

I would try to implement the whole thing and we can then make further adjustments in the PR.

Let's do that 🚀 let me know if there's anything I can help you with. Let's try to keep breaking changes to a minimum besides the ones listed above. If you find any improvements/fixes on the Base UI side, let us know so we can discuss 🙌🏼.

@lhilgert9
Copy link
Contributor Author

@DiegoAndai I didn't quite understand myself what the value-prop was for, but it was only passed on to the SwitchBase and as this is no longer the case, I would have seen no reason to keep it. Unless we can somehow incorporate it into the useSwitch-hook, in case it does have a function.

@DiegoAndai
Copy link
Member

@lhilgert9, the SwitchBase component forwards the value prop to the input element; let's keep that for now.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 6, 2024

Should we add the "on hold" label to all the MD3 issues?

I can't think of a supporting reason to work on those until the MUI System and Base UI are finalized. What I see is that it would slow us to get Material UI v6 out of the door, while also taking a significant risk for needed to redo the effort, there is a good chunk of uncertainties:

  • MUI System might require us to rewrite @mui/material-next
  • The merge with Joy UI might require to rethink of the whole implementation of Material UI v7.
  • Base UI API might improve, to a state where we don't need to rely on the hook API, and instead where we can use the component API that is recommended in our docs.
  • I don't think we can release a major style change without having all the components with an unstyled counterpart that developers can migrate to, so it seems to me that Base UI completeness and great DX should come first.

@DiegoAndai
Copy link
Member

Should we add the "on hold" label to all the MD3 issues?

It's "on hold" regarding the Core team. Do you think we should also hold on community contributors for it?

I see benefits in having the community continue the effort:

  • It gives early access to people who need it
  • It gives us a blueprint for v7: The style changes, composed classes, and Base UI refactors should be the same, or at least very similar, so we can effectively use it as a guide. This is useful so we know what things to deprecate earlier.

What do you think?

cc @mnajdova to get your opinion on it as well

@mnajdova
Copy link
Member

mnajdova commented Jan 9, 2024

I agree internally, that we should not work on the migration for now. For external contributions, I would leave that up to @DiegoAndai. My feeling is that we should not receive contributions. Few reasons why:

  • it could be distracting working on one thing and reviewing something else (or not, depending on you Diego :))
  • we could have a "wrong starting point" for v7, considering we would be iterating on this code - not on the original one
  • there would likely be a lot of lost effort, depending on the decisions we would make for how to build the components

@DiegoAndai
Copy link
Member

@mnajdova, fair points. I'm marking the MD3 issues as "on hold" for now.

@lhilgert9 I hope this doesn't discourage you from contributing to other Material UI issues. You've been making significant contributions. We will need help with v6 soon, and in the second half of the year, we will resume the MD3 work 😊.

@DiegoAndai DiegoAndai added the on hold There is a blocker, we need to wait label Jan 10, 2024
@lhilgert9
Copy link
Contributor Author

@DiegoAndai Sounds understandable to me. I'll see if I can contribute to V6. I will do my best. I'm looking forward to learning and developing new things ☺️🚀. Do we also put the ButtonGroup on hold or do we want to finish it since it is as good as finished? On the other hand, there is still a lot to do on the normal button component, so on hold probably makes more sense.

@DiegoAndai
Copy link
Member

Do we also put the ButtonGroup on hold

I replied on the ButtonGroup PR 🙌🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: switch This is the name of the generic UI component, not the React module! design: material you on hold There is a blocker, we need to wait
Projects
None yet
Development

No branches or pull requests

6 participants