WordPress.org

Make WordPress Core

Opened 4 months ago

Last modified 4 days ago

#49025 new defect (bug)

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

Reported by: acosmin Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 5.3.1
Component: Bundled Theme Keywords:
Focuses: ui, javascript Cc:

Description

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();

Change History (4)

#1 @raQai
3 months ago

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)

#2 @raQai
3 months ago

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() returning false.

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;

				this.untoggleModal(target.closest('.cover-modal.active'));

				if ('' !== target.hash && -1 !== targetHref.indexOf(url)) {
					setTimeout(function () {
						document.getElementById(target.hash.substr(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.

Last edited 3 months ago by raQai (previous) (diff)

#3 @bdcstr
6 weeks ago

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));
    },
  1. Hope it helps ;)

Cheers!

Last edited 6 weeks ago by bdcstr (previous) (diff)

#4 @bluewill
4 days ago

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!

Note: See TracTickets for help on using tickets.