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.

Sign up for GitHub

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

Jump to bottom

[material-next][ButtonGroup] Copy ButtonGroup to material next #39739

Merged
merged 18 commits into from
Nov 7, 2023
Merged

[material-next][ButtonGroup] Copy ButtonGroup to material next #39739

merged 18 commits into from
Nov 7, 2023
+940 −2

Conversation

lhilgert9
Copy link
Contributor

@lhilgert9 lhilgert9 commented Nov 4, 2023 •
edited

ButtonGroup issue: #39686
Material You umbrella issue: #29345

Changes

  • Copy ButtonGroup from material v5
  • Fix imports
  • implement ButtonGroupContext in material-next Button
  • change disableFocusRipple to disableTouchRipple like Button

@DiegoAndai @mj12albert

@mui-bot
Copy link

mui-bot commented Nov 4, 2023 •
edited

Netlify deploy preview

https://deploy-preview-39739--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 5697536

@lhilgert9 lhilgert9 mentioned this pull request Nov 4, 2023
Open
7 tasks
@lhilgert9 lhilgert9 marked this pull request as ready for review November 4, 2023 20:26
@lhilgert9
Copy link
Contributor Author

@DiegoAndai, I think the way I have done it here should be much better. Could you also change in #29345 that the ButtonGroup is In progress and no longer Ready to take?

@mj12albert mj12albert self-assigned this Nov 6, 2023
@zannager zannager added the component: ButtonGroup The React component. label Nov 6, 2023
@mj12albert mj12albert mentioned this pull request Nov 7, 2023
Closed
1 task
mj12albert
mj12albert requested changes Nov 7, 2023
View reviewed changes
Copy link
Member

@mj12albert mj12albert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lhilgert9 Looks good 👍 just a few minor changes ~

packages/mui-material-next/src/ButtonGroup/ButtonGroupContext.ts Outdated Show resolved Hide resolved
packages/mui-material-next/src/Button/Button.tsx Outdated Show resolved Hide resolved
packages/mui-material-next/src/ButtonGroup/ButtonGroup.test.js Outdated Show resolved Hide resolved
@mj12albert mj12albert added the v6.x label Nov 7, 2023
lhilgert9 and others added 4 commits November 7, 2023 09:55
@lhilgert9 @mj12albert
fix ButtonGroupContext naming
ce5b00f 
Co-authored-by: Albert Yu <albert@albertyu.co>
Signed-off-by: Lucas Hilgert <77863078+lhilgert9@users.noreply.github.com>
@lhilgert9 @mj12albert
fix comment naming
2578811 
Co-authored-by: Albert Yu <albert@albertyu.co>
Signed-off-by: Lucas Hilgert <77863078+lhilgert9@users.noreply.github.com>
@lhilgert9
fix positionClassName naming
85c4db1
@lhilgert9
fix ButtonGroupContextType
5697536
mj12albert
mj12albert approved these changes Nov 7, 2023
View reviewed changes
Copy link
Member

@mj12albert mj12albert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lhilgert9 Nice work 👍

@mj12albert mj12albert merged commit 6bc9bab into mui:master Nov 7, 2023
20 checks passed
@lhilgert9 lhilgert9 deleted the copy-button-group-to-material-next branch November 7, 2023 17:07
@DiegoAndai DiegoAndai added v7.x and removed v6.x labels Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mj12albert mj12albert mj12albert approved these changes

Assignees

@mj12albert mj12albert

Labels
component: ButtonGroup The React component. v7.x
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@lhilgert9 @mui-bot @mj12albert @DiegoAndai @zannager