Make WordPress Core

Opened 8 years ago

Last modified 4 years ago

#40862 assigned enhancement

Partially visible controls within a pane do not scroll into view

Reported by: desrosj's profile desrosj Owned by: jpurdy647's profile jpurdy647
Milestone: Future Release Priority: low
Severity: normal Version:
Component: Customize Keywords: has-screenshots good-first-bug has-patch needs-refresh needs-testing
Focuses: Cc:

Description (last modified by SergeyBiryukov)

If the widget pane has many widgets, sometimes the widget dialogue will not be visible when the edit shortcut icon is clicked on the page. If any part of the control is visible, it does not fully scroll into view even though the focus is properly set.

A few notes:

  • If the widget is not in view at all wihtin the pane, it properly scrolls it into view and focuses.
  • The problem also occurs if the pane is not currently visible when the edit shortcut is clicked.

Clicked edit shortcut on a widget in view in the sidebar:
https://cldup.com/EPHX4omF81.png

Clicked edit shortcut on a widget with part of the header visible in the pane:
https://cldup.com/gOsa9rovWG.png

Attachments (4)

scrollToOpenedWidgets.diff (1.9 KB) - added by jpurdy647 8 years ago.
Patch to scroll clipped widgets into view
scrollToOpenedWidgets-2.diff (1.5 KB) - added by jpurdy647 8 years ago.
Patch to scroll clipped widgets into view - revised
40862.patch (541 bytes) - added by m1tk00 8 years ago.
Scroll to opened widget
widgets.js.patch (1.8 KB) - added by jpurdy647 7 years ago.
Patch to scroll opened widget settings into view

Download all attachments as: .zip

Change History (13)

#1 @westonruter
8 years ago

  • Milestone changed from Awaiting Review to Future Release

The issue is that the widget's toggle is a focusable element, and so it can be focused without the rest of the widget control being shown. In fact, the rest of the widget control may not be fully expanded by the time the control is focused. Maybe when a control is focused, and it has an expanded state (such as widgets and nav menu items), when it finishes expanding it should also call scrollIntoView if needed (or even scrollIntoViewIfNeeded).

@jpurdy647
8 years ago

Patch to scroll clipped widgets into view

@jpurdy647
8 years ago

Patch to scroll clipped widgets into view - revised

#2 @jpurdy647
8 years ago

  • Keywords has-patch added; needs-patch removed

Widgets now scroll into view when opened. It will only scroll down if the widget is clipped on the bottom of the screen. Can't think of any way a widget could be clipped on the top of the screen. I can add that if there is some situation scrolling up to make a widget visible would be necessary.

@m1tk00
8 years ago

Scroll to opened widget

#3 @jpurdy647
8 years ago

The problem with scrollIntoView() is that it jumps the element to the top of the screen immediately, interrupting the user experience.

My approach is to check whether the widget will be clipped on the bottom (or be too close to the bottom of the screen) and if so smoothly scroll down while the widget is sliding open.

#4 @DrewAPicture
8 years ago

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

Assigning ownership to mark the good-first-bug as "claimed".

#5 @desrosj
7 years ago

  • Keywords needs-refresh added

I tested 40862.patch, but that approach did not seem to fix the issue for me.

@jpurdy647 It seems that there are issues with your newest patch file. I was unable to apply it to trunk. scrollToOpenedWidgets-2.diff appears to be missing Index information. It also looks like the surrounding code in the patch has changed. Are you able to recreate and refresh the patch?

In case you need some information on creating patches, check out https://make.wordpress.org/core/handbook/tutorials/working-with-patches/.

@jpurdy647
7 years ago

Patch to scroll opened widget settings into view

#6 @jpurdy647
7 years ago

Here you go, updated to reflect more recent changes in that file.

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


4 years ago

#8 @SergeyBiryukov
4 years ago

  • Description modified (diff)

#9 @SergeyBiryukov
4 years ago

  • Keywords needs-testing added

Thanks for the patch! Marking the ticket for testing.

Looking at widgets.js.patch, the number 130 seems somewhat arbitrary, but there are also some other numbers like 250 or 30 in the existing code. It would be great to document where they come from for future reference.

Note: See TracTickets for help on using tickets.