-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix scroll jitter in Customize Widgets #32479
Conversation
Size Change: +7 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
45941f6
to
15c079f
Compare
Tried giving this a test today, but I'm struggling to reproduce it. I've noticed sometimes the customizer doesn't enable the sticky header area, and I don't know what determines that, but today |
Thanks for testing. How odd! I tested today with gutenberg.run and saw the same as you, the lack of any sticky related classes being applied when in the widgets editor. Then I tried the latest trunk on my local setup and found it still has this issue. Turns out that the theme somehow has something to do with it. Using Twenty Twenty in either gutenberg.run or locally the issue is present. All the other themes I tried aren't sticking the header. 🤔 I don't think that means it's an issue for Twenty Twenty because the sticky header is expected. It was apparently the reason for these overrides/#32140. I've not figured out why this can vary between themes but I still think this PR is FTW. It also fixes one other tiny thing that's present no matter the theme:
Should be 12px .
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I managed to reproduce the problem and this PR, in fact, fixes it. The test failures are likely unrelated – I bet everything will work if you rebase.
15c079f
to
2adc1cd
Compare
Thanks Adam! This is rebased and now the only failure is the same android e2e that seems broken in trunk. |
This PR is purely customizer screen scss, so can't be affecting react native. Given that I'll merge the PR. Thanks for fixing this @stokesman! |
Follow up of #32140 which has left the editor with some scroll jitter. No separate issue is made so including reproduction steps here for confirmation of the issue.
UPDATE: So far I can only reproduce this with the Twenty Twenty theme active. It's not clear why other themes seem to prevent the sticky behavior of the Customizer header only in the widgets editor.
Steps to reproduce
Screen recording
The video first shows how the jump can be spotted when returning to the top of the scroll. A bit later, it is shown how the jitter may be triggered.
customize-widgets-scroll-glitch.mp4
About the apparent cause and changes in the PR
The negative
margin-top
of the sticky header is cleared during upward scrolling as part of the customizer’s js-controlled “sticky” positioning. Overriding that to keep the margin constantly applied avoids the issue.How has this been tested?
In the customizer trying and failing to recreate the problems shown in the screen recording.
Types of changes
Bug fix
Checklist:
*.native.js
files for terms that need renaming or removal).