Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#41850 closed defect (bug) (fixed)

Customize: Prevent panel sticky headers when scrolling if description is expanded

Reported by: westonruter's profile westonruter Owned by: delawski's profile delawski
Milestone: 4.9 Priority: high
Severity: normal Version: 4.7
Component: Customize Keywords: has-patch
Focuses: Cc:

Description (last modified by westonruter)

When a section's description text is expandable and is included in the panel header area, it then becomes part of the area that is sticky when the panel is scrolled (per #34343). However, when the description is expanded and the user scrolls back up it can cause the header to jump unexpectedly:

See demonstration of issue:

The issue is more pronounced when the description text is longer, as will be the case in 4.9 for Custom CSS:

And from @melchoyce in duplicate #41879:

When I was testing the Additional CSS panel in trunk, I noticed that clicking into the textarea moves my whole screen down, which I found surprising and kind of alarming. Then, when I try to scroll back up to read, it snaps back up instead of naturally scrolling, which felt even more alarming. We should not make any sort of scroll/movement assumptions here.


Attachments (1)

41850_2.patch (4.7 KB) - added by delawski 7 years ago.

Download all attachments as: .zip

Change History (11)

#1 @westonruter
7 years ago

#41879 was marked as a duplicate.

#2 @westonruter
7 years ago

  • Description modified (diff)

#3 @westonruter
7 years ago

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

Assigning this to @delawski since he thinks he can take a look.

If anyone else wants to take a look before him… be our guest. 🎶

#4 @westonruter
7 years ago

In 41384:

Code Editor: Scroll the cursor line into view instead of the entire editor when focused.

See #41850.
Fixes #41879.

#5 @westonruter
7 years ago

In 41385:

Code Editor: Remove scrollIntoView of editor's cursor line upon focus to prevent jumping issues for editor scroll position.

See #41850, #41879.

#6 @westonruter
7 years ago

I think a problem is that the height of the header isn't re-calculated when the section description is expanded.

#7 @westonruter
7 years ago

@delawski Another ticket that is directly impacted by this is the addition of notifications to panels and sections. See #38794. There is some JS on there you can throw into the console to add notifications to panels and sections for testing.

#8 @westonruter
7 years ago

  • Priority changed from normal to high

Bumping priority to high for visibility and alignment with 4.9 goals, and given proximity to beta 1 deadline.

7 years ago

#9 @delawski
7 years ago

  • Keywords has-patch added; needs-patch removed

@westonruter you were right about the cause of the issue. The header height was calculated only once when the container was first expanded. However, any time the help block was expanded, the actual height changed. Height value stored in the data attribute became invalid in such cases.

Calculating the header height inside scroll event handler would cause performance issues.

That's why a custom event is now issued each time a help block gets expanded or collapsed. This event triggers header height recalculation, so that an expensive outerHeight() function is called only when it's really needed.

Attached patch file

I'm also providing a link to the PR on GitHub:

#10 @westonruter
7 years ago

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

In 41679:

Customize: Fix jumping behavior upon scrolling up when a sticky header has its description expanded.

Props delawski.
See #34343, #41879.
Fixes #41850.

Note: See TracTickets for help on using tickets.