Twenty Twenty: Modal menu link with hash should scroll to that section on the page.

[acosmin]

Based on this support ticket: ​https://wordpress.org/support/topic/possible-bug-anchor-links-dont-work-in-js-flow-out-menu/

A possible fix would be to add this code in index.js, after ​line 156:

goToAnchor: function() {
	document.addEventListener( 'click', function( event ) {
		var target = event.target;

		if ( target.closest( '.modal-menu' ) && 'a' === target.tagName.toLowerCase() ) {
			var url, targetHref;
			
			url = location.href.split( '#' )[ 0 ];
			targetHref = target.href;
			
			this.untoggleModal( target.closest( '.cover-modal.active' ) );

			if( '' !== target.hash && -1 !== targetHref.indexOf( url ) ) {
				setTimeout( function() {
					var fakeEl = document.createElement( 'a' );

					fakeEl.href = targetHref;
					fakeEl.click();
				}, 550 );
			}
		}
	}.bind( this ) );
},

And after ​line 105, a call to the method: this.goToAnchor();

[anonymized_14254218]

This fix does not work reliably. Sometimes it works, sometimes it doesn't. Currently I do not have reproducible cases for working and non working behavior. Will update once I figured it out.

See ​https://foos.bogdan-iws.de/angebot/

"Menu" > "Angebot" > "Kicker Jugend" uses the anchor /angebot/#jugend
"Menu" > "Angebot" > "Für Unternehmen" uses the anchor /angebot/#unternehmen

Chromium 79.0.3945.88 (Official Build) Arch Linux (64-bit)

[anonymized_14254218]

