Opened 4 years ago
Last modified 10 days ago
#52116 accepted defect (bug)
Twenty Twenty: Menu + Search can cause a scroll jump on close
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.9 | Priority: | normal |
Severity: | normal | Version: | 5.3 |
Component: | Bundled Theme | Keywords: | has-patch has-test-info |
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:
Originally noted in the [support forums https://wordpress.org/support/topic/toggle-close-search-menu-rapidly-scrolls-glitch/].
Attachments (1)
Change History (22)
#1
@
4 years ago
- Focuses javascript added
- Keywords needs-patch added
- Milestone changed from Awaiting Review to Future Release
#2
@
10 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.
#5
@
9 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
@
9 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.
7 months ago
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
7 months ago
#12
@
3 months ago
- Keywords changes-requested added
- Milestone changed from 6.8 to Future Release
I am moving this back to future release since there has been no updates.
This ticket was mentioned in PR #8461 on WordPress/wordpress-develop by @sainathpoojary.
3 months ago
#13
This PR addresses an issue where closing search modal (e.g., mobile menu or search) causes the page to briefly jump to the top and scroll back. The fix ensures smooth scroll restoration by:
- Using
behavior: 'instant'
inscrollTo
for instant position restoration
Trac ticket: #52116
@sainathpoojary commented on PR #8461:
3 months ago
#14
#15
@
3 months ago
Hey @sabernhardt @poena
I've revisited this issue and proposed an updated fix in PR. This addresses the scroll jump when closing modals while retaining accessibility features and admin bar height.
Please have a look whenever you get a moment.
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
2 months ago
#17
@
2 months ago
- Keywords 2nd-opinion changes-requested removed
- Milestone changed from Future Release to 6.9
- Owner changed from williampatton to joedolson
- Status changed from assigned to accepted
- Version set to 5.3
@nikunj8866 commented on PR #8461:
8 weeks ago
#18
Test Report
This report validates that the indicated patch addresses the issue.
Patch tested: https://github.com/WordPress/wordpress-develop/pull/8461
Environment
- OS: Windows
- Web Server: Nginx
- PHP: 8.2.23
- WordPress: 6.7.2
- Browser: Chrome Version 134
- Theme: Twenty Twenty
Actual Results
- ✅ Issue resolved with patch.
Supplemental Artifacts
Before Fix:
https://github.com/user-attachments/assets/ed00e54e-aed6-4974-b2b9-dfc8210bcf42
After Fix:
https://github.com/user-attachments/assets/a441d400-a200-4bf9-ac0c-f72556680d12
This relates to the
scrollTo
function inside a setTimeout, withinhideAndShowModals
.