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.

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

[Menu] Overhaul the component API #468

Merged
merged 122 commits into from
Aug 2, 2024
Merged

[Menu] Overhaul the component API #468

merged 122 commits into from
Aug 2, 2024

Conversation

michaldudak
Copy link
Member

@michaldudak michaldudak commented Jun 26, 2024

@atomiks
Copy link
Contributor

atomiks commented Jul 25, 2024

Bug when traversing back to the submenu trigger, menu doesn't open

Screen.Recording.2024-07-25.at.11.33.36.PM.mov

@colmtuite
Copy link
Contributor

@michaldudak Feedback based on our chat:

  1. Add the Arrow component
  2. Consider making focus looping configurable. I vote it should be.
  3. Rename positionStrategy to positionMethod across all components.

@michaldudak
Copy link
Member Author

@colmtuite I did 1 and 2. positionStrategy rename should be done on the other components first in a separate PR.

@colmtuite
Copy link
Contributor

colmtuite commented Jul 26, 2024

@michaldudak In this vid, the menu is initially opening above the trigger, then quickly repositioning itself below.
https://github.com/user-attachments/assets/dee98c5c-a19b-47ab-8f8b-2c2f07b0eb30

@michaldudak
Copy link
Member Author

Bug when traversing back to the submenu trigger, menu doesn't open

@atomiks, Should be working OK now.

In this vid, the menu is initially opening above the trigger, then quickly repositioning itself below.

@colmtuite, could you prepare a reproduction on codesandbox?

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 1, 2024
@colmtuite colmtuite self-requested a review August 1, 2024 11:43
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 1, 2024
@michaldudak michaldudak mentioned this pull request Aug 2, 2024
14 tasks
Copy link
Contributor

@atomiks atomiks left a comment

Choose a reason for hiding this comment

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

Looks great!

@michaldudak michaldudak merged commit a71c099 into mui:master Aug 2, 2024
18 checks passed
@michaldudak michaldudak deleted the menu branch August 2, 2024 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: menu This is the name of the generic UI component, not the React module!
Projects
None yet
6 participants