Maniphest T200927

Enable smooth/animated scrolling between links on pages
Open, LowPublic
Actions

Description

Background

The recent design for moved paragraphs in mobile diffs (T197491) called for an animated scroll when clicking from one instance of the paragraph in question to the other. The purpose of this animation is to help the user stay oriented in terms of where the paragraph got moved to.

We initially thought this behavior would benefit Minerva in general (e.g. when using the table of contents to navigate to a page section it would be awesome to animate the scroll), but after trying it out decided to scope this change just to the mobile diffs view for now.

Developer notes

https://phabricator.wikimedia.org/T200927#4590778 provides a simple lightweight solution to this.

Let's try it out.

If it doesn't have the desired effect we should not spend time investigating alternatives. Instead we should put it back in the backlog and re-evaluate and estimate.

Note it's important that scroll-behaviour applies to a scrollable element, otherwise it won't work as we found out when we previously attempted this task.

QA instructions

*Note that this feature is not intended to work on iOS

Environment: staging
Browser & device: Android/Chrome
Skin: MFE
Steps:

visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Special:MobileDiff/388527#movedpara_1_1_lhs
scroll down to the bottom of the first yellow paragraph
tap the downwards-pointing yellow triangle
the view should scroll smoothly to the top of the second yellow paragraph
click the upwards-pointing yellow triangle (at the top of the second yellow paragraph)
the view should scroll smoothly to the top of the first yellow paragraph

Related Objects
Search...

		Status	Assigned	Task
		Open	None	T200927 Enable smooth/animated scrolling between links on pages
		Invalid	Edtadros	T214082 test case for enabling smooth scrolling

Event Timeline

alexhollender created this task.Aug 1 2018, 6:04 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 1 2018, 6:04 PM

alexhollender edited projects, added Readers-Web-Backlog (Design); removed Readers-Web-Backlog.Aug 1 2018, 6:05 PM

alexhollender moved this task from Needs analysis to Next up on the Readers-Web-Backlog (Design) board.Sep 17 2018, 3:18 PM

Restricted Application changed the subtype of this task from "Deadline" to "Task". · View Herald TranscriptSep 17 2018, 3:18 PM

We could do smooth scrolling with one line of CSS:

scroll-behavior: smooth;

If we're OK with it working only in Firefox, Chrome, and Chrome for Android (right now).

https://caniuse.com/#feat=css-scroll-behavior
https://developer.mozilla.org/en-US/docs/Web/CSS/scroll-behavior
https://css-tricks.com/almanac/properties/s/scroll-behavior/

...but it's one line!

Sounds great to me : )

@Jdlrobson how do you feel about Jan's proposed solution? T200927#4590778

Jdlrobson edited projects, added Readers-Web-Backlog; removed Readers-Web-Backlog (Design).Sep 24 2018, 8:24 PM

Jdlrobson updated the task description. (Show Details)

Jdlrobson moved this task from Incoming to Triaged but Future on the Readers-Web-Backlog board.

Jdlrobson added projects: good first bug, Google-Code-in-2018.Sep 24 2018, 10:41 PM

ovasileva set the point value for this task to 1.Sep 26 2018, 4:32 PM

ovasileva moved this task from Triaged but Future to Upcoming on the Readers-Web-Backlog board.Oct 2 2018, 9:10 PM

ovasileva edited projects, added Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2); removed Readers-Web-Backlog.Oct 2 2018, 9:13 PM

Which code base is this task about? MobileFrontend?

Aklapper moved this task from Proposed tasks to Information needed on the Google-Code-in-2018 board.Oct 7 2018, 2:31 AM

Jdlrobson removed a project: Google-Code-in-2018.Oct 8 2018, 5:56 PM

Change 480443 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Enable smooth scrolling on in-page links for Chrome and Firefox

https://gerrit.wikimedia.org/r/480443

gerritbot added a project: Patch-For-Review.Dec 18 2018, 9:06 AM

Jdrewniak moved this task from To Do to Needs Design Review on the Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Dec 18 2018, 9:06 AM

@alexhollender I put the patch up on staging. It should work in Chrome and Firefox.

Unfortunately this CSS property doesn't allow us to tweak the timing at all :/

http://readers-web-staging.wmflabs.org/w/index.php?title=South_Africa#Name

Jdrewniak assigned this task to alexhollender.Dec 18 2018, 1:35 PM

Due to the slow speed of the scroll the affect is a bit frustrating on long articles. Additionally, if you've used the TOC to navigate to multiple sections, then use the back button, it results in slightly confusing behavior. My apologies for not anticipating this earlier.
Here is a screen recording

For now can we restrict the implementation to anchor links within diff views, which was the initial place we realized we needed this feature?

alexhollender reassigned this task from alexhollender to Jdrewniak.Dec 18 2018, 10:08 PM

alexhollender moved this task from Needs Design Review to Needs More Work on the Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.

Change 481156 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/extensions/MobileFrontend@master] Enable smooth scrolling on mobile diff page for Chrome and Firefox

https://gerrit.wikimedia.org/r/481156

Change 480443 abandoned by Jdrewniak:
Enable smooth scrolling on in-page links for Chrome and Firefox

Reason:
This has been superseded by https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/ /481156/

https://gerrit.wikimedia.org/r/480443

Jdlrobson moved this task from Needs More Work to Needs Code Review on the Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Dec 21 2018, 5:15 PM

Jdlrobson moved this task from Needs Code Review to Ready for Signoff on the Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2) board.Dec 21 2018, 5:32 PM

