-
Notifications
You must be signed in to change notification settings - Fork 64
Toggle submenu on click menu link text #161
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
Conversation
Also fix the smooth scroll behavior of menu link. |
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.
This code change makes sense to me and simplifies a bit. Do we have a sense of how compatible it is across browsers?
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.
This code change makes sense to me and simplifies a bit. Do we have a sense of how compatible it is across browsers?
@bkardell, could you review this? |
@othree - this looks good to me, can you rebase? |
Use css instead of JS to scroll to target Which use less code and can contain navigation history, also make the url in location bar correct.
@codehag I have updated the PR. But the CI failed for unknown reason. |
I found its prettier check failed issue. Should we run prettier manually? |
generally one runs linting/formatting manually. |
This looks good now, thanks. Regarding the prettier question, we can bring it up in the website meeting. |
Fixes #146
Didn't call
t.click()
because iOS might disable all these kind of operations(js to mimic user interaction) in the future.