Timebox: 5 days

Will the popup pre-save be shown before or after the pre-save edit checks?

after chatting with @nayoub and @bmartinezcalvo we suggest to display the pre-save dialog at the very last stage, so after pre-save edit checks. the goal is to have a stronger separation of concerns between the editing activity (and addressing editing-related checks) and publishing (either with a temp account or via a permanent account).

In T411880#11464331, @AAlhazwani-WMF wrote:

Will the popup pre-save be shown before or after the pre-save edit checks?

after chatting with @nayoub and @bmartinezcalvo we suggest to display the pre-save dialog at the very last stage, so after pre-save edit checks. the goal is to have a stronger separation of concerns between the editing activity (and addressing editing-related checks) and publishing (either with a temp account or via a permanent account).

Could someone clarify or give examples of what checks are we talking about here. One thing that adds complexity on this decision is that my guess is the ideal UX here entails also to preserve the edit summary if any to make use of it at the time of submitting the editing without having to re-prompt the user. Can you confirm this @AAlhazwani-WMF ?

An unrelated/related nitpick in the designs is that I'm missing a "success screen" at the end of the workflow. I'm assuming we'd want to land in the article page, read mode, with some success toast. And also things need to look as if you were logged in.

Could someone clarify or give examples of what checks are we talking about here.

Currently you'd see various potential edit checks. If you added sufficient uncited content, you'll see the reference check. If you're in the right a/b test and you added questionably-toned content, you'll see tone check. If you're in the right a/b test and you pasted some content (and didn't answer our immediate questions), you'll see paste check. More to come.

An unrelated/related nitpick in the designs is that I'm missing a "success screen" at the end of the workflow. I'm assuming we'd want to land in the article page, read mode, with some success toast. And also things need to look as if you were logged in.

There's already a success toast after you save, so absent a specified change I'd assume this is still what you'd see:

logged in
logged out

In T411880#11476335, @Sgs wrote:

In T411880#11464331, @AAlhazwani-WMF wrote:

Will the popup pre-save be shown before or after the pre-save edit checks?

after chatting with @nayoub and @bmartinezcalvo we suggest to display the pre-save dialog at the very last stage, so after pre-save edit checks. the goal is to have a stronger separation of concerns between the editing activity (and addressing editing-related checks) and publishing (either with a temp account or via a permanent account).

Could someone clarify or give examples of what checks are we talking about here.

here's an example

when editors click the first publish button (blue background, right-pointing arrow), they're prompted with edit checks if there are issues in the article. the publish button becomes inactive until they resolve the edit check(s). once resolved, the button is active again - and clicking it takes them to the pre-save dialog. is this helpful?

One thing that adds complexity on this decision is that my guess is the ideal UX here entails also to preserve the edit summary if any to make use of it at the time of submitting the editing without having to re-prompt the user. Can you confirm this @AAlhazwani-WMF ?

in our proposal the edit summary is the very last step (after account creation/log in), so i don't think we need to preserve the summary because it wasn't entered, or am i missing something here?

An unrelated/related nitpick in the designs is that I'm missing a "success screen" at the end of the workflow. I'm assuming we'd want to land in the article page, read mode, with some success toast. And also things need to look as if you were logged in.

yeah correct, what happens after publishing is exactly as in production: article in read mode and success message.

a-ha, thanks @DLynch! i didn't refresh the page hence didn't see your answer!

looking at the other questions under the task description..

Which editors are we targeting here exactly? VE, and the VE source editor? The 2017 Wikitext editor? The 2010 Wikitext Editor?

given that we're focusing on mobile i'd target only the editors fully supported on mobile.. so VE, and the VE source editor.

another thing that i don't see in the task description that might be worth mentioning is that if someone is already a temp account, we shouldn't display the logged-out reminder - given that there's already the temp account banner present, nor the pre-save dialog (thou it might be an interesting idea to be explored in the future).

when editors click the first publish button (blue background, right-pointing arrow), they're prompted with edit checks if there are issues in the article. the publish button becomes inactive until they resolve the edit check(s). once resolved, the button is active again - and clicking it takes them to the pre-save dialog. is this helpful?

Just to note, this isn't *necessarily* the edit check flow. You only have to click "publish" again if you were kicked back to editing the article. Right now that will only happen with tone check, because you have to go actually edit some text. Adding a reference can be done entirely through the edit check tools, and so won't send you back like that.

Change #1220625 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):

[mediawiki/extensions/MobileFrontend@master] [DNM] VisualEditorOverlay: experimental post edit anon warning

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

Change #1220627 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):

[mediawiki/extensions/GrowthExperiments@master] [DNM] PoC: new logged out warning for VE on mobile

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

Test wiki created on Patch demo by SGimeno (WMF) using patch(es) linked to this task:
https://64beace24e.catalyst.wmcloud.org/w/

I tried a PoC for the VE flow to get familiar with the problem space and even with the autosave functionality working as expected there are some areas to work out. The main changes I did:

