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

[ButtonGroup] Fix style to use last-child #32424

Closed
wants to merge 4 commits into from
Closed

Conversation

zoy-l
Copy link
Contributor

@zoy-l zoy-l commented Apr 22, 2022

last-of-type or first-of-type will look up different elements as two queues, so the style will be invalidated when customizing the button component

I looked at the information emotion-js/emotion#1178 (comment) emotion-js/emotion#1105 last-child should be safe

I'm not sure if accept situation, if sure, please let me know if I need to add a test case Thanks! 🤔

demo: https://codesandbox.io/s/suspicious-mopsa-o1bqtu?file=/index.html

@zoy-l
Copy link
Contributor Author

zoy-l commented Apr 22, 2022

#31210,#29224

@zoy-l zoy-l changed the title [buttonGroup] style fix #31210 [buttonGroup] style fix Apr 22, 2022
@mui-bot
Copy link

mui-bot commented Apr 22, 2022

Details of bundle changes

Generated by 🚫 dangerJS against c040112

@danilo-leal danilo-leal changed the title [buttonGroup] style fix [ButtonGroup] Fix style to use last-child Apr 26, 2022
@danilo-leal danilo-leal added the component: ButtonGroup The React component. label Apr 26, 2022
@zoy-l
Copy link
Contributor Author

zoy-l commented Apr 27, 2022

I checked the ci, and I don't quite understand why this fails, it seems that it has nothing to do with my modification 😅

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label May 6, 2022
@mnajdova
Copy link
Member

Can we add regression tests for the examples that fails, for example using ButtonGroup component. You can check some of the tests already added in https://github.com/mui/material-ui/tree/master/test/regressions/fixtures

@mnajdova mnajdova self-assigned this May 18, 2022
@michaldudak
Copy link
Member

IMO the better way to go here would be to implement the change mentioned in #29514. This way, each element of the group will be aware of the position it has and will be able to be styled appropriately.

@zoy-l zoy-l closed this Nov 1, 2022
@zoy-l zoy-l deleted the buttonGroup branch November 1, 2022 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: ButtonGroup The React component. PR: out-of-date The pull request has merge conflicts and can't be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ButtonGroup] Breaking when adding href to Button
5 participants