Page MenuHomePhabricator

Add a link: post-edit dialog
Closed, ResolvedPublic

Description

After the user publishes (when they have chosen "yes" at least once) or submitted (when they have not chosen "yes" at least once), they go to the post-edit dialog state. This is almost exactly the same as the post-edit dialog we built for the "guidance" effort in T245790: Newcomer tasks: post-edit dialog, with a few changes.

  • The suggested task should be another "add a link" task from that user's selected topics of interest.
  • If the user had not chosen "yes" at least once:
    • The banner above the dialog should read: "Thanks for reviewing suggestions. Keep going!"
    • Instead of "Edit another suggestion:", the header in the body should read: "Next suggestion:"
    • The bottom button on the dialog, which reads, "Close and edit this article again", should just say "Close".
  • If the user dismisses the dialog, they should be in read mode.
Mobile mockups as of 2021-01-12:
image.png (678×772 px, 158 KB)
Desktop mockups as of 2021-01-12:
image.png (530×842 px, 218 KB)

NOTE: Refer to Figma for up-to-date detailed redline mocks and specs:
Mobile: https://www.figma.com/file/2SONd8P1tsexIB5coMOp8h/Growth-Structured-tasks?node-id=181%3A65
Desktop: https://www.figma.com/file/2SONd8P1tsexIB5coMOp8h/Growth-Structured-tasks?node-id=112%3A0

Event Timeline

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

For the Desktop the following spec is not done:

  • If the user had not chosen "yes" at least once:
    • The banner above the dialog should read: "Thanks for reviewing suggestions. Keep going!"
    • Instead of "Edit another suggestion:", the header in the body should read: "Next suggestion:"
    • The bottom button on the dialog, which reads, "Close and edit this article again", should not be present at all. There should only be one button: "View more suggested edits".

For @RHo review (other items that the spec above) - Desktop only (mobile is still WIP)

The mockup post-edit dialog and the existing post-edit dialog have several UI differences - should the existing dialog be changed to the mockup dialog?

  • "Next steps" title is centerd vs the mock-up left-aligned position
  • the buttons labels are different on the mockups
  • "Vew more suggested edits" and "Close to view..." labels are displayed as blue links in the mockup
  • there is no reload icon on the implemented post-edit dialog

Desktop post-edit dialog

mockup dialogexisting dialog
Screen Shot 2021-04-29 at 12.53.01 PM.png (366×539 px, 60 KB)
Screen Shot 2021-04-29 at 12.12.24 PM.png (904×1 px, 254 KB)

@Etonkovidova, I think @Tgr is still working on this one. (@Tgr if so please move back to in progress)

Change 683755 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/GrowthExperiments@master] Add Link: changes to post-edit dialog

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

Yeah, this wasn't implemented yet (it's just fairly similar to the existing feature). Also I think the design changes are not part of this task, the mocks include other tasks like T272664: Add a link: refresh button on post-edit dialog (nice to have).

The bottom button on the dialog, which reads, "Close and edit this article again", should not be present at all. There should only be one button: "View more suggested edits".

I'm assuming this refers to link recommendation tasks in general, not just the case where the user did not make an edit - it would be weird to offer a close option in one case but not in other. @MMiller_WMF let me know if I misunderstood.

Change 683755 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Add Link: changes to post-edit dialog

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

@RHo do you want the trailing colon in Next suggestion: and Edit another suggestion:? Figma doesn't show it, but the phab description includes the colon.

Yeah, this wasn't implemented yet (it's just fairly similar to the existing feature). Also I think the design changes are not part of this task, the mocks include other tasks like T272664: Add a link: refresh button on post-edit dialog (nice to have).

The bottom button on the dialog, which reads, "Close and edit this article again", should not be present at all. There should only be one button: "View more suggested edits".

I'm assuming this refers to link recommendation tasks in general, not just the case where the user did not make an edit - it would be weird to offer a close option in one case but not in other. @MMiller_WMF let me know if I misunderstood.

My understanding is that "Close and edit this article again" button should appear for all task types. The only time it shouldn't be in the footer is if the user skipped all suggestion or said no to suggestions but didn't make an edit to the text.

@Tgr @kostajh @RHo -- thank you for questioning this. I'm thinking this through again. As @kostajh says, the original requirements are, in fact, to not have that "close and edit this article again" button if the user did not actually edit the article by choosing "yes" on any suggestions. And the reason we made that requirement is because we didn't want to say "edit this article again", because they hadn't edited it at all yet. But perhaps removing the button was not the right solution to that.

Here's what I think: let's have the button in both cases, but in the case that no edit was saved, just have different copy: "Close". That should be easier on our business rules.

How does that sound? If good, I'll update the task description.