1. Stop showing the current anon warning on the editor overlay (VisualEditorOverlay: experimental post edit anon warning). This is pretty straightforward, although I wonder, who/what component should be responsible of this warning as it's currently coming from different places in desktop and mobile.
2. Add a dedicated LoggedOutMobileArticleTarget which intercepts the showSaveDialog call to show the new anon warning dialog.
3. Add a dialog for the three options, I made it with Vue/codex but I'm not sure it is the best choice. On the one hand this would allow to use the same dialog for visual and source modes and use the latest MW front-end standard, on the other hand the communication between Vue and vanilla JS or VE is cumbersome. I had to use mw.hook events although I know there should be some more elegant ways.
4. On the dialog Special:UserLogin and Special:CreateAccount urls I appended a query parameter geloggedoutguidance=1 as a way to flag the initiated flow to be able to display the publishing form on top of the recovered changes. This is not working as expected, for some reason I can't yet tell VE is not calling the addPlugin callbacks right after login. The idea is that the custom article target would override the initAutosave method with a version that recovers the changes and shows the dialog right away.
5. For the create account flow I'm observing instances of the issue T397193 in patchdemo, although I think it may be a no-scenario since the CA/SUL3 auth flow is not enabled there.

I would appreciate directions from @DLynch specially on points (2) and the issue with registering an article target (4). I will continue the investigation for the source mode in the following days and post updates.

I wanted to reverse engineer the "mid edit" workflow on desktop with VE to take inspiration from and I'm observing different issues while performing it in testwiki and another prod wikis like enwiki. In testwiki I'm observing again the regression from T397193 which was apparently solved, so we may be experiencing a different instance of issue. In enwiki the WS is not shown and the user is properly redirected to the editing page, however the changes are not recovered into the editor.

2. Making an entire different Target wouldn't be the pattern we'd normally use here. The existing targets already do plenty of "is this an anon?" checking inside themselves. What you probably actually want to do is have something add to the target's preSaveProcess, which would let you show the warning dialog there as part of the showSaveDialog flow. (You'd need to affirmatively decide how this is supposed to be interacting with the pre-save edit checks, which also use that process.)

3. Particularly if you're wanting to drop this into the middle of the VE save flow, an OOUI dialog is going to be significantly easier to integrate.

In T411880#11508206, @DLynch wrote:

2. Making an entire different Target wouldn't be the pattern we'd normally use here. The existing targets already do plenty of "is this an anon?" checking inside themselves. What you probably actually want to do is have something add to the target's preSaveProcess, which would let you show the warning dialog there as part of the showSaveDialog flow. (You'd need to affirmatively decide how this is supposed to be interacting with the pre-save edit checks, which also use that process.)

Do you mean something similar to the pattern in editcheck/controller.js?

3. Particularly if you're wanting to drop this into the middle of the VE save flow, an OOUI dialog is going to be significantly easier to integrate.

Agreed, I will refactor the dialog to be OOUI.

Do you mean something similar to the pattern in editcheck/controller.js?

Yes -- that's doing fundamentally the same thing as you want to, by inserting itself into the space between "the user presses the save button" and "the save dialog is shown".

Mobile VE visual mode

I was able to put together a PoC (thanks to DLynch's directions) in 64beace24e. It's a MVP that only works articles without sections, if the edit is in an article section the recovering after authenticating will fail. It needs quite some polishing but the approach seems doable. Some of the learnings and areas to work further are:

• Manually testing this flow is very time consuming, building some Cypress test for it could be convenient. It could be used for easing local development and also as a secondary CI. It could be used to test against a matrix of auth setups. Also because currently it is not possible to test GE in conjunction with CA.
• The PoC changes the ownership of the Logged out warning on mobile from MobileFronted to GrowthExperiments, while on desktop the warning continues to appear in the VE toolbar. Not sure which component is responsible for it. It would be good if the logged out warning has a clear place to sit and owner.
• In the PoC, after the user logs in or creates an account and it is redirected to the article, there's a deliberate delay of 800ms between the article target creation and showing the edit summary dialog to let the autosave toast "Changes recovered" show. This needs a more refined approach, we need to decide if we want to show the toast at all or skip it, and that may require some changes in VE's autosave cc @DLynch . A more challenging optimization would be to avoid having to load the full editor to recover the changes and show the summary.

Mobile VE source mode

I haven't explored in depth David's suggestions in the description but from a UX pov the autosave approach seems the most consistent with the visual experience. I'm happy to explore this option with some extra time but building the autosave seems the most time consuming from it. We should also keep in mind that the experience described in this task is not the same as for desktop, in neither of the modes. It would be interesting to find the common parts, if any, of both experiences in order to scatter the less the logic across components.

I have noticed that the login dialog that's shown on desktop has a height that cuts off the bottom button and forces the person to scroll. You may have also noticed this when you were testing locally and chosen to fix it later as well since this is a PoC.

The PoC was and this task in general was scoped to mobile platform. I didn't check any desktop flow. The PoC does not check for the platform when loading the module that makes the intervention. I can be easily fixed and it's a fair reminder when we want to turn the PoC into production ready code. Specifically I used HelpPanelHooks#onBeforePageDisplay because these already load in the main article namespace, but it was a makeshift shortcut just for the PoC. Making the logged out warning depend on the help panel is probably not a good idea and this module should be loaded separatedly in the real implementation. cc @DMburugu

@AAlhazwani-WMF did you have a chance to try the patchdemo? Your feedback is higly welcome. I had to adapt a bit the UI because we're using OOUI dialogs and building one from scratch was time consuming.