Page MenuHomePhabricator

Refactor help panel code
Closed, DeclinedPublic

Description

The help panel has evolved a lot from its initial implementation. It's now used:

  • in read mode
  • edit mode
  • for guidance with suggested edits
  • on Special:Homepage

And further it has different modes and capabilities depending on how it is used.

The code to control the business logic for all of the above is spread out across backend code (HelpPanelHooks.php, but also Homepage module code) as well as front-end (init.js, cta.js, helppanelprocessdialog.js). The front end code in particular is lightly tested.

This task would propose to:

  1. create a help panel controller in JS that handles the business logic and is unit testable (using our existing node-qunit setup)
  2. slim the init.js code to the minimum needed to load the CTA and create the controller (which could then also open up the process dialog if in a suggested edit session), addressing a performance concern from T218591
  3. related to the previous point, remove business logic from HelpPanelProcessDialog.js wherever possible

Event Timeline

MMiller_WMF subscribed.

I prioritized this for us to work on before starting Variant C vs. D work.

Change 605998 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[mediawiki/extensions/GrowthExperiments@master] WIP: Refactor help panel

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

@kostajh -- could you please list out a few different scopes for this, with a few different estimates? Perhaps by Board Refinement on June 23? That will help us choose what to do.

Didn't have a chance to do this yet, but it's (high up) on my todo list.

@kostajh -- could you please update the task description to describe what we decided here? Maybe this should become the long-term task about converting the help panel to Vue.js.

One thing that would be nice IMO is making the code more debugging-friendly - expose all the classes via some means (like mw.libs), and maybe the main objects too (although for that the common counter-argument is that it makes it easy for gadgets to interfere with deployed code and that makes maintenance harder).

From T256926: Help panel code cleanup July 2020 (which I did not get around to, unfortunately):

The refactoring that would have matched the task well would have been splitting up HelpPanelProcessDialog by extracting panels/screens, creating a base class for them, creating subclasses for the various panels and making those panel objects provide the root screen buttons, the actions etc. That would make the code easier to navigate and variations in panel content based on workflow and user configuration (and the homepage reuse of the help panel) easier to handle. (It would also make writing JS unit tests easier, if we ever got around to that.)

A related issue is that with the current logic, the various help panel screens are always built, even on the homepage where most of them will never be used. That's easy to miss and can cause errors where the builder logic throws an error and the mentor dialog breaks, even though that screen would work fine. (T259501 is a recent example.)

Change 605998 abandoned by Kosta Harlan:
[mediawiki/extensions/GrowthExperiments@master] WIP: Refactor help panel

Reason:

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

Declining, we'll do refactoring as part of the Vue migration.