Twenty Twenty: anchor links don't work in mobile menu

[Giorgio25b]

I’m testing a scenario where there is only 1 single page and therefore the main and mobile menu are structured on a #anchor-links structure: #project, #about, #and-so-on
I did try to use the shortcut url structure /#project and also the full domain path ​https://my-site.com/#project but there is nothing that is actually triggering the mobile menu to close and reach that url; though the url query changes in the browser without actually getting there.
This issue was reported and fixed on the Chaplin theme ​https://wordpress.org/support/topic/mobile-menu-doesnt-work-for-anchor-links/#post-12134002 though despite the same code approach, Chaplin is using jQuery while TwentyTwenty is plain JS.

[ianbelanger]

Just tested this and can confirm that anchor links do not work in the Mobile Menu or Desktop Expanded Menu locations.

[suzylah]

I also have this issue – one-pager website with the main navigation all linking to #anchors within the page. The problem is, they DO actually work (smooth scroll), but only on the second click. In fact, even if the second click is yet on another link, it then scrolls to the previous one.
Example (when performed in this order):
1) Click link #A – Scrolls to #A as expected.
2) Click link #B – nothing happens
3) Click link #C – scrolls to #B
4) Click link #D – scrolls to #C
5) Click link #B, scrolls to #D
And so on...
It appears as if the target is just saved on first click, and acted upon on the next click – but I couldn't find the place in the code yet where this happens.

PS. In the mobile menu, there was a second (CSS) issue on top, the html element gets the inline styles {overflow-y: scroll;position: fixed; ... etc...} set when menu opens, which fixes the whole window and nothing seem to happen on click.
I solved it with a quick&dirty html {position: static!important;overflow-y: unset!important;}

[yuhin]

I have exactly the same issue. They don't go all the way down to the right anchor after the first anchor.

Replying to suzylah:

I also have this issue – one-pager website with the main navigation all linking to #anchors within the page. The problem is, they DO actually work (smooth scroll), but only on the second click. In fact, even if the second click is yet on another link, it then scrolls to the previous one.
Example (when performed in this order):
1) Click link #A – Scrolls to #A as expected.
2) Click link #B – nothing happens
3) Click link #C – scrolls to #B
4) Click link #D – scrolls to #C
5) Click link #B, scrolls to #D
And so on...
It appears as if the target is just saved on first click, and acted upon on the next click – but I couldn't find the place in the code yet where this happens.

PS. In the mobile menu, there was a second (CSS) issue on top, the html element gets the inline styles {overflow-y: scroll;position: fixed; ... etc...} set when menu opens, which fixes the whole window and nothing seem to happen on click.
I solved it with a quick&dirty html {position: static!important;overflow-y: unset!important;}

[samful]

The patch made by @bdcstr (​https://wordpress.org/support/topic/mobile-menu-doesnt-work-for-anchor-links/#post-12487136) works for me, but he says it is "temporary-and-not-perfect". I'm not sure why this is, but it seems to make the mobile menu go away and go to the correct anchor when a hyperlink on the mobile menu is clicked for me, and this doesn't occur without the fix.

I know you are busy @anlino, but can you comment on this patch? It looks like it's almost fixed to me and after fixing the Chaplin version hopefully you know what to do.

[bdcstr]

"Temporary" because the fix could be overwritten when you update the theme to the future version -> you need to keep that in mind
"Not perfect".. Honestly I don't recall why I said that 2 months ago :D It worked for me, and I'm happy if it helps someone ;)

Replying to samful:

