Close overlay menu when clicking an anchor link #39625

Trapsta · 2022-03-22T06:58:51Z

What?

This PR adds an event Listener to toggle the navigation block overlay menu on clicking links inside the overlay.

Why?

Navigating to anchor links in the navigation block in an overlay menu does not automatically close the overlay. This requires manually closing the modal to view the link's content.

How?

Since we are using Micromodal to implement the overlay menu, am using the MicroModal.close() method to toggle the modal on clicking the navigation link. My initial idea was to use the data-micromodal-close custom attribute which Micromodal uses as a trigger to close the modal but I couldn't get it to work reliably. The overlay would close but sometimes the browser would not navigate to the correct anchor.

Testing Instructions

Open a post or page.
Create a navigation menu and add an anchor link.
View the post/page on the frontend.
Make your browser screen smaller, below 600px.
Open the menu and click the anchor link.
Observe that the overlay menu modal closes and page navigates to the anchor link's content. Try also testing by changing the navigation block overlay menu display setting to off , always or mobile.

Screenshots or screencast

Before

Screen.Recording.2022-03-20.at.12.23.56.mov

After

Screen.Recording.2022-03-21.at.12.40.57.mov

Fixes #39585

github-actions · 2022-03-22T06:59:15Z

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @Trapsta! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

getdave · 2022-03-22T14:29:21Z

Thanks for the PR! 👏

I'm wondering could you complete the How section to describe why you feel this is the best approach and what alternatives you may have considered.

I was considering tackling this so it's great to see it's already being addressed. Thank you!

JosVelasco · 2022-03-22T23:49:42Z

Hello @Trapsta. Thanks for the PR. For testing purposes consider that you can also set the overlay menu to be visible always.

Collapses the navigation options in a menu icon opening an overlay:
Off | Mobile | Always

Trapsta · 2022-03-23T01:08:45Z

Thanks @getdave , sure thing - I will add some context on that. Noted @JosVelasco , thanks for the heads up , I had probably overlooked that part

getdave · 2022-03-23T11:12:08Z

Thanks @getdave , sure thing - I will add some context on that. Noted @JosVelasco , thanks for the heads up , I had probably overlooked that part

Thanks. Let me know when it's ready for review.

Trapsta · 2022-04-13T10:49:41Z

@getdave @JosVelasco I have updated the PR, please review when you can

paaljoachim · 2022-04-14T14:36:40Z

I was asked by Jos to take a look at this PR.
I am following this method: https://make.wordpress.org/design/2021/03/03/testing-a-gutenberg-pull-request-pr/

Testing.
Local site
WP 6.0 Beta 1.
Twenty Twenty One
Chrome
OSX on my Macbook Pro.

Opened an existing page.
Added a Navigation block.
Added a few random links to pages and posts.
Added an anchor link.
Noticed that the Navigation block also used the dot save system. Saved the page and the Navigation Menus.

Previewed on the frontend.
Opened up Chrome dev tools and made the screen smaller than 600px.
Noticed the menu is now showing the hamburger/mobile menu.
Clicked the anchor link.

Overlay menu modal closes and the page navigates to the anchor link's content.

Backend Navigation block settings.
Overlay Menu: Off - Mobile - Always. All three worked nicely with this PR fix.

Thank you @Trapsta !

Ps.
We should probably test this in multiple browsers.....

paaljoachim · 2022-04-14T14:37:36Z

As this is a bug fix I went ahead and added the backport to beta label.

paaljoachim

My focus was on testing the functionality that it worked as it should using Chrome.
The fix works very well!

JosVelasco · 2022-04-14T19:54:54Z

Also tested the PR on my end successfully. I even installed the popular plugin Page scroll to id to animate the scroll and it worked like a charm. Thanks @Trapsta.

Tested on
Local site
WordPress 5.9.3 and then WordPress 6.0-beta1
Twenty Twenty Two
Latest Firefox, Chrome and Safari.
OSX on my Macbook Air.

https://www.youtube.com/watch?v=uyP1rrljjuE

getdave

Thank you for this PR. I tested it and it worked as expected.

Screen.Capture.on.2022-04-27.at.15-19-52.mp4

Once issue I noticed was that for non-anchor links, the modal will close prior to Navigation. That might not always be desirable and we can guard against it by testing for an anchor before we add the handlers.

getdave · 2022-04-27T14:36:57Z

packages/block-library/src/navigation/view.js

