Page MenuHomePhabricator

[Implementation] Introduce editing functionality to sticky header
Closed, ResolvedPublic

Description

This task involves the work with introducing edit affordances within the desktop site's new sticky header (T283505).


Requirements

Notes: once finalized, this section will end up being a combination of the requirements originally defined in T287545 as well as those that emerged during the Sticky Header Mini Offsite the Editing Team held on 1-Dec-2021.

Meta
  • Platforms: Desktop
  • Editing Interfaces: Visual editor and 2010 Wikitext editor
    • #TODO: do we get support for the 2017 wikitext editor "for free"?
  • People Affected: people who A) Are Logged in and B) Have the Use Legacy Vector setting within Special:Preferences#mw-prefsection-rendering disabled
    • Note: the Web Team will revisit the question of whether the sticky header will be made available to logged out users in Q3FY2021-22.
  • Relationship to settings: Logged in editors who have explicitly set a preference for the Editing mode setting within Special:Preferences#mw-prefsection-editing will notice their preference reflected within the reading mode sticky header by way of the editing affordance(s) they see/do not see.
    • Note: This requirement will likely already have been met by way of T289723.
    • Note: This requirement is expanded upon on in ==== Noticing the edit affordance below.
Noticing the edit affordance
  • In single edit tab contexts, one affordance, that opens the editing interface projects will have defined as their default or individual people will have defined as their preferred interface, must be present within the sticky header.
    • This requirement will likely have been met by way of T291414 and T289723.
  • In contexts where two edits tabs are available, affordances to open the wikitext and visualeditor must be present within the sticky header
    • This requirement will likely have been met by way of T291414 and T289723.
Transitioning from reading into editing

Source + Visual

  • Upon landing in editing mode, people ought to see the section they were just viewing focused in-view while still being able to scroll to any other part of the article.
    • Note: this functionality is already implemented within the Visual editor. Additional work will be required to implement this in the 2010 wikitext editor.

Visual

  • Upon clicking an edit affordance within the sticky header, we should strive to minimize the amount that the viewport/content moves, so that people can maintain their focus on the content they are seeking to change, and ultimately, begin making that change as quickly as possible after the editing interface becomes ready.
    • Note: the approach we implement now will be improved when the new parser is deployed.
  • #TODO: define which section heading we should pick as point we use to anchor the reading and editing experiences
    • Note: the work to define the section anchor point will happen in T296977.

Source

  • #TODO: define which section heading we should pick as point we use to anchor the reading and editing experiences
    • Note: the work to define the section anchor point will happen in T296977.
    • Note: "section heading" in this context refers to all section levels: ==H2==, ===H3===, ====H4====, etc.
Making a change

Source + Visual

  • When the editing interface loads, the sticky header people will see while reading should not be visible so: A) they do not become confused what modality they're in and B) they can continue to access the editing toolbar they will need to make changes.
Transitioning from editing into reading