The patch made by @bdcstr (​https://wordpress.org/support/topic/mobile-menu-doesnt-work-for-anchor-links/#post-12487136) works for me, but he says it is "temporary-and-not-perfect". I'm not sure why this is, but it seems to make the mobile menu go away and go to the correct anchor when a hyperlink on the mobile menu is clicked for me, and this doesn't occur without the fix.

I know you are busy @anlino, but can you comment on this patch? It looks like it's almost fixed to me and after fixing the Chaplin version hopefully you know what to do.

https://core.trac.wordpress.org/ticket/48916 patch mobile menu anchor links, modal menu won't close after click.

<!--
Hi there! Thanks for contributing to WordPress!

Pull Requests in this GitHub repository must be linked to a ticket in the WordPress Core Trac instance (​https://core.trac.wordpress.org), and are only used for code review. No pull requests will be merged on GitHub.

See the WordPress Handbook page on using PRs for Code Review more information: ​https://make.wordpress.org/core/handbook/contribute/git/github-pull-requests-for-code-review/

If this is your first time contributing, you may also find reviewing these guides first to be helpful:

• FAQs for New Contributors: ​https://make.wordpress.org/core/handbook/tutorials/faq-for-new-contributors/
• Contributing with Code Guide: ​https://make.wordpress.org/core/handbook/contribute/
• WordPress Coding Standards: ​https://make.wordpress.org/core/handbook/best-practices/coding-standards/
• Inline Documentation Standards: ​https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/
• Browser Support Policies: ​https://make.wordpress.org/core/handbook/best-practices/browser-support/
• Proper spelling and grammar related best practices: ​https://make.wordpress.org/core/handbook/best-practices/spelling/

-->

Trac ticket:

[Giorgio25b]

I've created a [PR](​https://github.com/WordPress/wordpress-develop/pull/254) with the patch suggested by @bdcstr
The patch has been tested and it works; the PR carries comments and console.log lines.

[ianbelanger]

Updated patch that adds modal check

[ianbelanger]

Thanks for all of your work on this @Giorgio25b, @bdcstr, @samful, @yuhin and @suzylah. I have just uploaded an updated patch that includes a check to see if the modal is active. After applying @Giorgio25b patch clicking on an anchor link that was not in the modal would throw an error in the console.

Uncaught TypeError: Cannot read property 'dataset' of null

My patch fixes this by adding a check to make sure that modal is not null.

Testing on multiple different devices would be appreciated.

[Giorgio25b]

Hi @ianbelanger , thanks for the attention to details!
I've pushed a comprehensive PR with a fully tested patch (local, live, mobile, tablet, win/mac, etc).
You can find it here, I wasn't sure if your diff push was requiring a new PR, but just in case ;-)
​https://github.com/WordPress/wordpress-develop/pull/254/commits/b58ce5062a390413cea7aed317186dd417b4071f#diff-d25e57a01a8a4bcabdb796cfefbe4b63R141

[ianbelanger]

Thanks for testing @Giorgio25b. There is no need to submit a PR on the github repo, as trac (here) is where WordPress is actually patched. I am reopening this ticket because the patch I submitted has not been committed to core yet. I will commit this on Monday. Thanks again!!

[yuhin]

@ianbelanger Thanks for working on this!

[Giorgio25b]

@yuhin @ianbelanger I thoroughly tested again on different mobile devices and even older ones, different browsers passed the test. On Desktop I did not experience any issue with the developer tool on any browser.

[samful]

Just tested 48916.diff on Linux (Firefox 75.0), iPhone XR (latest Safari), and Google Pixel 3a (latest Chrome). All seems ok! Good job with the modal fix @ianbelanger

[ianbelanger]

In 47784:

Bundled Themes: Twenty Twenty anchor links don't work in mobile menu.

Modifies the mobile modal menu javascript, so that anchor links will close the modal and scroll to the anchor within the page.

Props Giorgio25b, suzylah, yuhin, samful, bdcstr.
Fixes #48916.

[ianbelanger]

Reopening for backport.

[whyisjake]

In 47829:

Bundled Themes: Twenty Twenty anchor links don't work in mobile menu.
Modifies the mobile modal menu javascript, so that anchor links will close the modal and scroll to the anchor within the page.

This brings the changes from [47784] to the 5.4 branch.

Props Giorgio25b, suzylah, yuhin, samful, bdcstr.
Fixes #48916.

Committed in https://core.trac.wordpress.org/changeset/47784.