In terms of code complexity, it doesn't make much difference, all the options are trivial to implement. But I think it would feel confusing to me as a user if the "close" option would sometimes appear and sometimes not; whether the user accepted any recommendations does not have much to do with whether they might want to interact more with the article. (And there are other ways than the button to close the dialog, so removing the button doesn't remove the complexity of how to handle if the user clicks "Edit" again.)

Always having the buttons is the most consistent option so it sounds good to me.

Change 684041 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/GrowthExperiments@master] Fix postedit dialog button logic

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

Change 684041 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Fix postedit dialog button logic

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

@RHo (@MMiller_WMF) The dialog with 'Close' button doesn't appear on mobile at all - did I miss that the spec for mobile was different?
For review

  • the font color for buttons is different on Desktop and mobile -it's different from the mockup
  • I agree with @Tgr comment:

In terms of code complexity, it doesn't make much difference, all the options are trivial to implement. But I think it would feel confusing to me as a user if the "close" option would sometimes appear and sometimes not; whether the user accepted any recommendations does not have much to do with whether they might want to interact more with the article. (And there are other ways than the button to close the dialog, so removing the button doesn't remove the complexity of how to handle if the user clicks "Edit" again.)

Always having the buttons is the most consistent option so it sounds good to me.

The way it works now:

AnswersPost-edit dialog
Desktopall skipped
Screen Shot 2021-05-18 at 3.13.16 PM.png (1×1 px, 199 KB)
'yes'
Screen Shot 2021-05-18 at 6.22.00 PM.png (1×1 px, 360 KB)
'no'
Screen Shot 2021-05-18 at 12.41.18 PM.png (1×2 px, 701 KB)
AnswersPost-edit dialog
Mobileall skipped the Post-edit dialog is not displayed
'yes'
Screen Shot 2021-05-18 at 6.46.19 PM.png (1×756 px, 147 KB)
'no'
Screen Shot 2021-05-18 at 6.47.28 PM.png (1×726 px, 134 KB)

Thanks @Etonkovidova and @Tgr. I agree about having the Close button for both cases (whether links were added or not), but a few very minor UI fixes based on your review and screenshots below, and this is ready to close:

  • Level and Robot icon in the dialog is larger than the expected 16x16dp
  • Pageviews should be removed from the next suggested article card.

@RHo (@MMiller_WMF) The dialog with 'Close' button doesn't appear on mobile at all - did I miss that the spec for mobile was different?
For review

  • the font color for buttons is different on Desktop and mobile -it's different from the mockup
  • I am guess it's cos mobile uses a more custom drawer element, whilst the Desktop uses a standard dialog with neutral buttton colour, so I'm OK to keep the different colours until we have a more standardised drawer component (and have updated the Desktop spec accordingly)
  • I agree with @Tgr comment:

In terms of code complexity, it doesn't make much difference, all the options are trivial to implement. But I think it would feel confusing to me as a user if the "close" option would sometimes appear and sometimes not; whether the user accepted any recommendations does not have much to do with whether they might want to interact more with the article. (And there are other ways than the button to close the dialog, so removing the button doesn't remove the complexity of how to handle if the user clicks "Edit" again.)

Always having the buttons is the most consistent option so it sounds good to me.

The way it works now:

AnswersPost-edit dialog
Desktopall skipped
Screen Shot 2021-05-18 at 3.13.16 PM.png (1×1 px, 199 KB)
'yes'
Screen Shot 2021-05-18 at 6.22.00 PM.png (1×1 px, 360 KB)
'no'
Screen Shot 2021-05-18 at 12.41.18 PM.png (1×2 px, 701 KB)
AnswersPost-edit dialog
Mobileall skipped the Post-edit dialog is not displayed
'yes'
Screen Shot 2021-05-18 at 6.46.19 PM.png (1×756 px, 147 KB)
'no'
Screen Shot 2021-05-18 at 6.47.28 PM.png (1×726 px, 134 KB)

@Tgr @Etonkovidova -- if the user skips all suggestions on mobile, there should be a post-edit dialog. Let's add that.

And we are all agreed about the changes for the "close" button -- it should always be there, just with different wording if the user has or has not skipped all suggestions. The task description is up-to-date with this specification.

  • I am guess it's cos mobile uses a more custom drawer element, whilst the Desktop uses a standard dialog with neutral buttton colour, so I'm OK to keep the different colours until we have a more standardised drawer component (and have updated the Desktop spec accordingly)

We specifically override the button color to be progressive on mobile and blackish on desktop, IIRC that was part of the design in T245790: Newcomer tasks: post-edit dialog (to keep with how other buttons look on desktop/mobile, I imagine?). Easy to change if preferred.

@Tgr @Etonkovidova -- if the user skips all suggestions on mobile, there should be a post-edit dialog. Let's add that.