+ navigationLinks.forEach( function ( link ) {
+ // we need to find the specific parent modal for this link
+ // since .close() won't work without an ID in case we have
+ // mutiple navigation menus in a post/page.
+ const modal = link.closest(


Suggested change

navigationLinks.forEach( function ( link ) {

// we need to find the specific parent modal for this link

// since .close() won't work without an ID in case we have

// mutiple navigation menus in a post/page.

const modal = link.closest(

navigationLinks.forEach( function ( link ) {

// Ignore non-anchor links.

if ( ! link.getAttribute( 'href' )?.startsWith( '#' ) ) {

return;

}

// we need to find the specific parent modal for this link

// since .close() won't work without an ID in case we have

// mutiple navigation menus in a post/page.

const modal = link.closest(

We can ignore links that are no anchors.

This would also need to update the comment

// Close modal on clicking internal and external links inside modal.

I think this makes sense. The other cases I thought about where:

What if the link opens in new tab?

What if the link is slow to load or is broken?

But I think in both of these cases we wouldn't want to close the overlay.

…o an anchor link Closes #gutenberg-39585

…lay menu nav links Closes #gutenberg-39585

… overlay menu is open before trying to close it Closes #gutenberg-39585

…enu overlay for anchor links Closes #gutenberg-39585

…links to seperate view-modal.js script Closes #gutenberg-39585

…closing modal on clicking link Closes #gutenberg-39585

Trapsta · 2022-04-28T15:59:31Z

Great feedback , I can see adding the filter @getdave suggested enhances the UX for non-anchor links quite a bit and I also added a filter for any anchor links that would open in a new tab to cover some the edge cases @scruffian pointed out. Also updated the comment to be more accurate. Thanks everyone for taking your time to test and review.

getdave

This looks good to go. I've also pinged @scruffian and @draganescu for their opinions but I'd say this is a good improvement. Thank you again for your work here 🙇

packages/block-library/src/navigation/view-modal.js

update comment description Co-authored-by: Dave Smith <getdavemail@gmail.com>

fix comment typo Co-authored-by: Dave Smith <getdavemail@gmail.com>

aristath

LGTM! 👍

getdave · 2022-05-03T13:42:08Z

Thank you again @Trapsta for the contribution.

github-actions · 2022-05-03T13:42:31Z

Congratulations on your first merged pull request, @Trapsta! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

adamziel · 2022-06-21T13:32:19Z

I am removing the Backport label here. This PR depends on #36176 which wasn't backported in time for 6.0 and is now planned for 6.1.

coreyworrell · 2022-11-07T22:24:15Z

If this was the fix recently included in 6.1, it does not cover the common case of a link like /#anchor or /some-page/#anchor. It only works if the href exactly starts with #. So if you are on /some-page/ and you click the /some-page/#anchor link, it will scroll down to the anchor, but the modal will remain open.

My current fix:

import MicroModal from 'micromodal'

// Fix navigation block
const links = document.querySelectorAll('.wp-block-navigation-item__content')

for (const link of links) {
    const modal = link.closest('.wp-block-navigation__responsive-container')
    const id = modal?.id || ''

    if (id && link.href.indexOf('#') !== -1 && link.pathname === location.pathname) {
        link.addEventListener('click', () => {
            if (modal.classList.contains('has-modal-open')) {
                MicroModal.close(id)
            }
        })
    }
}

getdave · 2022-11-08T17:35:58Z

If this was the fix recently included in 6.1, it does not cover the common case of a link like /#anchor or /some-page/#anchor. It only works if the href exactly starts with #. So if you are on /some-page/ and you click the /some-page/#anchor link, it will scroll down to the anchor, but the modal will remain open.

My current fix:

import MicroModal from 'micromodal'

// Fix navigation block
const links = document.querySelectorAll('.wp-block-navigation-item__content')

for (const link of links) {
    const modal = link.closest('.wp-block-navigation__responsive-container')
    const id = modal?.id || ''

    if (id && link.href.indexOf('#') !== -1 && link.pathname === location.pathname) {
        link.addEventListener('click', () => {
            if (modal.classList.contains('has-modal-open')) {
                MicroModal.close(id)
            }
        })
    }
}

@draganescu I looks like we'll need a bug fix PR for this.

github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Mar 22, 2022

Trapsta changed the title ~~Fix/gutenberg 39585~~ Close overlay menu when clicking an anchor link Mar 22, 2022

getdave self-requested a review March 23, 2022 11:11

getdave mentioned this pull request Mar 23, 2022

Navigation Project Tracking Issue #38275

Open

38 tasks

Trapsta marked this pull request as ready for review April 13, 2022 10:43

Trapsta requested a review from tellthemachines as a code owner April 13, 2022 10:43

paaljoachim added [Type] Bug An existing feature does not function as intended Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Apr 14, 2022

paaljoachim self-requested a review April 14, 2022 14:37

paaljoachim reviewed Apr 14, 2022

View reviewed changes

getdave reviewed Apr 27, 2022

View reviewed changes

Trapsta added 6 commits April 28, 2022 18:16

fix(block-library): close overlay menu in nav block when navigating t…

9245d64

…o an anchor link Closes #gutenberg-39585

fix(block-library): add eventlistener to close modal on clicking over…

bed0175

…lay menu nav links Closes #gutenberg-39585

fix(block-library): pass id of modal to micromodal.close and check if…

c26932a

… overlay menu is open before trying to close it Closes #gutenberg-39585

fix(block-library): ignore non-anchor links when closing navigation m…

82f0a1b

…enu overlay for anchor links Closes #gutenberg-39585

chore(block-library): move code for closing modal on clicking anchor …

a4d82e0

…links to seperate view-modal.js script Closes #gutenberg-39585

fix(block-library): ignore anchor links which open on a new tab when …

9c4815b

…closing modal on clicking link Closes #gutenberg-39585

getdave requested review from draganescu and scruffian April 29, 2022 08:47

getdave approved these changes Apr 29, 2022

View reviewed changes

packages/block-library/src/navigation/view-modal.js Outdated Show resolved Hide resolved

packages/block-library/src/navigation/view-modal.js Outdated Show resolved Hide resolved

Trapsta and others added 2 commits May 3, 2022 13:09

Update packages/block-library/src/navigation/view-modal.js

faa02c2

update comment description Co-authored-by: Dave Smith <getdavemail@gmail.com>

Update packages/block-library/src/navigation/view-modal.js

d024733

fix comment typo Co-authored-by: Dave Smith <getdavemail@gmail.com>

aristath approved these changes May 3, 2022

View reviewed changes

getdave merged commit cc93e66 into WordPress:trunk May 3, 2022

github-actions bot added this to the Gutenberg 13.2 milestone May 3, 2022

gziolo added Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release and removed Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels May 6, 2022

georgeh added the [Feature] Navigation in Site View Navigation section in the Site Editor when in Site View, offering a way to manage Navigation Menus a label May 11, 2022

adamziel removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Jun 21, 2022

Mamaduka mentioned this pull request Jul 14, 2022

Navigation Block: improve page jump experience in responsive modal #37387

Closed

t-hamano mentioned this pull request Aug 15, 2022

Hamburger-style (overlay) menu items do not work when contains internal #anchor links #43205

Closed

getdave mentioned this pull request Nov 8, 2022

Fix Nav block anchor link closing #45608

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Close overlay menu when clicking an anchor link #39625

Close overlay menu when clicking an anchor link #39625

Trapsta commented Mar 22, 2022 •

edited

Loading

github-actions bot commented Mar 22, 2022

getdave commented Mar 22, 2022

JosVelasco commented Mar 22, 2022

Trapsta commented Mar 23, 2022

getdave commented Mar 23, 2022

Trapsta commented Apr 13, 2022

paaljoachim commented Apr 14, 2022 •

edited

Loading

paaljoachim commented Apr 14, 2022

paaljoachim left a comment •

edited

Loading

JosVelasco commented Apr 14, 2022

getdave left a comment

getdave Apr 27, 2022

draganescu Apr 27, 2022

scruffian Apr 27, 2022 •

edited

Loading

Trapsta commented Apr 28, 2022

getdave left a comment

aristath left a comment

getdave commented May 3, 2022

github-actions bot commented May 3, 2022

adamziel commented Jun 21, 2022 •

edited

Loading

coreyworrell commented Nov 7, 2022 •

edited

Loading

getdave commented Nov 8, 2022

Close overlay menu when clicking an anchor link #39625

Close overlay menu when clicking an anchor link #39625

Conversation

Trapsta commented Mar 22, 2022 • edited Loading

What?

Why?

How?

Testing Instructions

Screenshots or screencast

Before

After

github-actions bot commented Mar 22, 2022

getdave commented Mar 22, 2022

JosVelasco commented Mar 22, 2022

Trapsta commented Mar 23, 2022

getdave commented Mar 23, 2022

Trapsta commented Apr 13, 2022

paaljoachim commented Apr 14, 2022 • edited Loading

paaljoachim commented Apr 14, 2022

paaljoachim left a comment • edited Loading

Choose a reason for hiding this comment

JosVelasco commented Apr 14, 2022

getdave left a comment

Choose a reason for hiding this comment

getdave Apr 27, 2022

Choose a reason for hiding this comment

draganescu Apr 27, 2022

Choose a reason for hiding this comment

scruffian Apr 27, 2022 • edited Loading

Choose a reason for hiding this comment

Trapsta commented Apr 28, 2022

getdave left a comment

Choose a reason for hiding this comment

aristath left a comment

Choose a reason for hiding this comment

getdave commented May 3, 2022

github-actions bot commented May 3, 2022

adamziel commented Jun 21, 2022 • edited Loading

coreyworrell commented Nov 7, 2022 • edited Loading

getdave commented Nov 8, 2022

Trapsta commented Mar 22, 2022 •

edited

Loading

paaljoachim commented Apr 14, 2022 •

edited

Loading

paaljoachim left a comment •

edited

Loading

scruffian Apr 27, 2022 •

edited

Loading

adamziel commented Jun 21, 2022 •

edited

Loading

coreyworrell commented Nov 7, 2022 •

edited

Loading