Page MenuHomePhabricator

Newcomer tasks: get rid of reload after suggested edits initiation
Closed, ResolvedPublic

Description

@Catrope recommends that we make it so that initiating suggested edits will not cause a page reload. This will help with issues like:

Event Timeline

Moving off sprint board in favor of Newcomer Tasks V1.0 tasks.

MMiller_WMF moved this task from Inbox to Upcoming Work on the Growth-Team board.Nov 13 2019, 7:48 AM
Tgr added a comment.Nov 19 2019, 9:11 AM

I think the architectural constraints are:

  • we don't want to maintain the HTML structure in two different places (both client and server side)
  • we don't want to keep that structure in different places for different modules
  • we don't want to move it to the client side for all modules (eventually, maybe, but not now)

That leaves a couple options:

  • keep doing everything on the server side, but always output the HTML for suggested edits when it is enabled on the feature flag level, even if the user did not go through the activation process (intro overlay) yet (just hide it in that case). The client-side code will only have to know which blocks to hide / unhide when the user performs the activation. Con:
    • the page layout changes significantly on desktop after activation and this would force us to render the old and new one with identical HTML and handle all the rearranging (ie the stuff now done by SpecialHomepage::getModuleGroups) with CSS or maintain client-side JS logic to handle it
    • if we want to do T236738: Newcomer tasks: server-side rendered version of suggested edits module there might be a performance hit for the homepage even with suggested edits not activated.
  • Expose rendered module HTML via an API, after suggested edits activation fetch and display it. Con: same issue with rearranging as above, plus somewhat slower (but T236738 is easy to handle now).
  • Use some method of sharing templates between server and client (API, ResourceLoader, put it in HTML...). Probably overcomplication for the module contents, but could be a reasonable way of handling the layout rearranging issue.

I would probably go with a mix of the second and third option - use templates for the pre- and post-activation homepage layout (shared via ResourceLoader, since they are very small), and create a query API endpoint for getting the contents of modules.

Tgr added a comment.Nov 26 2019, 10:36 PM
  • we don't want to move it to the client side for all modules (eventually, maybe, but not now)

The suggested edits module has both server side and client side logic for rendering the module skeleton (the full HTML is mostly rendered server-side via OOUI), so the above is moot. The mixed approach is maybe not ideal in the long term, but for now we can just keep using (or, to some extent, start using) the client-side logic.

Tgr claimed this task.Nov 26 2019, 11:49 PM

Change 554021 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/extensions/GrowthExperiments@master] [WIP] Initialize suggested edits module on the client side

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

Change 558018 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/extensions/GrowthExperiments@master] Add HomepageModule::needsTemplate()

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

Change 558018 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Allow modules to export arbitrary module data, not just overlays

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

Change 554021 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] Initialize suggested edits module on the client side

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

Tgr added a comment.Dec 17 2019, 10:16 PM

Note that the dialog closes sooner than ideal on mobile (when the route is changed), before the first task is loaded. That's not ideal but probably non-trivial to fix (our code is not significantly different for mobile and desktop, so this is probably something introduced by MobileFrontend). Should be fixed at some point but less of a priroity; filed as T241002: Newcomer tasks: StartEditingDialog should stay open on mobile while the first task loads.

Etonkovidova closed this task as Resolved.Dec 18 2019, 11:33 PM

Note that the dialog closes sooner than ideal on mobile (when the route is changed), before the first task is loaded. That's not ideal but probably non-trivial to fix (our code is not significantly different for mobile and desktop, so this is probably something introduced by MobileFrontend). Should be fixed at some point but less of a priroity; filed as T241002: Newcomer tasks: StartEditingDialog should stay open on mobile while the first task loads.

Each initial load - when a user navigates to Suggested edits module - is noticeably slow (~4-5s), but since it'll be addressed in T238171, I'm closing this task as Resolved by now.

Urbanecm edited subscribers, added: Urbanecm_WMF; removed: Urbanecm.Aug 26 2020, 1:51 PM