Skip to content

Move menu toggle to the items#600

Merged
NullVoxPopuli merged 1 commit into
universal-ember:mainfrom
barryofguilder:jb/menu-link
Dec 19, 2025
Merged

Move menu toggle to the items#600
NullVoxPopuli merged 1 commit into
universal-ember:mainfrom
barryofguilder:jb/menu-link

Conversation

@barryofguilder
Copy link
Copy Markdown
Contributor

@barryofguilder barryofguilder commented Dec 18, 2025

If you have a Menu thats fixed in your app and then you click a LinkItem to navigate to another route in your app, the menu content will still be displaying. This is because it seems that the click event on the link isn't bubbling up to the Content component's click handler.

This moves the toggle click handler to the Item and LinkItem components of a Menu rather than on the Content component. This way, clicking on a LinkItem will always fire the toggle of the menu.

This fixes #591

@bolt-new-by-stackblitz
Copy link
Copy Markdown

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@barryofguilder barryofguilder marked this pull request as ready for review December 18, 2025 14:41
Copy link
Copy Markdown
Contributor

@NullVoxPopuli NullVoxPopuli left a comment

Choose a reason for hiding this comment

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

Approach looks good!

Are there existing tests around this behavior?

@NullVoxPopuli NullVoxPopuli added the bug Something isn't working label Dec 18, 2025
@barryofguilder
Copy link
Copy Markdown
Contributor Author

Approach looks good!

Are there existing tests around this behavior?

Good call. I don't think there's a test that currently verifies clicking on an item closes the menu. I can add one. I'm not sure how to test the LinkItem though because we don't want the test to navigate away to another page.

@NullVoxPopuli
Copy link
Copy Markdown
Contributor

LinkItem can have # for the href
Tyty!

@barryofguilder barryofguilder force-pushed the jb/menu-link branch 2 times, most recently from e58a0ed to 2c2614b Compare December 19, 2025 14:19
@NullVoxPopuli NullVoxPopuli merged commit 37091b3 into universal-ember:main Dec 19, 2025
19 checks passed
@github-actions github-actions Bot mentioned this pull request Dec 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Selecting a LinkItem with your keyboard doesn't close the Menu

2 participants