As we since discussed elsewhere, that's T282546: AddLink: Skipped all dialog throws exception on ve.init.target.tryTeardown.then on mobile.

And we are all agreed about the changes for the "close" button -- it should always be there, just with different wording if the user has or has not skipped all suggestions. The task description is up-to-date with this specification.

So it seems like on mobile a no-change save is not recognized as such. (We should check whether we also get that wrong in the instrumentation or just in the dialog config. The dialog is set up very differently on mobile since it reloads the page after an edit, so it's more likely the latter.)

So what's left in this task is that, plus these:

  • Level and Robot icon in the dialog is larger than the expected 16x16dp
  • Pageviews should be removed from the next suggested article card.

This task contains lingering issues from "add a link" that can be fixed as a snack.

Change 720280 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/MobileFrontend@master] postEditMobile hook: Pass newRevId

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

Change 720281 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] postEditMobile: Set task state depending on newRevId value

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

Change 720290 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] SmallTaskCard: Apply icon sizing rules to post edit dialog

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

Pageviews should be removed from the next suggested article card.

@RHo / @MMiller_WMF Is that just for add link type tasks, or any task? The original task for the post-edit dialog (T245790: Newcomer tasks: post-edit dialog) appears to have us adding page view data for desktop, but not for mobile.

Change 720290 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] SmallTaskCard: Apply icon sizing rules to post edit dialog

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

Change 720280 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] postEditMobile hook: Pass newRevId

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

Change 720281 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] postEditMobile: Set task state depending on newRevId value

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

Pageviews should be removed from the next suggested article card.

@RHo / @MMiller_WMF Is that just for add link type tasks, or any task? The original task for the post-edit dialog (T245790: Newcomer tasks: post-edit dialog) appears to have us adding page view data for desktop, but not for mobile.

Thanks for noticing @kostjh. We should update for all tasks to remove the page view data from the Desktop and Mobile post edit dialog.

Change 722328 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] PostEditDialog: Don't show pageview data

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

Pageviews should be removed from the next suggested article card.

@RHo / @MMiller_WMF Is that just for add link type tasks, or any task? The original task for the post-edit dialog (T245790: Newcomer tasks: post-edit dialog) appears to have us adding page view data for desktop, but not for mobile.

Thanks for noticing @kostjh. We should update for all tasks to remove the page view data from the Desktop and Mobile post edit dialog.

Got it, thanks. Updated in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GrowthExperiments/+/722328

Change 722328 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] PostEditDialog: Don't show pageview data

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

Checked in wmf.1

  • the all skipped option on mobile will have the post-edit dialog

Screenshot_20210927-175616_Samsung Internet.jpg (2×1 px, 352 KB)

  • pageviews are removed
  • the icons size is fixed

The task description is updated - the are removed to reflect that the specs are done.

RHo moved this task from QA to Ready for Development on the Growth-Team (Current Sprint) board.
RHo added a subscriber: mewoph.

Hi @mewoph and @Etonkovidova - apologies for seeing this late but it looks like the icon size is still larger than expected.

image.png (636×1 px, 178 KB)

It may be that there needs to be a width:16px; set along with the min-width and height?

I've also noticed that there is no 1px Base70 (#C8CCD1) to delineate the buttons. Can this also be provided for this task? Thanks and apologies again for being late to review!

Checked in wmf.1

  • the all skipped option on mobile will have the post-edit dialog

Screenshot_20210927-175616_Samsung Internet.jpg (2×1 px, 352 KB)

  • pageviews are removed
  • the icons size is fixed

The task description is updated - the are removed to reflect that the specs are done.

Hi @RHo — I just realized that the current Figma files don't have the robot icon in the post-edit dialog. Can you include them?

I've also noticed that there is no 1px Base70 (#C8CCD1) to delineate the buttons

Can you point to this in the spec? I don't know what this is referring to.

Hi @RHo — I just realized that the current Figma files don't have the robot icon in the post-edit dialog. Can you include them?

Sorry about that, I've updated both Desktop and Mobile now with the icon.

I've also noticed that there is no 1px Base70 (#C8CCD1) to delineate the buttons

Can you point to this in the spec? I don't know what this is referring to.

Sure, I mean this:

image.png (1×1 px, 275 KB)

Change 724849 had a related patch set uploaded (by MewOphaswongse; author: MewOphaswongse):

[mediawiki/extensions/GrowthExperiments@master] Post-edit dialog: Explicitly set icon size & show button border on both platforms

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

I see, thanks for the specs @RHo. Here are the updated screens:

Desktop

Screen Shot 2021-09-29 at 2.14.03 PM.png (802×1 px, 184 KB)

Mobile

Screen Shot 2021-09-29 at 2.12.40 PM.png (812×814 px, 221 KB)

I see, thanks for the specs @RHo. Here are the updated screens:

Desktop

