Make WordPress Core

Opened 2 years ago

Closed 4 months ago

Last modified 4 months ago

#55143 closed defect (bug) (fixed)

Twenty Twenty One: focus position jumps when opening popup

Reported by: paaljoachim's profile paaljoachim Owned by:
Milestone: 6.2 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: reporter-feedback
Focuses: javascript Cc:

Description (last modified by joedolson)

The following was noticed in this My Calendar Github issue:
https://github.com/joedolson/my-calendar/issues/374

Twenty Twenty One contains code that intercepts all anchor links containing a '#' and moves scroll position to that link. This causes an undesirable focus position movement for any link containing a '#' character that is not intended to move focus.

/**
		 * Close menu and scroll to anchor when an anchor link is clicked.
		 * Adapted from Twenty Twenty.
		 *
		 * @since Twenty Twenty-One 1.1
		 */
		document.addEventListener( 'click', function( event ) {
			// If target onclick is <a> with # within the href attribute
			if ( event.target.hash && event.target.hash.includes( '#' ) ) {
				wrapper.classList.remove( id + '-navigation-open', 'lock-scrolling' );
				twentytwentyoneToggleAriaExpanded( mobileButton );
				// Wait 550 and scroll to the anchor.
				setTimeout(function () {
					var anchor = document.getElementById(event.target.hash.slice(1));
					anchor.scrollIntoView();
				}, 550);
			}
		} );

This code adds a listener to all links that scrolls to that target anchor. For any case where a link is used to open a modal, an accordion, or as a faux-button in any way while containing a hash character, this will cause problems.

Change History (11)

#1 @sabernhardt
2 years ago

  • Component changed from Themes to Bundled Theme

#2 @poena
2 years ago

  • Keywords reporter-feedback added

Hi @paaljoachim & @joedolson, would you be able to add the steps to reproduce this issue?
Thank you.

#3 @poena
2 years ago

Is this the same issue as reported in https://core.trac.wordpress.org/ticket/53331 ?

#4 @joedolson
20 months ago

  • Description modified (diff)

I don't think so, although it's related.

#5 @joedolson
20 months ago

  • Description modified (diff)

#6 @joedolson
20 months ago

Reproduction steps:

  • Activate TwentyTwentyOne
  • Set up a primary menu. (This code is in primary-navigation.js, which is only enqueued with a menu.)
  • Create a link with an anchor, e.g. <a id="test-target" href="#target">Target</a> and a corresponding target, e.g. <div id="target"></div>
  • Add scripting to prevent the default event on the target, e.g.
var testTarget = document.getElementById( 'test-target' );
testTarget.addEventListener( 'click', function(event) {
    event.preventDefault();
});

Observe that although the event is attached, the page still scrolls to the target.

#7 @poena
20 months ago

Does still happen with PR #2833 applied?

Version 0, edited 20 months ago by poena (next)

#8 @poena
4 months ago

  • Keywords needs-testing-info added

Hi @joedolson @paaljoachim Is this still an issue?

Last edited 4 months ago by poena (previous) (diff)

#9 @poena
4 months ago

  • Keywords needs-testing-info removed

#10 @joedolson
4 months ago

  • Resolution set to fixed
  • Status changed from new to closed

No, this appears to no longer be an issue. Thanks for following up!

#11 @swissspidy
4 months ago

  • Milestone changed from Awaiting Review to 6.2
Note: See TracTickets for help on using tickets.