I found the issue. It was related to the Menu Image (​https://de.wordpress.org/plugins/menu-image/) plugin since it creates span elements within the a link elements of the menu if images are present.

This would then trigger the click event on the span instead of the a resulting in 'a' !== target.tagName.toLowerCase().

my temporary solution looks like this:

	goToAnchor: function () {
		document.addEventListener('click', function (event) {
			var target = event.target.closest('a');

			if (target && target.closest('.modal-menu') && 'a' === target.tagName.toLowerCase()) {
				var url, targetHref;

				url = location.href.split('#')[0];
				targetHref = target.href;

				if ('' !== target.hash && -1 !== targetHref.indexOf(url)) {
					this.untoggleModal(target.closest('.cover-modal.active'));
					setTimeout(() => document.getElementById(target.hash.slice(1)).scrollIntoView(), 700);
				}
			}
		}.bind(this));
	},

Edit1
After further testing (especially on mobile devices) I figured the timeout of 550 ms is not enough for some browser/device combinations to wait for the menu to be closed. Changing this value to 700ms solved this issue on my test phones.

I also prefer #scrollIntoView instead of creating a fake element using #createElement.

Edit2
Now only untoggles the modal menu if this is a valid anchor, otherwise simply follows the link.

[bdcstr]

Noticed the same issue. This is my temporary-and-not-perfect solution (so don't judge me!) while waiting for the official patch.

1. Go to themes/twentytwenty/assets/js/index.js
2. Look for the outsideUntoggle: function () { (at line 135 for me)
3. Change with the following code:

    outsideUntoggle: function () {
        document.addEventListener('click', function (event) {
            var target = event.target;
            var modal = document.querySelector('.cover-modal.active');

            if (event.target.tagName.toLowerCase() === 'a' && event.target.hash.includes('#')) {
                this.untoggleModal(modal);
                setTimeout(function () {
                    var anchor = document.getElementById(event.target.hash.slice(1)); // 
                    anchor.scrollIntoView();
                }, 550);
            }

            if (target === modal) {
                this.untoggleModal(target);
            }
        }.bind(this));
    },

4. Hope it helps ;)

Cheers!

[bluewill]

There is still a problem in mobile platforms when using hash anchors from the nav menu.

The thing seem to be a conflict with another function that scrolls the page to where the user was before opening the nav menu.

Comenting "_win.scrollTo( 0, Math.abs( _win.twentytwenty.scrolled + getAdminBarHeight() ) );" this line after having implemented any of the previous methods will do the work resolving the main bug, but you loose the cool page scrolling down when you close the menu feature. Don't have time to look forward how to compatibilize the two :(

Also: This is my first comment so: Is this the way to inform this kind of thing? Is this usefull at all?

Cheers!

[anonymized_14254218]

Replying to bdcstr:

Noticed the same issue. This is my temporary-and-not-perfect solution (so don't judge me!) while waiting for the official patch.

1. Go to themes/twentytwenty/assets/js/index.js
2. Look for the outsideUntoggle: function () { (at line 135 for me)
3. Change with the following code:

    outsideUntoggle: function () {
        document.addEventListener('click', function (event) {
            var target = event.target;
            var modal = document.querySelector('.cover-modal.active');

            if (event.target.tagName.toLowerCase() === 'a' && event.target.hash.includes('#')) {
                this.untoggleModal(modal);
                setTimeout(function () {
                    var anchor = document.getElementById(event.target.hash.slice(1)); // 
                    anchor.scrollIntoView();
                }, 550);
            }

            if (target === modal) {
                this.untoggleModal(target);
            }
        }.bind(this));
    },

4. Hope it helps ;)

Cheers!

This would by default use any anchor link on the current site.
Using a menu link /path/to/page1#anchor would also scroll to #anchor on /current/open/page if it is available.

[anonymized_14254218]

Okay, I made some rather big changes now because some things simply did not work as intended.

1. as @bluewill already pointed out, the scrolling back to the previous location after toggeling the modal menu messes around with some things that might be intended.

Why is this an issue?
e.g. use a sticky header by applying a fixed property to it and every time you close the navigation again, you will first be put at the top and scrolled to the previous location after.

Why does this happen?
Because at the current state, the toggle will force a position: fixed on the html element to allow scrolling within the modal navigation without changing location on the current page. I also see this as an issue because it will move the view port to the top of the page which will look weird when using a fixed header navigation.

How to fix this?
Instead of changing the position attribute of the page, we can simply apply overflow:hidden. That's how the overlays on Pinterest work for example. For now, this can simply be achieved using either js or simply by using the already by the JavaScript applied class

body.showing-modal {
    overflow: hidden;
}

2. And I know this might be a corner case, but let's say you have multiple pages setup which contain anchors. Using a plain JavaScript based solution, you will heavily have to rely on the current window location to use the same format as the href of the link.

Why is this an issue?
Let's say you have page bar setup with a parent page foo. The actual url would be <host>/foo/bar/. A valid anchor within the navigation menu item would be bar#baz. Hover if bar does not only exists once this would lead to possibly false negative matches when checking the anchor link based on the setup of the page.

Why does this happen?
Well, because WordPress handles such URLs gracefully in the background, which is nice.

How to fix this?
We will need check whether the menu item url corresponds to an existing page and simplify the url for the JavaScript. I did this using the wp_nav_setup_menu_item filter to avoid messing around with the core classes.

function anchor_nav_menu_items($menu){
  // contains # and does not start with it
  if (strpos($menu->url,'#')) {
    // check if this is a relative url
    // FIXME this condition needs some additional work because it relys on using the host properly
    //       www.domain.com/foo/bar would probably not match https://domain.com/foo/bar
    if (strpos(get_site_url(), $menu->url) === false) {
      $menu_site_url = get_site_url(null, $menu->url);
      $menu_site_id = url_to_postid($menu_site_url);
      // check if the current site is referenced in the menu item
      if ($menu_site_id === get_the_id()) {
        $menu_site_url_explode = explode('#', $menu->url);
        $menu->url = '#' . end($menu_site_url_explode);
      }
    }
  }
  return $menu;
}

add_filter('wp_setup_nav_menu_item', 'anchor_nav_menu_items', 1);

3. Using the above changes, some functions in the JavaScript can be simplified/removed (see attached index.js.diff and index.js)

I would create a PR for this, but I do not know where to actually put the php code since I am only used to hooks and filters, not the WordPress core libraries.

You can see a working example on ​https://foos.bogdan-iws.de/
Relevant Navigation: "Angebot" -> all sub elements are anchors.

This is certainly not a final solution, but imo it goes into the right direction. I put fixmes on every line which would need further investigation.

[anonymized_14254218]

changes of the index.js to allow anchor navigation

[anonymized_14254218]

edited index.js to allow anchor navigation

[lichttag]

The fix by @raQai works like a charm for me! When will it be merged?