Make WordPress Core

Opened 4 years ago

Last modified 12 days ago

#52116 assigned defect (bug)

Twenty Twenty: Menu + Search can cause a scroll jump on close

Reported by: kjellr's profile kjellr Owned by: williampatton's profile williampatton
Milestone: 6.8 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch needs-testing 2nd-opinion
Focuses: accessibility, javascript Cc:

Description

If the Twenty Twenty Menu or Search modals are activated while the user is partially scrolled down the page, they will cause a glitchy reaction upon close: the page quickly scrolls to the top, and then back down to the original position. This is visible in Chrome or Firefox, but doesn't seem visible in Safari.

Here are some GIFs to illustrate the issue:

https://cldup.com/82Y1UCd6mV.gif

https://cldup.com/Oarqq6OQzP.gif

Originally noted in the [support forums https://wordpress.org/support/topic/toggle-close-search-menu-rapidly-scrolls-glitch/].

Attachments (1)

52116.diff (3.8 KB) - added by nikunj8866 3 months ago.
Attached patch to fix scrol jump

Download all attachments as: .zip

Change History (12)

#1 @sabernhardt
3 years ago

  • Focuses javascript added
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

This relates to the scrollTo function inside a setTimeout, within hideAndShowModals.

#2 @karmatosed
3 months ago

I can confirm this is still an issue in testing so would be good to look at if we do want to resolve.

@nikunj8866
3 months ago

Attached patch to fix scrol jump

#3 @nikunj8866
3 months ago

  • Keywords has-patch needs-testing needs-unit-tests added; needs-patch removed

#4 @nikunj8866
3 months ago

@karmatosed I've attached patch to fix this issue.

#5 @karmatosed
3 months ago

  • Keywords 2nd-opinion added

Thank you @nikunj8866. Let's see about getting some testing and also some eyes on this.

@sabernhardt I'd love to get your opinion on this patch as you've been involved in this ticket before if you have time.

#6 @sabernhardt
3 months ago

  • Focuses accessibility added
  • Keywords needs-unit-tests removed

This is JavaScript, so I need someone else's opinion too :)

The patch removes a lot, and at least some of it should remain.

  • The clickedEl lines were added to avoid focus loss, and those should be kept.
  • The modal needs to account for the admin toolbar height when users are logged in. With a new theme, subtracting --wp-admin--admin-bar--height could achieve that, but child themes can replace the stylesheet without replacing the JS.

The comment above the event listener says

Hide the modal after a delay, so animations have time to play out.

The jumping problem with in-page menu links seems to occur only when animations are turned off (prefers-reduced-motion: reduce). If so, the script could check for that media query and then either set the timeout to 0 or possibly even skip the scrollTo effect.

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


3 weeks ago

#8 @joedolson
3 weeks ago

  • Milestone changed from Future Release to 6.8

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


12 days ago

#10 @joedolson
12 days ago

  • Owner set to williampatton
  • Status changed from new to assigned

#11 @joedolson
12 days ago

Assigning to @williampatton to look over.

Note: See TracTickets for help on using tickets.