Page MenuHomePhabricator

Add Image: Generalize Add Link ResourceLoader modules
Closed, ResolvedPublic

Description

Split the four Add Link ResourceLoader modules (ext.growthExperiments.AddLink, ext.growthExperiments.AddLink.desktop, ext.growthExperiments.AddLink.mobile, ext.growthExperiments.AddLink.onboarding) into a generic part that's reusable for all structured tasks, and an AddLink specific part. (We might want to keep them in the same module for the sake of keeping startup module size down, but at a minimum the classes need to be split.)

Generic concerns:

  • visual / machine editing mode and the related functionality (toolbar icon/label, switch button, removing other tools, warning dialog, hacks for making VE readonly mode behave, autosave hack)
    • loading/initialization for this (maybe change mobile.js/desktop.js into classes so that inheritance can be used)
  • onboarding dialog, probably
  • ​null edit mechanics and autosin AddLinkArticleTarget
  • A/B testing support for task types
  • the EventLogging logger (though probably not the schema)
  • having a save dialog base class; also adding the extra data to the API call in AddLinkArticleTarget.save()
  • maybe save button + having some abstraction for the "some recommendations have been accepted" / "some recommendations have been rejected" checks that save button mechanics are based on? (with some=1 for Add Image) We might want to just auto-open the dialog if the user accepts/rejects though, so this should wait for the final design.
  • probably error dialogs like "no suggestions"

Add Link specific concerns:

  • everyting to do with annotations
  • fragment matching logic in AddLinkArticleTarget
  • link inspector dialog + rejection dialog
  • skip dialog
  • onboarding dialog content

Task to rename message keys: T290040: Add Image: Rename message keys that are not specific to add link

Event Timeline

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

[mediawiki/extensions/GrowthExperiments@master] Create ext.growthExperiments.StructuredTask module

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

kostajh triaged this task as Medium priority.Aug 27 2021, 11:02 AM

Here is a diagram showing new module structure w/the patch:

structuredtasks.png (1×992 px, 136 KB)

Change 714869 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Create ext.growthExperiments.StructuredTask module

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

Checked in betalabs (desktop/mobile) - all looks and works as expected.

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

[mediawiki/extensions/GrowthExperiments@master] Update StructuredTask mixins

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

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

[mediawiki/extensions/GrowthExperiments@master] Add a link: Move duplicated loadSuccess & surfaceReady methods into AddLinkArticleTarget

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

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

[mediawiki/extensions/GrowthExperiments@master] Structured Task: Rename RecommendedLinkToolbarDialogButton to MinimizedToolbarDialogButton

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

Change 717522 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Update StructuredTask mixins

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

Change 717594 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Add a link: Move duplicated loadSuccess & surfaceReady methods into AddLinkArticleTarget

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

Change 717657 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Structured Task: Rename RecommendedLinkToolbarDialogButton to MinimizedToolbarDialogButton

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

I think everything is done here. Great work! This was probably the biggest of the tasks that got classified as backend.

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

[mediawiki/extensions/GrowthExperiments@master] Log error in SuggestionInteractionLogger if log() is called before logger instance is set

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

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

[mediawiki/extensions/GrowthExperiments@master] Move functions for updating the editing surface to MachineSuggestionsMode

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

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

[mediawiki/extensions/GrowthExperiments@master] Move logic specific to add link save dialog to AddLinkSaveDialog

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

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

[mediawiki/extensions/GrowthExperiments@master] Update StructuredTaskArticleTarget.isSaveable to check if the user has made any edits or reviewed suggestions

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

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

[mediawiki/extensions/GrowthExperiments@master] Create RejectionReasonSelect class for converting list of reasons into a widget & supporting both single and mutiple selections

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

Change 719305 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Log error in SuggestionInteractionLogger if log() is called before logger instance is set

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

Change 719310 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Move functions for updating the editing surface to MachineSuggestionsMode

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

Change 719315 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Move logic specific to add link save dialog to AddLinkSaveDialog

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

Change 719344 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Update StructuredTaskArticleTarget.isSaveable to check if the user has made any edits or reviewed suggestions

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

Change 719373 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Create RejectionReasonSelect class for converting list of reasons into a widget & supporting both single and mutiple selections

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

The regression testing (and logstash monitoring for eventlogging errors) was done. All works as expected.