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

[docs] Add MUI packages explanation #29073

Merged
merged 18 commits into from
Oct 22, 2021
Merged

[docs] Add MUI packages explanation #29073

merged 18 commits into from
Oct 22, 2021

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Oct 15, 2021 •
edited by mnajdova
Loading

@siriwatknp siriwatknp added the docs Improvements or additions to the documentation label Oct 15, 2021
@mui-pr-bot
Copy link

mui-pr-bot commented Oct 15, 2021 •
edited
Loading

No bundle size changes

Generated by 🚫 dangerJS against 1667fb9

mnajdova
mnajdova previously requested changes Oct 15, 2021
View reviewed changes
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
siriwatknp and others added 2 commits October 17, 2021 10:04
danilo-leal
danilo-leal reviewed Oct 18, 2021
View reviewed changes
Copy link
Contributor

@danilo-leal danilo-leal left a comment

Choose a reason for hiding this comment

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

Nice one! I have just pushed a commit with a different layers image, just with a tad more polished design.

docs/src/pages.ts Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/mui-packages/mui-packages.md Outdated Show resolved Hide resolved
@mbrookes
Copy link
Member

I'm holding fire on reviewing until the outstanding reviews are agreed and merged.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 19, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 19, 2021
@siriwatknp
Copy link
Member Author

I'm holding fire on reviewing until the outstanding reviews are agreed and merged.

@mbrookes Please go ahead.

mnajdova
mnajdova reviewed Oct 19, 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.

One last comment. All good from my side 👍

docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
@mnajdova mnajdova dismissed their stale review October 19, 2021 12:51

Comments resolved

@mbrookes
Copy link
Member

Review coming, but I ran out of credits :D

mnajdova
mnajdova approved these changes Oct 20, 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.

All good from my end. Looks awesome!

@siriwatknp siriwatknp added the on hold There is a blocker, we need to wait label Oct 20, 2021
mbrookes
mbrookes requested changes Oct 20, 2021
View reviewed changes
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
danilo-leal
danilo-leal approved these changes Oct 20, 2021
View reviewed changes
Copy link
Contributor

@danilo-leal danilo-leal left a comment

Choose a reason for hiding this comment

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

Looking great! :)

@mbrookes
Copy link
Member

#29073 (comment)

I think @michaldudak had similar concerns. Perhaps we should have announced "MUI Material v5" instead? 😅

mbrookes
mbrookes requested changes Oct 20, 2021
View reviewed changes
Copy link
Member

@mbrookes mbrookes left a comment

Choose a reason for hiding this comment

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

Last suggestion

@siriwatknp
Copy link
Member Author

MUI Material v5

Yeah, I like that.

mbrookes
mbrookes requested changes Oct 20, 2021
View reviewed changes
Copy link
Member

@mbrookes mbrookes left a comment

Choose a reason for hiding this comment

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

Sorry, I had only reviewed the markdown. After I approved the PR I looked over the preview, and noticed some more opportunities for possible improvement, and a couple of questions...

docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
@siriwatknp siriwatknp requested a review from mbrookes October 21, 2021 08:44
mbrookes
mbrookes reviewed Oct 21, 2021
View reviewed changes
Copy link
Member

@mbrookes mbrookes left a comment

Choose a reason for hiding this comment

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

Final, final feedback in the most reviewed docs PR in the history of docs PRs :)

docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Show resolved Hide resolved
docs/src/pages/guides/understand-mui-packages/understand-mui-packages.md Outdated Show resolved Hide resolved
@siriwatknp siriwatknp merged commit 5912e7b into mui:master Oct 22, 2021
@mbrookes
Copy link
Member

🚀

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

@michaldudak michaldudak michaldudak left review comments

@mbrookes mbrookes mbrookes approved these changes

@mnajdova mnajdova mnajdova approved these changes

@hbjORbj hbjORbj hbjORbj approved these changes

@danilo-leal danilo-leal danilo-leal approved these changes

@oliviertassinari oliviertassinari Awaiting requested review from oliviertassinari

Assignees
No one assigned
Labels
docs Improvements or additions to the documentation on hold There is a blocker, we need to wait
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@siriwatknp @mui-pr-bot @mbrookes @mnajdova @michaldudak @hbjORbj @danilo-leal