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

Navigation menubar: Fix scroll-to-top problem reported in issue #1307 #1308

Merged
merged 4 commits into from
Feb 5, 2020

Conversation

carmacleod
Copy link
Contributor

@carmacleod carmacleod commented Jan 24, 2020

Issue #1307 reports 2 issues with the navigation menubar example. This PR resolves the scroll to top issue by preventing menuitem anchor elements from jumping to href="#" (i.e. top of page in Chrome) if they have a menu/submenu.

Here's a preview link.

Review checklist

@mcking65 mcking65 changed the title Fixes the scroll-to-top problem in issue #1307 Navigation menubar: Fix scroll-to-top problem reported in issue #1307 Jan 26, 2020
Copy link
Contributor

@spectranaut spectranaut left a comment

Choose a reason for hiding this comment

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

This change doesn't need or effect existing tests, so the "test review" can be marked as complete. As soon as I understand the necessity of the if (this.popupMenu) I'll approve it.

Thanks for fixing this bug, @carmacleod!

@carmacleod
Copy link
Contributor Author

@spectranaut
I improved the comment a bit in the 2 places with if (this.popupMenu).
Please let me know if you think it's clearer now.

@mcking65 mcking65 merged commit 4667318 into master Feb 5, 2020
@mcking65 mcking65 deleted the issue1307-scroll-to-top branch February 5, 2020 07:17
michael-n-cooper pushed a commit that referenced this pull request Feb 5, 2020
Navigation menubar: Fix scroll-to-top problem for issue 1307 (pull #1308)

Issue #1307 reported 2 problems with the navigation menubar example.
This commit resolves the scroll to top issue by
preventing menuitem anchor elements from jumping to href="#" (i.e. top of page in Chrome) if they have a menu/submenu.

Co-authored-by: Matt King <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation menubar example: Clicking menu item scrolls page to top in Chrome
4 participants