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

feat: adjust Menu #3118

Merged
merged 4 commits into from
May 26, 2022
Merged

feat: adjust Menu #3118

merged 4 commits into from
May 26, 2022

Conversation

lukewalczak
Copy link
Member

@lukewalczak lukewalczak commented Mar 11, 2022 •
edited
Loading

Summary

PR

  • adjusts Menu and Menu.Item into Material You
  • introduces trailingIcon - trailing icon to display for the MenuItem
  • renamed from 'icon' to 'leadingIcon' in v3.x indicating leading icon to display for the MenuItem.

Test plan

  • added menu item snapshot
  • check application in both themes, if Menu and Menu items works and have correct colors in both themes on both platforms

@callstack-bot
Copy link

callstack-bot commented Mar 11, 2022 •
edited
Loading

Hey @lukewalczak, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

@lukewalczak lukewalczak marked this pull request as ready for review March 27, 2022 11:58
@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

1 similar comment
@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

p-syche
p-syche approved these changes Apr 27, 2022
View reviewed changes
Copy link
Contributor

@p-syche p-syche left a comment

Choose a reason for hiding this comment

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

Looking good 😎

I left one nitpick suggestion about leaving a comment in the code, take it or leave it ;)

src/components/Menu/MenuItem.tsx Show resolved Hide resolved
@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

Drakeoon
Drakeoon approved these changes May 25, 2022
View reviewed changes
Copy link
Contributor

@Drakeoon Drakeoon left a comment

Choose a reason for hiding this comment

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

My requested change is mostly for readability purposes, it doesn't prevent merging the PR

src/components/Menu/MenuItem.tsx Outdated Show resolved Hide resolved
@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here

.

@lukewalczak lukewalczak merged commit 9d0ae64 into next May 26, 2022
@lukewalczak lukewalczak deleted the feat/adjust-menu branch May 26, 2022 12:16
lukewalczak added a commit that referenced this pull request Jun 8, 2022
@lukewalczak
feat: adjust Menu (#3118)
377ab3b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drplauska drplauska drplauska left review comments

@Drakeoon Drakeoon Drakeoon approved these changes

@p-syche p-syche p-syche approved these changes

Assignees
No one assigned
Labels
Material You (v5) Menu
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lukewalczak @callstack-bot @p-syche @drplauska @Drakeoon