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

[ExpansionPanel] Add TransitionComponent prop #14617

Merged
merged 8 commits into from
Feb 22, 2019
Merged

[ExpansionPanel] Add TransitionComponent prop #14617

merged 8 commits into from
Feb 22, 2019

Conversation

ptbrowne
Copy link
Contributor

@ptbrowne ptbrowne commented Feb 21, 2019 •
edited by oliviertassinari

Help with #10931

Adding Prop to customize the Collapse component used in ExpansionPanel.
Increase customizability and opportunities to improve performance.

Checklist

  • The branch is targeted at: next for everything else including breaking changes.
  • It includes relevant tests.
  • The code is formatted (run yarn prettier).
  • The code is linted (run yarn lint).
  • If API documentation is being changed in the source, yarn docs:api was run.
  • If prop types were changed, the TypeScript declarations were updated.
  • If TypeScript declarations were changed, yarn typescript passed.
  • The PR title follows the pattern [Component] Imperative commit message. (See: How to Write a Git Commit Message for a great explanation)

@ptbrowne ptbrowne mentioned this pull request Feb 21, 2019
Open
1 task
eps1lon
eps1lon previously requested changes Feb 21, 2019
View reviewed changes
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

The implementation is perfect. Only the tests are lacking. I know this can be tedious but it's incredibly helpful when maintaining the components.

packages/material-ui/src/ExpansionPanel/ExpansionPanel.js Outdated Show resolved Hide resolved
packages/material-ui/src/ExpansionPanel/ExpansionPanel.test.js Outdated Show resolved Hide resolved
packages/material-ui/src/ExpansionPanel/ExpansionPanel.test.js Outdated Show resolved Hide resolved
@eps1lon eps1lon added new feature New feature or request component: accordion This is the name of the generic UI component, not the React module! labels Feb 21, 2019
@mui-pr-bot
Copy link

mui-pr-bot commented Feb 21, 2019 •
edited

This PR introduced some changes to the bundle size.

Details of bundle changes.

Comparing: f0cdca8...99c275a

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core/Paper 0.00% 0.00% 83040 83040 21410 21410
@material-ui/core/Paper.esm 0.00% 0.00% 76728 76728 20462 20462
@material-ui/core +0.01%:small_red_triangle: 0.00% 484797 484869 130083 130086
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16235 16235 5159 5159
@material-ui/styles 0.00% 0.00% 63755 63755 18119 18119
@material-ui/system 0.00% 0.00% 15915 15915 3938 3938
colorManipulator 0.00% 0.00% 2290 2290 838 838
Button 0.00% 0.00% 212888 212888 61970 61970
Modal 0.00% 0.00% 211086 211086 61681 61681
@material-ui/core/Popper 0.00% 0.00% 146815 146815 46826 46826
@material-ui/core/useMediaQuery 0.00% 0.00% 9386 9386 3563 3563
docs.main 0.00% 0.00% 675841 675841 204947 204947
docs.landing 0.00% 0.00% 48963 48963 10275 10275
packages/material-ui/build/umd/material-ui.production.min.js +0.02%:small_red_triangle: +0.03%:small_red_triangle: 320431 320503 84703 84726

Generated by 🚫 dangerJS against 99c275a

@ptbrowne
Copy link
Contributor Author

Will rebase once everything is OK with you @eps1lon

eps1lon
eps1lon previously requested changes Feb 21, 2019
View reviewed changes
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Test looks perfect. "Just" one minor question about naming.

packages/material-ui/src/ExpansionPanel/ExpansionPanel.js Outdated Show resolved Hide resolved
@oliviertassinari oliviertassinari changed the title [ExpansionPanel] Prop to customize Collapse [ExpansionPanel] Add TransitionComponent property Feb 22, 2019
@ptbrowne
Copy link
Contributor Author

Do you prefer that I squash my commits ? Rebase ? Or is it OK right now ?

@eps1lon eps1lon merged commit 3e0ae73 into mui:next Feb 22, 2019
@eps1lon
Copy link
Member

eps1lon commented Feb 22, 2019

Do you prefer that I squash my commits ? Rebase ? Or is it OK right now ?

All good. We squash when merging. Thank you for working on this. Great first PR. Hope to see more from you 👍

@eps1lon eps1lon changed the title [ExpansionPanel] Add TransitionComponent property [ExpansionPanel] Add TransitionComponent prop Feb 22, 2019
@ptbrowne ptbrowne deleted the collapse-component branch February 22, 2019 15:53
@NMinhNguyen NMinhNguyen mentioned this pull request Nov 28, 2019
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliviertassinari oliviertassinari oliviertassinari approved these changes

@eps1lon eps1lon eps1lon approved these changes

Assignees
No one assigned
Labels
component: accordion This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ptbrowne @mui-pr-bot @eps1lon @oliviertassinari