Make WordPress Core

Opened 3 years ago

Closed 17 months ago

#55342 closed defect (bug) (fixed)

If the contextual help panel is open and then I scroll, I have to click on help twice to close the panel

Reported by: mikecho's profile mikecho Owned by: joedolson's profile joedolson
Milestone: 6.3 Priority: normal
Severity: minor Version: 5.9.1
Component: Help/About Keywords: has-patch needs-testing commit
Focuses: ui, javascript Cc:

Description

Steps to reproduce:

  1. click "Help" to open a contextual help panel
  2. scroll
  3. click "Help" to close the panel; note that it does not close
  4. click "Help" again; note that it does close this time

This is annoying for the user--can this be solved so that it only takes one click to close the contextual help after scrolling?

In our plugin, we are using enough contextual help on one page that you have to scroll to reach the "Help" button (#contextual-help-link") to close it again.

Further notes: I have a live expression for document.activeElement in the console. On the first attempt to close the section, the document.activeElement shows:

button#contextual-help-link.button.show-settings.screen-meta-active

on the second, click it shows:

button#contextual-help-link.button.show-settings

More info:
happens in several tested themes (2022, 2021, 2020)
happens when all plugins are deactivated
Chrome/Edge/Firefox, Windows 10, Ubuntu 20.04


Attachments (1)

contextual-help-panel-is-open-and-then-scroll.gif (895.1 KB) - added by Heiko_Mamerow 18 months ago.

Download all attachments as: .zip

Change History (15)

#1 @sabernhardt
2 years ago

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

Hi and thanks for the report!

This seems related to the scrollIntoView WebKit fix added in r22249.

This ticket was mentioned in PR #4272 on WordPress/wordpress-develop by @studionashvegas.


21 months ago
#2

  • Keywords has-patch added; needs-patch removed

This request fixes a bug in the WordPress Admin screen. Currently, opening the contextual help menu, scrolling, and trying to close the Help menu causes the user to have to click twice to scroll thanks to the behavior of scrollIntoView(). This request changes that function to use scrollIntoViewIfNeeded() which allows the user to successfully close the contextual menu with one click.

Fixes: https://core.trac.wordpress.org/ticket/55342

#3 @studionashvegas
21 months ago

I've submitted the PR above to fix this issue - it's been a while since I've contributed, so if I've forgotten a step in the process please let me know; otherwise, I've verified this works on trunk and does indeed solve the issue.

This ticket was mentioned in Slack in #core by thatmitchcanter. View the logs.


19 months ago

#5 @audrasjb
19 months ago

  • Keywords needs-testing added
  • Milestone changed from Future Release to 6.3

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


18 months ago

#8 @Heiko_Mamerow
18 months ago

Test Report

This report validates that the indicated patch addresses the issue:
https://github.com/WordPress/wordpress-develop/pull/4272.diff

Environment

  • OS: Ubuntu 23.04
  • Web Server: Nginx
  • PHP: 8.1.9
  • WordPress: 6.2.2
  • Browser: Firefox 115
  • Theme: Twenty Ten
  • Active Plugins: none


Actual Results


  • ✅ Issue resolved with patch.


Supplemental Artifacts

It works with the patch - see my video (animated gif) above.

This ticket was mentioned in Slack in #core-test by heiko_mamerow. View the logs.


18 months ago

HeikoMamerow commented on PR #4272:


18 months ago
#10

I tested and it works with the patch.
https://core.trac.wordpress.org/ticket/55342#comment:8

#11 @joedolson
18 months ago

  • Owner set to joedolson
  • Status changed from new to reviewing

#12 @costdev
17 months ago

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/4272.diff

Environment

  • WordPress: 6.3-beta3-56130
  • PHP: 7.4.33
  • Server: Apache/2.4.56 (Ubuntu)
  • Database: mysqli (Server: 5.7.41-0ubuntu0.18.04.1 / Client: mysqlnd 7.4.33)
  • Browser: Chrome, Firefox, Edge and Opera
  • Theme: Twenty Twenty-Three 1.1
  • MU-Plugins: None activated
  • Plugins:
    • WordPress Beta Tester 3.5.0.1

Actual Results

  • ❌ Before the patch: Closing the Help tab requires two clicks. Issue reproduced in all four browsers.
  • ✅ After the patch: Closing the Help tab requires one click. Issue resolved with the patch in all four browsers.

#13 @joedolson
17 months ago

  • Keywords commit added

While this patch does appear to resolve the issue, it depends on a non-standard proprietary method that has not yet been incorporated into W3C standards. Usage of this does seem to be growing based on data and there is relatively recent activity in the standards repositories on it. The non-standard method gives me some pause.

The patch does fix the problem, however, even in Firefox which does not theoretically support this method, so it may be OK. This may throw errors in uncommon browser situations. In my opinion, the issue this fixes is probably more significant than the risk of incompatibility, and hopefully if problems do come out, they'll be found prior to release.

I'm marking this commit, but if anybody else has thoughts about the use of scrollIntoViewIfNeeded(), please voice those thoughts. I'll commit it later this evening if not.

#14 @joedolson
17 months ago

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

In 56243:

Help/About: Fix closing contextual help when scrolled.

Change from scrollIntoView to scrollIntoViewIfNeeded so scrolling will only fire when it is required. scrollIntoViewIfNeeded is a proprietary method that is not standardized and not currently on a track towards standards. However, testing has it working well, including in Firefox, which supposedly does not support it.

Props mikecho, studionashvegas, mai21, piotrek, Heiko_Mamerow, costdev, joedolson.
Fixes #55342.

Note: See TracTickets for help on using tickets.