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

[Tabs] Migrate Tabs to emotion #25824

Merged
merged 15 commits into from
Apr 23, 2021
Merged

[Tabs] Migrate Tabs to emotion #25824

merged 15 commits into from
Apr 23, 2021

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Apr 18, 2021 •
edited by oliviertassinari
Loading

One chunk of #24405

@mui-pr-bot
Copy link

mui-pr-bot commented Apr 18, 2021 •
edited
Loading

Details of bundle changes

@material-ui/core: parsed: +0.34% , gzip: +0.22%
@material-ui/lab: parsed: +0.41% , gzip: +0.28%

Generated by 🚫 dangerJS against 1cc92d9

@siriwatknp siriwatknp changed the title [Tabs] migrate Tabs to emotion [Tabs] Migrate Tabs to emotion Apr 18, 2021
@siriwatknp

This comment has been minimized.

Sign in to view
@mnajdova mnajdova mentioned this pull request Apr 18, 2021
Closed
1 task
@oliviertassinari oliviertassinari added the component: tabs This is the name of the generic UI component, not the React module! label Apr 18, 2021
oliviertassinari
oliviertassinari approved these changes Apr 18, 2021
View reviewed changes
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

At a high level, it looks good, but I didn't dive deep into the review. Best to have Marija review it

@siriwatknp siriwatknp mentioned this pull request Apr 18, 2021
Closed
mnajdova
mnajdova reviewed Apr 19, 2021
View reviewed changes
packages/material-ui/src/Tabs/Tabs.js Show resolved Hide resolved
packages/material-ui/src/Tabs/Tabs.js Show resolved Hide resolved
packages/material-ui/src/Tabs/Tabs.js Show resolved Hide resolved
packages/material-ui/src/Tabs/Tabs.js Show resolved Hide resolved
packages/material-ui/src/Tabs/Tabs.js Show resolved Hide resolved
packages/material-ui/src/Tabs/Tabs.js Outdated Show resolved Hide resolved
packages/material-ui/src/Tabs/Tabs.js Show resolved Hide resolved
@github-actions github-actions bot mentioned this pull request Apr 20, 2021
Closed
mnajdova
mnajdova reviewed Apr 21, 2021
View reviewed changes
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Good job, couple of things to be resolved before merging.

packages/material-ui/src/Tabs/Tabs.js Outdated Show resolved Hide resolved
packages/material-ui/src/Tabs/Tabs.js Outdated Show resolved Hide resolved
packages/material-ui/src/Tabs/Tabs.js Outdated Show resolved Hide resolved
packages/material-ui/src/Tabs/Tabs.js Outdated Show resolved Hide resolved
siriwatknp and others added 3 commits April 22, 2021 08:57
@siriwatknp @mnajdova
Update packages/material-ui/src/Tabs/Tabs.js
5e157ee 
Co-authored-by: Marija Najdova <mnajdova@gmail.com>
@siriwatknp @mnajdova
Update packages/material-ui/src/Tabs/Tabs.js
5095496 
Co-authored-by: Marija Najdova <mnajdova@gmail.com>
@siriwatknp
remove deepmerge
4de486e
mnajdova
mnajdova reviewed Apr 22, 2021
View reviewed changes
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Few final comments

packages/material-ui/src/Tabs/Tabs.js Outdated Show resolved Hide resolved
packages/material-ui/src/Tabs/Tabs.js Outdated Show resolved Hide resolved
packages/material-ui/src/Tabs/Tabs.js Outdated Show resolved Hide resolved
packages/material-ui/src/Tabs/Tabs.js Outdated Show resolved Hide resolved
packages/material-ui/src/Tabs/Tabs.js Show resolved Hide resolved
siriwatknp and others added 4 commits April 23, 2021 00:09
@siriwatknp @mnajdova
Update packages/material-ui/src/Tabs/Tabs.js
39c50af 
Co-authored-by: Marija Najdova <mnajdova@gmail.com>
@siriwatknp @mnajdova
Update packages/material-ui/src/Tabs/Tabs.js
1eafc19 
Co-authored-by: Marija Najdova <mnajdova@gmail.com>
@siriwatknp @mnajdova
Update packages/material-ui/src/Tabs/Tabs.js
9a7170f 
Co-authored-by: Marija Najdova <mnajdova@gmail.com>
@siriwatknp
update and remove unused code
1f3a77f
mnajdova
mnajdova approved these changes Apr 23, 2021
View reviewed changes
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

I just pushed one fix for the scrollButtons overrides, looks good otherwise 👍

@mnajdova mnajdova merged commit 7f3653e into mui:next Apr 23, 2021
@oliviertassinari
Copy link
Member

@material-ui/core => 100% migrated 🙏

@mnajdova
Copy link
Member

@material-ui/core => 100% migrated 🙏

Looks like we both waited to see this too long now 😄

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

@mnajdova mnajdova mnajdova approved these changes

Assignees
No one assigned
Labels
component: tabs This is the name of the generic UI component, not the React module!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@siriwatknp @mui-pr-bot @oliviertassinari @mnajdova