Abandoning the editing interfaces (source + visual)

  • No changes to the current experience: upon abandoning the editing interfaces (e.g. pressing Escape or clicking the browser's Back button) people should be returned [as closely as possible] to the scroll position they were at in the moment they clicked the Edit affordance within the sticky header. Note: we will investigate improving this experience in T296975.
Publishing a change

Visual

  • No changes to the current experience: upon publishing an edit, people should land back in reading mode at the same position in the article as they were just before they clicked Publish changes. Note: we will investigate improving this experience in T296975.

Source

  • No changes to the current experience: upon publishing an edit, people should land back in reading mode at the top of the section they were in just before they clicked Publish changes. Note: we will investigate improving this experience in T296975.

Prototype

via @Esanders; last updated 21-Dec-2021:
https://patchdemo.wmflabs.org/wikis/557db5804f/w/index.php?title=Special:UserLogin&returnto=Douglas+Adams (login as Patch Demo)

Minimum test case

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

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

[mediawiki/extensions/VisualEditor@master] Pass visibleSection & visibleSectionOffset to target

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

Test wiki created on Patch demo by JKlein (WMF) using patch(es) linked to this task:

https://patchdemo.wmflabs.org/wikis/590e79e5a7/w/

For the prototype, what steps should I take to see/interact with the sticky header experience? @Esanders

Test wiki on Patch demo by JKlein (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/590e79e5a7/w/

In terms of functionality, the latest prototype seems to achieve the functionality that we were aiming for here. It nicely switches to edit mode without losing your position within the article, and places your cursor in a somewhat reasonable location.

The cursor placement seems like it could be improved some more. I still found myself getting lost and not being able to find the cursor many times which could be supported with UI improvements at a later stage.

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

[VisualEditor/VisualEditor@master] Try to select a visible offset when focusing a null-selection surface

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

Change 734885 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] Try to select a visible offset when focusing a null-selection surface

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

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

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

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

Change 752812 merged by jenkins-bot:

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

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

The patch https://gerrit.wikimedia.org/r/734885 broke my local dev environment. I don't need to do much, just start VisualEditor on an existing page that contains a template transclusion.

Uncaught Error: Offset out of bounds
    at VeCeDocumentNode.ve.BranchNode.getNodeFromOffset (load.php?lang=en&modules=ext.visualEditor.core%2Cmwtransclusion&skin=vector&version=16b9t:10:83)
    at VeCeSurface.ve.ce.Surface.findFocusedNode (load.php?lang=en&modules=ext.visualEditor.core%2Cmwtransclusion&skin=vector&version=16b9t:482:77)
    at VeCeSurface.ve.ce.Surface.getFocusedNode (load.php?lang=en&modules=ext.visualEditor.core%2Cmwtransclusion&skin=vector&version=16b9t:481:425)
    at VeCeLinearSelection (load.php?lang=en&modules=ext.visualEditor.core%2Cmwtransclusion&skin=vector&version=16b9t:514:577)
    at OoFactory.OO.Factory.create (<anonymous>:1246:592)
    at Object.ve.ce.Selection.static.newFromModel (load.php?lang=en&modules=ext.visualEditor.core%2Cmwtransclusion&skin=vector&version=16b9t:424:491)
    at VeCeSurface.ve.ce.Surface.getSelection (load.php?lang=en&modules=ext.visualEditor.core%2Cmwtransclusion&skin=vector&version=16b9t:434:42)
    at binarySearch (load.php?lang=en&modules=ext.visualEditor.core%2Cmwtransclusion&skin=vector&version=16b9t:497:903)
    at VeCeSurface.ve.ce.Surface.getViewportRange (load.php?lang=en&modules=ext.visualEditor.core%2Cmwtransclusion&skin=vector&version=16b9t:498:161)
    at VeCeSurface.ve.ce.Surface.selectFirstVisibleContentOffset (load.php?lang=en&modules=ext.visualEditor.core%2Cmwtransclusion&skin=vector&version=16b9t:498:383)

I tried to understand what's happening here, but couldn't figure it out. When I undo the change everything works just fine.

The patch https://gerrit.wikimedia.org/r/734885 broke my local dev environment. I don't need to do much, just start VisualEditor on an existing page that contains a template transclusion.

Can you dump the wikitext of the page you are using somewhere?

We just found a related issue (https://gerrit.wikimedia.org/r/c/VisualEditor/VisualEditor/+/753101) so maybe you can test that patch as well (one line change).

Change 749252 merged by jenkins-bot:

[mediawiki/extensions/VisualEditor@master] Pass visibleSection & visibleSectionOffset to target

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

Other known issues caused by these changes: [copying from a Slack thread in case anyone else runs into them]

@DLynch "It does still leave the cursor position lower than I'd have expected on a local test page, though. (Start of the second paragraph, rather than the first.)"

@matmarex "There is also a problem on mobile that seems related: a part of the editing surface is stuck under the toolbar."

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

[VisualEditor/VisualEditor@master] selectFirstVisibleContentOffset: Only apply line height hack when scrolling

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

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

[VisualEditor/VisualEditor@master] getViewportRange: Handle node not being rendered

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

Can you dump the wikitext of the page you are using somewhere?

Sure. I found the following minimal example that triggers this error:

  • The page I try to edit contains nothing but {{foo}}.
  • That [[Template:foo]] contains nothing but plain text:
A

B

I tried some of the patches linked here (I know they are not necessarily related).

Change 753199 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] selectFirstVisibleContentOffset: Only apply line height hack when scrolling

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

Seems like the issue is that our find-a-valid-offset code in getViewportRange is failing us because this document doesn't contain any content offsets.

Change 753516 had a related patch set uploaded (by DLynch; author: DLynch):

[VisualEditor/VisualEditor@master] getViewportRange: don't error in a no-contentoffsets document

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

I can confirm that with the patch https://gerrit.wikimedia.org/r/753516 in place the error doesn't happen any more.

Change 753516 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] getViewportRange: don't error in a no-contentoffsets document

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