Change 481156 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Enable smooth scrolling on mobile diff page for Chrome and Firefox

https://gerrit.wikimedia.org/r/481156

ReleaseTaggerBot added a project: MW-1.33-notes (1.33.0-wmf.12; 2019-01-08).Dec 21 2018, 6:00 PM

Alex should sign this off on his return using http://reading-web-staging.wmflabs.org/wiki/Special:MobileDiff/9?useformat=mobile a

It looks like this introduced a regression in the automated browser tests (N.B. the failures started on 22nd December as they were run 2 hours prior to T200927#4840811): https://integration.wikimedia.org/ci/job/selenium-MobileFrontend/

I take it back. Correlation doesn't imply causation and all that…

Edit

The regression appears to have been caused by rMWa67a4b10871a: Add missing, dynamically declared properties in ContribsPager. This is tracked in T212771: [Bug] Special:Contributions broken when the Flow extension is loaded: "Cannot access private property ContribsPager::$tagFilter".

MBinder_WMF moved this task from Readers-Web-Kanbanana-Board-2018-19-Q2 to Readers-Web-Kanbanana-Board-2018-19-Q3 on the Readers-Web-Backlog board.Jan 2 2019, 5:31 PM

MBinder_WMF edited projects, added Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q3); removed Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q2).

MBinder_WMF moved this task from To Do to Ready for Signoff on the Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q3) board.Jan 2 2019, 5:34 PM

Android / Chrome: the scroll effect is working, however I have to press the up/down arrow twice in order to trigger the scroll.

iOS / Chrome, Safari: the scroll effect is not working. When I press the up/down arrow the page seems to reload at the appropriate location.

Curious if others get the same results? @ABorbaWMF @Ryasmeen @ovasileva

ovasileva moved this task from Ready for Signoff to Needs QA on the Readers-Web-Backlog (Readers-Web-Kanbanana-Board-2018-19-Q3) board.Jan 7 2019, 5:14 PM

ovasileva reassigned this task from alexhollender to ABorbaWMF.Jan 7 2019, 6:08 PM

Jdlrobson added a project: MobileFrontend.Jan 9 2019, 6:06 PM

OS / Chrome, Safari: the scroll effect is not working. When I press the up/down arrow the page seems to reload at the appropriate location.

To quote Jan "If we're OK with it working only in Firefox, Chrome, and Chrome for Android (right now)." T200927#4590778 - this will not work on iOS.

ah, thank you for the reminder @Jdlrobson : )

I've added QA instructions to the task. Also, I just tried this again on Android/Chrome, however the scroll is not animating as it was before.

ovasileva claimed this task.Jan 16 2019, 6:08 PM

Tested on Chrome 71.0 in production and I cannot see smooth scrolling (just jumps for me).

ovasileva added a project: Product-QA.Jan 17 2019, 8:37 PM

ovasileva closed subtask T214082: test case for enabling smooth scrolling as Invalid.Jan 17 2019, 8:41 PM

Jdlrobson updated the task description. (Show Details)Jan 17 2019, 8:52 PM

Confirmed. also cannot see this working. Let's pull this back into needs analysis and work out what went wrong?

Jdlrobson moved this task from Upcoming to Needs Prioritization on the Readers-Web-Backlog board.Jan 17 2019, 9:27 PM

So it seems to work when I put the scroll-behavior: smooth; rule on the <html> element instead of the <body>... kinda.
It only starts scrolling after the second tap. I have no idea why that is. I haven't found anything intercepting the click events (yet?).

Possibly unrelated, I did notice the anchor tags have sightly malformed HTML

<a class="mw-diff-movedpara-left data-title-tag="old" href="#movedpara_3_10_rhs">▼</a> - unclosed class attribute there. The only place I see that modified is here in core.

Test page

Oh, when I saw an anchor tag with href="#movedpara_3_10_rhs" I assumed there was an element with that same ID on the page. Nope.

There are instead anchor tags with a name attribute that matches the href. Wow! I guess that was a thing... in 1993.
https://eager.io/blog/the-history-of-the-url-path-fragment-query-auth/#fragments

I think if we change the name attribute to an id attribute it might work better (fingers crossed).

<a class="mw-diff-movedpara-left data-title-tag="old" href="#movedpara_3_10_rhs">▼</a> - unclosed class attribute there. The only place I see that modified is here in core.

Opened T214435 for the malformed HTML issue

I think if we change the name attribute to an id attribute it might work better (fingers crossed).

names and ids are interchangeable. This is valid HTML.

So it seems to work when I put the scroll-behavior: smooth; rule on the <html> element instead of the <body>... kinda.

I also see this and it makes sense as the scroll-behaviour CSS property only applies to scrolling boxes and body is not scrollable.

This works for example:

#mw-mf-minidiff {
  height: 200px; overflow: scroll; scroll-behavior: smooth;
}

Next steps: Let's revert the patchset that's been merged since it's a no-op re-discuss this (do we want to add the class to HTML via JS for example?) and re-estimate.

Jdlrobson removed ovasileva as the assignee of this task.Jan 22 2019, 8:55 PM

Jdlrobson updated the task description. (Show Details)

Jdlrobson lowered the priority of this task from Normal to Low.Wed, Jul 31, 6:56 PM

Jdlrobson moved this task from Triaged but Future to Needs Prioritization on the Readers-Web-Backlog board.

Enable smooth/animated scrolling between links on pagesOpen, LowPublicActions