Screen Shot 2021-09-29 at 2.14.03 PM.png (802×1 px, 184 KB)

Mobile

Screen Shot 2021-09-29 at 2.12.40 PM.png (812×814 px, 221 KB)

Thanks @mewoph – that looks better. One final tweak though - can the task name be vertically centered with the icons? It's currently a couple pixels higher than expected:

image.png (378×2 px, 355 KB)

Hi @RHo, updated

Mobile post-edit dialog

Screen Shot 2021-09-30 at 8.59.24 AM.png (748×808 px, 174 KB)

Mobile preview

Screen Shot 2021-09-30 at 8.58.46 AM.png (210×700 px, 83 KB)

Desktop post-edit dialog

Screen Shot 2021-09-30 at 9.01.10 AM.png (826×1 px, 240 KB)

Change 724849 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Post-edit dialog: Explicitly set icon size & show button border on both platforms

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

Hi @RHo, updated

Mobile post-edit dialog

Screen Shot 2021-09-30 at 8.59.24 AM.png (748×808 px, 174 KB)

Mobile preview

Screen Shot 2021-09-30 at 8.58.46 AM.png (210×700 px, 83 KB)

Desktop post-edit dialog

Screen Shot 2021-09-30 at 9.01.10 AM.png (826×1 px, 240 KB)

*chef's kiss* – thanks @mewoph!

@mewoph - the mobile looks perfectly aligned (just as in the screenshot in the above comment):

Screen Shot 2021-10-08 at 1.43.34 PM.png (960×872 px, 97 KB)

For Desktop - the improvement much less noticeable on FF 93:

FF 93 testwiki wmf.3Chrome 94 testwiki wmf.3
Screen Shot 2021-10-08 at 1.44.58 PM.png (1×1 px, 280 KB)
Screen Shot 2021-10-08 at 1.52.56 PM.png (1×1 px, 269 KB)

@mewoph - the mobile looks perfectly aligned (just as in the screenshot in the above comment):

Screen Shot 2021-10-08 at 1.43.34 PM.png (960×872 px, 97 KB)

For Desktop - the improvement much less noticeable on FF 93:

FF 93 testwiki wmf.3Chrome 94 testwiki wmf.3
Screen Shot 2021-10-08 at 1.44.58 PM.png (1×1 px, 280 KB)
Screen Shot 2021-10-08 at 1.52.56 PM.png (1×1 px, 269 KB)

Hi there, yes I agree and I've noticed that there is a different display issue in the new screenshots @Etonkovidova posted. The article title and wikidata description text block should be top aligned to the thumbnail, not centre aligned.

Expected (RHS shows expectation if there is no wikidata description):

image.png (402×1 px, 164 KB)

@mewoph - is this something that needs a separate task?

Hi @RHo this looks like a regression from the change to vertically align the task name :( Moving back to in progress to fix

Actually, I think this was an existing issue since the changes to the task name only concerns the metadata.

@RHo To confirm, in the current actual implementation, even when there is a description, the title and the description are not aligned to the top of the container. This should be fixed as well correct?

Actually, I think this was an existing issue since the changes to the task name only concerns the metadata.

@RHo To confirm, in the current actual implementation, even when there is a description, the title and the description are not aligned to the top of the container. This should be fixed as well correct?

Yes please!

Desktop w/description:

Screen Shot 2021-10-12 at 1.43.29 PM.png (328×990 px, 110 KB)

Desktop w/o description:

Screen Shot 2021-10-12 at 1.42.30 PM.png (330×988 px, 34 KB)

Mobile w/description:

Screen Shot 2021-10-12 at 1.40.20 PM.png (318×748 px, 84 KB)

Mobile w/o description:

Screen Shot 2021-10-12 at 1.39.29 PM.png (318×748 px, 34 KB)

Change 730341 had a related patch set uploaded (by MewOphaswongse; author: MewOphaswongse):

[mediawiki/extensions/GrowthExperiments@master] SmallTaskCard: align title to the top

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

Change 730341 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] SmallTaskCard: align title to the top

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

Checked on testwiki wmf.5 - all looks as on the screenshots in @mewoph's comment above.

There is a tiny issue with the letter descender displayed as cut off. It noticeable on this screenshot (from the previous comment) for y letter:

Screen Shot 2021-10-12 at 1.39.29 PM.png (318×748 px, 34 KB)

Another example:

Screen Shot 2021-10-19 at 4.14.08 PM.png (624×850 px, 84 KB)

The issue might be more noticeable on languages with scripts that have many descenders - I filed it as a separate, minor task T293845: [minor] Post-edit dialog - descenders displayed cut off that can be triaged according to the current team priorities.

The line-height on mobile is set to 1, which will cut off extenders.

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

https://patchdemo.wmflabs.org/wikis/8d2a32a63e/w/