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-ui][SpeedDial] Deprecate TransitionComponent #40698

Merged
merged 21 commits into from
May 3, 2024
Merged

[material-ui][SpeedDial] Deprecate TransitionComponent #40698

merged 21 commits into from
May 3, 2024

Conversation

harry-whorlow
Copy link
Contributor

Part of: #40417

@DiegoAndai here's my submission for the speedDial deprecation of TransitionComponent

Changes made to speedDial component
speedDial: Add deprecations for transition props and the slots API that should replace it.
speedDial: Updated prototypes and docs

Questions for reviewer
I see that this component has tests for the Fade transition should the Fade, in these tests, be passed in slots?

@mui-bot
Copy link

mui-bot commented Jan 20, 2024 •
edited
Loading

Netlify deploy preview

SpeedDial: parsed: +2.51% , gzip: +2.34%

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 727c790

@zannager zannager added the deprecation New deprecation message label Jan 22, 2024
@danilo-leal danilo-leal changed the title Deprecate TransitionComponent [material-ui] Deprecate TransitionComponent Jan 22, 2024
@danilo-leal danilo-leal added component: transitions This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material labels Jan 22, 2024
@DiegoAndai DiegoAndai changed the title [material-ui] Deprecate TransitionComponent [material-ui][SpeedDial] Deprecate TransitionComponent Jan 23, 2024
@DiegoAndai DiegoAndai added component: speed dial This is the name of the generic UI component, not the React module! and removed component: transitions This is the name of the generic UI component, not the React module! labels Jan 23, 2024
@harry-whorlow harry-whorlow marked this pull request as draft February 6, 2024 07:42
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 20, 2024
@harry-whorlow harry-whorlow mentioned this pull request Mar 13, 2024
Merged
@harry-whorlow harry-whorlow force-pushed the deprecate-speed-dial-transition branch from f7efb17 to c1b8ed4 Compare March 14, 2024 07:51
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 14, 2024
@harry-whorlow harry-whorlow changed the base branch from master to next April 8, 2024 11:23
@harry-whorlow
Copy link
Contributor Author

Sorry, I was away for holidays... I'll be picking this up over the coming two weeks.

@harry-whorlow harry-whorlow mentioned this pull request Apr 9, 2024
Open
34 tasks
@DiegoAndai
Copy link
Member

I wrote two utils that will make the codemods easier for this: #41685

It should be merged soon, I would recommend waiting for that 🙌🏼

@harry-whorlow
Copy link
Contributor Author

Sure thing! I'll check it out when I get a chance 🤟

@DiegoAndai
Copy link
Member

@harry-whorlow, #41685 was merged. You should be able to merge next into your branch and use the new util so the codemod is easier to implement.

@harry-whorlow
Copy link
Contributor Author

yo @DiegoAndai, hows it going man? I've got another one ready for code review.

Enjoy the weekend! 🤟

DiegoAndai
DiegoAndai reviewed Apr 15, 2024
View reviewed changes
Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Hey @harry-whorlow! Thanks for working on this, here's my initial review

packages/mui-codemod/src/deprecations/speed-dial-props/test-cases/theme.actual.js Outdated Show resolved Hide resolved
packages/mui-material/src/SpeedDial/SpeedDial.d.ts Outdated Show resolved Hide resolved
packages/mui-material/src/SpeedDial/SpeedDial.d.ts Outdated Show resolved Hide resolved
packages/mui-material/src/SpeedDial/SpeedDial.d.ts Outdated Show resolved Hide resolved
packages/mui-codemod/src/deprecations/speed-dial-props/speed-dial-props.js Show resolved Hide resolved
@harry-whorlow harry-whorlow force-pushed the deprecate-speed-dial-transition branch from f34de3c to c4c8f24 Compare April 16, 2024 18:22
@harry-whorlow
Copy link
Contributor Author

@DiegoAndai morning man, the changes are made as requested.🤟

DiegoAndai
DiegoAndai reviewed Apr 24, 2024
View reviewed changes
Copy link
Member

@DiegoAndai DiegoAndai left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hey @harry-whorlow! Sorry for the late review, here's a couple more comments 😊

Also, let's mark this PR as ready for review, as we're already reviewing it.

packages/mui-material/src/SpeedDial/SpeedDial.d.ts Outdated Show resolved Hide resolved
packages/mui-material/src/SpeedDial/SpeedDial.d.ts Outdated Show resolved Hide resolved
packages/mui-codemod/src/deprecations/speed-dial-props/test-cases/actual.js Outdated Show resolved Hide resolved
@DiegoAndai DiegoAndai marked this pull request as ready for review April 24, 2024 18:49
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 29, 2024
@harry-whorlow harry-whorlow force-pushed the deprecate-speed-dial-transition branch from fc83d0b to 6da354f Compare April 29, 2024 19:34
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 29, 2024
@harry-whorlow
Copy link
Contributor Author

@DiegoAndai, comments resolved... Let me know if you have any more requests, enjoy the start to the week🤟

@harry-whorlow
removed extra file
c52410c 
Signed-off-by: Harry Whorlow <[email protected]>
DiegoAndai
DiegoAndai reviewed Apr 30, 2024
View reviewed changes
Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Hey! A couple more comments 😊

When you implement the changes, remember to run pnpm proptypes and pnpm docs:api to update the corresponding docs.

Good week to you too!

packages/mui-material/src/SpeedDial/SpeedDial.d.ts Outdated Show resolved Hide resolved
packages/mui-codemod/src/deprecations/speed-dial-props/test-cases/theme.actual.js Outdated Show resolved Hide resolved
packages/mui-codemod/src/deprecations/speed-dial-props/test-cases/actual.js Outdated Show resolved Hide resolved
@harry-whorlow
Copy link
Contributor Author

@DiegoAndai coming at you with some fresh hot PR corrections.🤟

DiegoAndai
DiegoAndai reviewed May 2, 2024
View reviewed changes
Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Hey @harry-whorlow! here's a new review 😊

packages/mui-codemod/src/deprecations/speed-dial-props/test-cases/theme.expected.js Outdated Show resolved Hide resolved
packages/mui-material/src/SpeedDial/SpeedDial.d.ts Outdated Show resolved Hide resolved
packages/mui-material/src/SpeedDial/SpeedDial.d.ts Outdated Show resolved Hide resolved
@DiegoAndai
Copy link
Member

I just noticed your question:

I see that this component has tests for the Fade transition should the Fade, in these tests, be passed in slots?

Which tests are you referring to?

@harry-whorlow
Copy link
Contributor Author

@DiegoAndai, morning man! more revisions ready for review🤟

With regards to the question I would forget about it... I can't remember what it was about, plus it was made at the very start.

Enjoy the weekend!

DiegoAndai
DiegoAndai approved these changes May 3, 2024
View reviewed changes
Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Nice work @harry-whorlow! I hope you're able to continue contributing to this effort 😊

@DiegoAndai DiegoAndai merged commit 24a604e into mui:next May 3, 2024
22 checks passed
@harry-whorlow
Copy link
Contributor Author

Morning @DiegoAndai, thanks man! 🤟

I'll look at the task list Monday and pull out the next one, thanks for all the time spent reviewing the code and suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DiegoAndai DiegoAndai DiegoAndai approved these changes

Assignees
No one assigned
Labels
component: speed dial This is the name of the generic UI component, not the React module! deprecation New deprecation message package: material-ui Specific to @mui/material
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@harry-whorlow @mui-bot @DiegoAndai @danilo-leal @zannager