Change 753817 had a related patch set uploaded (by Jforrester; author: Jforrester):

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

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

Change 753817 merged by jenkins-bot:

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

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

Change 753200 merged by jenkins-bot:

[VisualEditor/VisualEditor@master] getViewportRange: Handle node not being rendered

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

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

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

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

Change 754021 merged by jenkins-bot:

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

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

Test Link: https://patchdemo.wmflabs.org/wikis/2dcc2d9bfb/wiki/Douglas_Adams
Using https://photos.app.goo.gl/X6LAceaPV8kZfqdV9 as reference:

  • 00:03: Edit affordance is visible for both wikitext and visualeditor
  • 00:16: Transition from reading to editing works fine for visualeditor
  • 00:28(visual), 1:20(source): The read mode sticky header is not visible
  • 00:42(visual), 1:13(source): No change to the Publish change experience
  • 1:36(esc), 1:41(back button): Transitioning from editing into reading

Also, the cursor shows up at the beginning of the first line as expected. See:

Screenshot 2022-02-20 at 06.27.22.png (762×2 px, 125 KB)

EAkinloose moved this task from Low Priority to Inbox on the Editing QA board.
EAkinloose removed a project: Editing QA.

Everything you described looks good to me save for one question: At ~1:02, I noticed you were able to open the 2010 wikitext editor using the [[ ]] button in the sticky header. You then seemed to scroll the editor some and then at ~1:05 it looks like your cursor jumps back up to the top of the editable area...did the browser do this for you? Did you do this intentionally? Something else?


Meta: the level of thoroughness you provided makes comparing what you experienced to what I would expect you to experience a great deal easier...I appreciate you taking the time to provide it.

@ppelberg , that wasn't intentional but only happens in cases where wikitext editor took longer to load(which might make it difficult to reproduce more than one time). In the video, the toolbar wasn't loaded at 1:03 even though I started interacting with the content in the editor. Towards the end of 1:04, say 1:05; the toolbar loads and the editor takes the user to the top of the content. What I interpreted this as: the editor finished loading at 1:05 and took the user back to what it knows to be the initial state i.e the top of the editable area. I'll let the Engineers come in here :).
Cc: @matmarex , @DLynch, @Esanders

Everything you described looks good to me save for one question: At ~1:02, I noticed you were able to open the 2010 wikitext editor using the [[ ]] button in the sticky header. You then seemed to scroll the editor some and then at ~1:05 it looks like your cursor jumps back up to the top of the editable area...did the browser do this for you? Did you do this intentionally? Something else?


Meta: the level of thoroughness you provided makes comparing what you experienced to what I would expect you to experience a great deal easier...I appreciate you taking the time to provide it.

That is unrelated to the sticky header work. There's a bug report for it here: T135995.

That is unrelated to the sticky header work. There's a bug report for it here: T135995.

Thanks for clarifying that. @ppelberg , the task is all yours :).

Test wiki created on Patch demo by PPelberg (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/611d599815/w/

Test wiki on Patch demo by PPelberg (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/611d599815/w/