-
-
Notifications
You must be signed in to change notification settings - Fork 285
[menu] Add <Menu.Link> part
#3400
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
base: master
Are you sure you want to change the base?
Conversation
dd405c8 to
5b39504
Compare
commit: |
Bundle size report
Check out the code infra dashboard for more information about this PR. |
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Or this could just be |
5b39504 to
ee02677
Compare
LukasTy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. 👍
Am I missing something? Or does this component really is missing the menuitem prop?
Screen reader behavior in the experiment example is not optimal. 🙈
| const { render, clock } = createRenderer({ | ||
| clockOptions: { | ||
| shouldAdvanceTime: true, | ||
| }, | ||
| }); | ||
|
|
||
| clock.withFakeTimers(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: Doesn't look like clock faking is needed. 🤔
| const { render, clock } = createRenderer({ | |
| clockOptions: { | |
| shouldAdvanceTime: true, | |
| }, | |
| }); | |
| clock.withFakeTimers(); | |
| const { render } = createRenderer(); |
8621e8f to
6366c8f
Compare
6366c8f to
7da6bb0
Compare
Replacement for
<MenuItem render={<a href="#" />}Preview: https://deploy-preview-3400--base-ui.netlify.app/experiments/menu/menu-fully-featured