Page MenuHomePhabricator

Add a link: link inspector
Closed, ResolvedPublic

Description

When the user is in the AI suggestions mode from T269638: Add a link: Suggestions mode, the link inspector should always be open. The link inspector is the place where the action happens of accepting, rejecting, skipping, or modifying link suggestions. There are some differences in the link inspector between desktop and mobile.

  • Location
    • On mobile, the link inspector is a card at the bottom of the screen.
    • On desktop, it is a popup over the editing surface that points to the link suggestion.
  • Header
    • It starts with a bot icon.
    • Next is an indication of which suggestion the user is on, e.g. "1/7" or "2/10".
    • Next is a series of bubbles visualizing which suggestion the user is on. If they are on suggestion 2/7, there should be 7 bubbles, and the first two should be filled in. These bubbles are not clickable/tappable.
    • On mobile, the far right of the header has a "?" icon. This opens the help panel. See T269654.
  • Content
    • On mobile, the context starts with the light gray sub-header "Text". Below it is the word or phrase that is the link label of the suggestion in bold Desktop does not have this part, because the word or phrase is pointed at by the link inspector.
    • Next is "Should this link to the following article?" in light gray.
    • Below that is a little article preview, with image, title, and article extract. The title should be a blue link that opens the article in a new tab. This should all be a fixed height, with the article title flowing into ellipses if it doesn't fit on one line, and the extract flowing into ellipses if it doesn't fit on two lines.
  • Actions
    • At the bottom of the card are the action buttons. On mobile, we should be careful not to put them so low that tapping on them triggers the browser's toolbar to pop up.
    • Left arrow: this is always present, but disabled on the first suggestion.
    • Yes: has a checkmark and the word "Yes".
    • No: has an X and the word "No". This button triggers the rejection reason dialog. See T269647.
    • Skip Next: this is one button that includes both the word "Skip" "Next" and a right arrow. (see T269647#6940826)
      • It advances to the next suggestion.
      • On the last suggestion, tapping it is the same as tapping "Publish", bringing the user to the edit summary.
    • There is a mock in the spec showing the desired overflow behavior in cases where the translations for "Yes", "No" and "Next" are long. In this case, the card should become taller, the "Yes" and "No" buttons should be in the same place, the left and right arrow buttons should be on the next line down.
    • Auto-advance
      • On mobile, after the user taps "Yes", they should be advanced to the next suggestion. If they tap "Yes" on the final suggestion, they should be advanced to the edit summary as if they had clicked to publish. The same concept applies if they tap "No", but is discussed in the task for the rejection dialog. See T269647.
      • On desktop, the suggestions do not auto-advance. After the user makes their selection, they should be in the same spot, with the button selected, and they have to use the arrows to navigate forward or backward.
    • The state of the "Yes" and "No" buttons is sticky as the user navigates around. They are allowed to change their selections.
  • Animation
  • T278486: Add a link: link inspector animations
  • Misc
Mobile mockups as of 2021-01-12:
image.png (681×598 px, 133 KB)
Desktop mockups as of 2021-01-12:
image.png (386×483 px, 56 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

Instrumentation

See T278116: Instrumentation: Link inspector for details.

Related Objects

Event Timeline

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

Feel free, I haven't done any work on this, just happened to run in the bug mentioned above in a different context.

Change 674306 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Rejection dialog: Stash result, adjust autoadvance

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

Change 674312 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Link inspector: Adjust icon and flags per spec

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

Change 674312 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Link inspector: Adjust icon and flags per spec

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

mepps removed mepps as the assignee of this task.Mar 25 2021, 2:23 PM

Based on conversation with @kostajh I'm going to take my name off this.

@RHo if the user reloads the page after they've started clicking yes/no/skip on recommendations, what should the behavior be? 1) user starts over from scratch or 2) we retain the judgments the user already added for those recommendations?

@RHo if the user reloads the page after they've started clicking yes/no/skip on recommendations, what should the behavior be? 1) user starts over from scratch or 2) we retain the judgments the user already added for those recommendations?

Can we follow the same behaviour as VE where a warning dialog is shown when the person reloads the page that their changes may be lost, but tries to retain the changes after reload with a message letting them know their changes may have been kept? Some screenshots of the existing behaviour in VE below:

Reload warning message mobile
Screenshot_20210330-101114.png (2×1 px, 185 KB)
Post-reload message (note the changes were recovered/retained)
Screenshot_20210330-101128.png (2×1 px, 252 KB)

@RHo if the user reloads the page after they've started clicking yes/no/skip on recommendations, what should the behavior be? 1) user starts over from scratch or 2) we retain the judgments the user already added for those recommendations?

Can we follow the same behaviour as VE where a warning dialog is shown when the person reloads the page that their changes may be lost, but tries to retain the changes after reload with a message letting them know their changes may have been kept? Some screenshots of the existing behaviour in VE below:

Reload warning message mobile
Screenshot_20210330-101114.png (2×1 px, 185 KB)
Post-reload message (note the changes were recovered/retained)
Screenshot_20210330-101128.png (2×1 px, 252 KB)

Yes, I think we can attempt that.

Change 675720 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] Link inspector: Keep the context item open by default

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

Change 674306 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Link inspector: Auto advance only on mobile

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

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

[mediawiki/extensions/GrowthExperiments@master] [WIP] Add a link: link inspector

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

Change 675720 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Link inspector: Force the context item to appear

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

It looks like the warning dialog is shown automatically when content editable region changes without being saved so I don't think we need to do anything specifically for this use case.

Safari:

Screen Shot 2021-04-01 at 9.36.25 AM.png (1×1 px, 793 KB)

Chrome:

Screen Shot 2021-04-01 at 9.41.55 AM.png (1×2 px, 1 MB)

As for the data restoration post-reload, I think that won't be doable until the real fix for plugin loading is in (T277228) since without it, our add link plugin isn't loaded at all upon reload.

Here are the updated link inspector UIs. The difference from the mocks is that there is no edit icon on the top right since link target cannot be edited (for now).

Desktop:

addlink_inspector_desktop.png (1×1 px, 694 KB)

Mobile:

addlink_inspector_mobile.png (1×746 px, 283 KB)

Hi @RHo, @MMiller_WMF mentioned that we are doing away with auto-advance entirely on both platforms. Can you confirm the behavior on mobile? Thanks!

Hi @RHo, @MMiller_WMF mentioned that we are doing away with auto-advance entirely on both platforms. Can you confirm the behavior on mobile? Thanks!

Hi @mewoph, as far as I know it's only Desktop that we do not have auto-advance, Mobile *still* should be auto-advance. Unless something has changed @MMiller_WMF while I was out?

Hi again @mewoph, clarified with @MMiller_WMF that the auto advance is still on for mobile. The only thing that changed is the label on right chevron icon button is now "Next" (no longer "Skip").

Hi again @mewoph, clarified with @MMiller_WMF that the auto advance is still on for mobile. The only thing that changed is the label on right chevron icon button is now "Next" (no longer "Skip").

I wanted to clarify this in the description:

If they tap "Yes" on the final suggestion, they should be advanced to the edit summary as if they had clicked to publish.

That should only be for mobile, and not on desktop, is that right? So the user taps "Yes" on the final suggestion on desktop and nothing special happens, they have to know that they should click the blue save changes button at the top right?

Change 676171 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Add a link: link inspector

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

Hi again @mewoph, clarified with @MMiller_WMF that the auto advance is still on for mobile. The only thing that changed is the label on right chevron icon button is now "Next" (no longer "Skip").

I wanted to clarify this in the description:

If they tap "Yes" on the final suggestion, they should be advanced to the edit summary as if they had clicked to publish.

That should only be for mobile, and not on desktop, is that right? So the user taps "Yes" on the final suggestion on desktop and nothing special happens, they have to know that they should click the blue save changes button at the top right?

Yes, that's correct because there is no auto-advance. However, if on the very last suggestion the person selects "Next" on Desktop, this is the same as tapping "Publish"

I wanted to clarify this in the description:

If they tap "Yes" on the final suggestion, they should be advanced to the edit summary as if they had clicked to publish.

That should only be for mobile, and not on desktop, is that right? So the user taps "Yes" on the final suggestion on desktop and nothing special happens, they have to know that they should click the blue save changes button at the top right?

Yes, that's correct because there is no auto-advance. However, if on the very last suggestion the person selects "Next" on Desktop, this is the same as tapping "Publish"

@Tgr do you want to implement that as part of your work on T269657: Add a link: edit summary and publish?

This comment was removed by mewoph.

For @RHo review

(1)

  • If the user reloads the page while proceeding through link recommendations, we should follow the same behaviour as VE where a warning dialog is shown when the person reloads the page that their changes may be lost, but tries to retain the changes after reload with a message letting them know their changes may have been kept.

It seems that VE has become very confident on the edits-in-progress recovery that they provide only "Changes recovered" message. When a user reloads a page-there is a browser-specific message as per @mewoph comment.
Presently in betalabs if a user reloads an article page in the AI mode - the links (confirmed and rejected) would not be recovered.

Since it seems cumbersome to restore a user's work on links ( as per this @mewoph comment), may be it's enough for now to just have that browser-default warning?

Items with
(2)

  • Below that is a little article preview, with image, title, and article extract. The title should be a blue link that opens the article in a new tab. This should all be a fixed height, with the article title flowing into ellipses if it doesn't fit on one line, and the extract flowing into ellipses if it doesn't fit on two lines.

A little article preview is not present. It might be a limitation of the way the articles are extracted for betalabs env?

(3)

  • There is a mock in the spec showing the desired overflow behavior in cases where the translations for "Yes", "No" and "Next" are long. In this case, the card should become taller, the "Yes" and "No" buttons should be in the same place, the left and right arrow buttons should be on the next line down.

I did not find the specific mock. I checked the current behavior and the behavior for longer strings for "Yes" and "No" does not seem to follow the specs,

Screen Shot 2021-04-14 at 1.55.43 PM.png (206×449 px, 26 KB)

(4)

  • On mobile, after the user taps "Yes", they should be advanced to the next suggestion. If they tap "Yes" on the final suggestion, they should be advanced to the edit summary as if they had clicked to publish.

Clicking on the final 'Yes' will bring "Save your changes" popup.

For @RHo review
(1)

  • If the user reloads the page while proceeding through link recommendations, we should follow the same behaviour as VE where a warning dialog is shown when the person reloads the page that their changes may be lost, but tries to retain the changes after reload with a message letting them know their changes may have been kept.

It seems that VE has become very confident on the edits-in-progress recovery that they provide only "Changes recovered" message. When a user reloads a page-there is a browser-specific message as per @mewoph comment.
Presently in betalabs if a user reloads an article page in the AI mode - the links (confirmed and rejected) would not be recovered.
Since it seems cumbersome to restore a user's work on links ( as per this @mewoph comment), may be it's enough for now to just have that browser-default warning?

Agree this is sufficient, thanks both for looking into this!

Items with
(2)

  • Below that is a little article preview, with image, title, and article extract. The title should be a blue link that opens the article in a new tab. This should all be a fixed height, with the article title flowing into ellipses if it doesn't fit on one line, and the extract flowing into ellipses if it doesn't fit on two lines.

A little article preview is not present. It might be a limitation of the way the articles are extracted for betalabs env?

Is this a candidate for breaking off into a task for "Test in Production"? This is how the article extract is meant to appear btw:

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

(3)

  • There is a mock in the spec showing the desired overflow behavior in cases where the translations for "Yes", "No" and "Next" are long. In this case, the card should become taller, the "Yes" and "No" buttons should be in the same place, the left and right arrow buttons should be on the next line down.

I did not find the specific mock. I checked the current behavior and the behavior for longer strings for "Yes" and "No" does not seem to follow the specs,

Screen Shot 2021-04-14 at 1.55.43 PM.png (206×449 px, 26 KB)

Thanks @Etonkovidova - I created two design styling tasks for this, the first is about button styles T280276 on the inspector, and the second T280277 are some layout tweaks including the positioning of the buttons when text is long. The example in the figma spec which shows how it should look when buttons are long is here btw: https://www.figma.com/file/2SONd8P1tsexIB5coMOp8h/Add-links-structured-task-v1.0?node-id=195%3A103

(4)

  • On mobile, after the user taps "Yes", they should be advanced to the next suggestion. If they tap "Yes" on the final suggestion, they should be advanced to the edit summary as if they had clicked to publish.

Clicking on the final 'Yes' will bring "Save your changes" popup.

Ah yes, @mewoph - is this because it needs to be "hooked up" to T269657?

Based on #2 (should be a no-issue in production, although testwiki does not show an article extract either) and #4 (the edit summary hasn't been implemented yet), I am moving this task Test in Production|Watching.

I'll start a list of open issues to re-check, so it will be easier to track down the issues mentioned in comments.

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

[mediawiki/extensions/GrowthExperiments@master] AddLink: Fix context intro string

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

@MMiller_WMF in your testing notes you mentioned that the string should be Should this link to the following article: (and that's what the phab description says too).

That looks like this:

image.png (274×415 px, 21 KB)

What do you think about adding a question mark to the end instead of a colon?

image.png (274×415 px, 21 KB)

Otherwise, it's kind of confusing IMHO to have a sentence phrased as a question but without a question mark at the end.

Alternatively we could use phrasing like "Please indicate if the highlighted suggestion should link to the following article."

cc @RHo

¢2: having it as a question is more clear and inviting.

@kostajh -- good idea. Let's do it with the question mark. I'll change the task description.

kostajh triaged this task as Medium priority.May 6 2021, 7:56 PM

Change 684813 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] AddLink: Fix context intro string

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

Re-checked the items with and and updated the task description with the items that are DONE.

Below that is a little article preview, with image, title, and article extract. The title should be a blue link that opens the article in a new tab. This should all be a fixed height, with the article title flowing into ellipses if it doesn't fit on one line, and the extract flowing into ellipses if it doesn't fit on two lines.

DONE

Screen Shot 2021-05-07 at 2.00.50 PM.png (692×1 px, 249 KB)
Screen Shot 2021-05-07 at 2.01.03 PM.png (674×1 px, 161 KB)

Ellipses are present too:

Screen Shot 2021-05-07 at 2.12.38 PM.png (578×964 px, 104 KB)

On mobile, after the user taps "Yes", they should be advanced to the next suggestion. If they tap "Yes" on the final suggestion, they should be advanced to the edit summary as if they had clicked to publish. The same concept applies if they tap "No", but is discussed in the task for the rejection dialog.

DONE partially. There is a slight difference in behavior when last Yes' is clicked vs when the last 'No' is clicked. When last 'Yes' is clicked - it'd be as described in the task: a user is advanced to the edit summary. When the last 'No' is clicked (all previous answers might be 'Yes'), the edit summary doesn't appear - a user needs to click 'Next' to see it. @RHo - please confirm whether it needs some additional work to match 'Yes' behavior.


not DONE

There is a mock in the spec showing the desired overflow behavior in cases where the translations for "Yes", "No" and "Next" are long. In this case, the card should become taller, the "Yes" and "No" buttons should be in the same place, the left and right arrow buttons should be on the next line down.

If the user reloads the page while proceeding through link recommendations, we should follow the same behavior as VE where a warning dialog is shown when the person reloads the page that their changes may be lost, but tries to retain the changes after reload with a message letting them know their changes may have been kept.

The current behavior with reloading (navigating away) from suggested links surface is captured in T280422: Link recommendation inspector - reloading page workflows. However, it turns out that on mobile there will be no warning to users when they reload a page (or navigate away). . VE on mobile will give a warning and then recover the changes. I re-checked the scenarios listed in T280422 in a mobile context and filed T282290: [mobile] suggested add link inspector - reloading page workflows.

Resolving as the 'not done' items have been filled as separate tasks T282424, T282427, and T282290.

Re-checked the items with and and updated the task description with the items that are DONE.

Below that is a little article preview, with image, title, and article extract. The title should be a blue link that opens the article in a new tab. This should all be a fixed height, with the article title flowing into ellipses if it doesn't fit on one line, and the extract flowing into ellipses if it doesn't fit on two lines.

DONE

Screen Shot 2021-05-07 at 2.00.50 PM.png (692×1 px, 249 KB)
Screen Shot 2021-05-07 at 2.01.03 PM.png (674×1 px, 161 KB)

Ellipses are present too:

Screen Shot 2021-05-07 at 2.12.38 PM.png (578×964 px, 104 KB)

On mobile, after the user taps "Yes", they should be advanced to the next suggestion. If they tap "Yes" on the final suggestion, they should be advanced to the edit summary as if they had clicked to publish. The same concept applies if they tap "No", but is discussed in the task for the rejection dialog.

DONE partially. There is a slight difference in behavior when last Yes' is clicked vs when the last 'No' is clicked. When last 'Yes' is clicked - it'd be as described in the task: a user is advanced to the edit summary. When the last 'No' is clicked (all previous answers might be 'Yes'), the edit summary doesn't appear - a user needs to click 'Next' to see it. @RHo - please confirm whether it needs some additional work to match 'Yes' behavior.

Thanks @Etonkovidova - this does need additional work to reflect the description. I have filed this as a new task T282424 for post-release though as it is not a major blocker (in that one can still proceed by tapping next or the publish button in the top toolbar).


not DONE

There is a mock in the spec showing the desired overflow behavior in cases where the translations for "Yes", "No" and "Next" are long. In this case, the card should become taller, the "Yes" and "No" buttons should be in the same place, the left and right arrow buttons should be on the next line down.

Again, I've filed a separate task T282427 to fix this item. We can also work on it post-=release, assuming that there are no issues with overflow on our 4 pilot languages.

If the user reloads the page while proceeding through link recommendations, we should follow the same behavior as VE where a warning dialog is shown when the person reloads the page that their changes may be lost, but tries to retain the changes after reload with a message letting them know their changes may have been kept.

The current behavior with reloading (navigating away) from suggested links surface is captured in T280422: Link recommendation inspector - reloading page workflows. However, it turns out that on mobile there will be no warning to users when they reload a page (or navigate away). . VE on mobile will give a warning and then recover the changes. I re-checked the scenarios listed in T280422 in a mobile context and filed T282290: [mobile] suggested add link inspector - reloading page workflows.

Great thanks for filing the task!