Use OO.ui.Element.static.getRootScrollableElement when setting/reading scrollTop in VE
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Esanders
	Jan 23 2022, 1:00 AM

Description

We use window but OOUI does this better and has handing for browser bugs. Also using window is not compatible with $.animate.

In ce.surface we cache $window. Most uses of that should probably be changed.

Project	Branch	Lines +/-	Subject
mediawiki/extensions/VisualEditor	master	+6 -6	VE-MW: Consistently use target/surface $scrollContainer to set/get scrollTop
mediawiki/extensions/VisualEditor	master	+1 -1	Update VE core submodule to master (c3f56a0b9)
VisualEditor/VisualEditor	master	+14 -12	Consistently use target/surface $scrollContainer to set/get scrollTop

Change 756179 had a related patch set uploaded (by Esanders; author: Esanders):

[VisualEditor/VisualEditor@master] Consistently use target/surface $scrollContainer to set/get scrollTop

Change 756723 had a related patch set uploaded (by Esanders; author: Esanders):

[mediawiki/extensions/VisualEditor@master] VE-MW: Consistently use target/surface $scrollContainer to set/get scrollTop

Change 756179 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] Consistently use target/surface $scrollContainer to set/get scrollTop

Change 757112 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (c3f56a0b9)

Change 757112 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Update VE core submodule to master (c3f56a0b9)

Change 756723 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] VE-MW: Consistently use target/surface $scrollContainer to set/get scrollTop

I tested a bunch locally, but we're making a lot of changes here and we might have broken something and not noticed.

Things to test: everything where we scroll the browser viewport, or react to the user scrolling, particularly:

copy-pasting shouldn't scroll unexpectedly
toolbar sticking to the top of the viewport when you scroll
opening the keyboard and toolbar dropdowns shouldn't scroll unexpectedly (on mobile)
find-and-replace dialog should scroll to the matches
context menu (when you e.g. focus a large image) should stay visible as you scroll

I checked the things listed in @matmarex 's comment and more. Ran regression for core features while focusing on the user scrolling.

All seems good.

ppelberg closed this task as Resolved.Jan 29 2022, 1:27 AM

ppelberg claimed this task.