Encourage progress on the Add links task on Desktop by turning on the same auto-advance behaviour as currently on mobile.
Description
Details
Subject | Repo | Branch | Lines +/- | |
---|---|---|---|---|
Add a link: Enable auto-advance for desktop | mediawiki/extensions/GrowthExperiments | master | +23 -21 |
Status | Subtype | Assigned | Task | ||
---|---|---|---|---|---|
Open | None | T276517 [EPIC] Growth: "add a link" structured task 3.0 | |||
Resolved | • mewoph | T283548 Add a link: encourage progress on desktop |
Event Timeline
Sure, my preference is to have Desktop auto-advance as per Mobile, but one of the reasons we decided to not auto-advance on desktop was partly because the additional animation we would like to scroll smoothly down to the next item and show the link inspector card added complexity. Maybe @mewoph and/or @kostajh can comment on how much work this would be?
Currently the user needs to click on the "Next" button, and then gets the same experience they would get during auto-advance. If jumpy scrolling is acceptable with manual advance (for some value of acceptable, since we intend to fix it eventually), shouldn't it be the same for auto-advance?
I don't agree that it is the same, because when the user manually advances by pressing next, the jumpiness is understood to be related to the action they took. If we autoscroll jankily, the user is more likely to be confused about whether they hit the wrong button or accidentally scrolled away to explain the jolty motion.
Auto-advance on desktop should be fairly trivial since we already have the logic in place for mobile. Unfortunately, I think adjusting the scrolling will not be trivial. Currently the scrolling is done via VE's scrollSelectionIntoView (VE constructs the position & scroll config for the current selection, and then call OOUI's scrollIntoView method). animate: false is passed — from ve.ui.Surface.prototype.scrollSelectionIntoView, this config is set when the selection is a native cursor selection.
Some options:
- Change how we are selecting the annotation so that it's not a native cursor selection
- Override ve.ui.Surface.prototype.scrollSelectionIntoView to animate the scroll for native cursor selection
- Make changes upstream to allow the caller to specify whether the scroll in response to a native cursor selection should be animated
- Use a custom scroll method instead of built-in one
This should be un-blocked once T284260: Add a link: desktop animations is merged — with these changes, we are no longer relying on VE's scrolling since the scrolling needs to occur before VE selection changes in order to achieve the interaction shown here: https://www.figma.com/proto/2SONd8P1tsexIB5coMOp8h/Growth-Structured-tasks?node-id=710%3A464&viewport=-3460%2C-3365%2C0.45993807911872864&scaling=min-zoom
@RHo where are we on this, now that we have smooth scrolling on desktop? This task is in Need Design, but it doesn't seem to (anymore).
Thanks @Tgr - I've updated the description since as you note the dependency of smooth scrolling is done. Also moved to Ready for dev since there is no design req'd but adding the auto-advance behaviour already in place in mobile.
Change 721870 had a related patch set uploaded (by MewOphaswongse; author: MewOphaswongse):
[mediawiki/extensions/GrowthExperiments@master] Add a link: Enable auto-advance for desktop
Change 721870 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Add a link: Enable auto-advance for desktop
@mewoph - (unrelated to the task scope) It seems that on betlabs the post-edit dialog, mw-ge-small-task-card-metadata-container attempts to load info for mw-ge-small-task-card-pageviews skeleton
testwiki doesn't provide pageview info in the post-edit dialog:
Let me know if a separate bug report should be filed.
For @RHo review
- no issues found
- in addition to @mewoph comments above, here another gif;
(click to view the animated gif)
If after Design review, there wouldn't be follow-ups, you may move it to Test in Production.
Filed as T291510: [betalabs]Post-edit dialog: mw-ge-small-task-card-metadata-container attempts to load pageviews info . Seems to be only a beatalabs issue.