Page MenuHomePhabricator

Cleanup AMC feature flagging
Closed, ResolvedPublic5 Story Points

Description

AMC features were designed to be configurable so that they could be enabled separately. Currently the set of features of AMC includes 2 features that are enabled when $MFAdvancedMobileContributions is true. This is a problem as it means it makes promoting these between modes e.g. all stable users will be tricky later.

The features are:

  • Menu changes - a personal menu in the top right corner when logged in (grayed out silhouette)
  • different items in the main hamburger menu

Another problem is that the icon packs for the menu changes are shared with the icon pack for stable - these should be cut into 2 modules, to make it clearer which icons are actually associated with this feature.

Acceptance criteria

  • The menu changes are guarded by dedicated feature flags: $wgMinervaPersonalMenu and $wgMinervaAdvancedMainMenu
  • Personal menu icons inside skins.minerva.mainMenu.icons should be split out into their own ResourceLoader module and separated from the standard experience.

Developer notes

  • Menu changes - a personal menu in the top right corner when logged in (grayed out silhouette) and different items in the main hamburger menu which is not configurable and turned on whenever $MFAdvancedMobileContributions is true.
  • 'page' and 'discussion' tabs at the top of the page configured via $wgMinervaTalkAtTop
  • a history link at the top of the page configured via $wgMinervaHistoryInPageActions
  • the history link will point to either Special:MobileHistory or the desktop history page depending on the value of $wgMFUseDesktopSpecialHistoryPage
  • an overflow menu (three dots) will appear with additional links based on the value of $wgMinervaOverflowInPageActions

The following configuration provides the entire AMC experience:

$wgMFAdvancedMobileContributions = true;
$wgMinervaTalkAtTop = [
	"base" => false,
	"beta" => false,
	"amc" => true, // enable in AMC and desktop
];
$wgMFUseDesktopSpecialHistoryPage = [
	"base" => false,
	"beta" => false,
	"amc" => true,
];
$wgMinervaHistoryInPageActions = [
	"base" => false,
	"beta" => false,
	"amc" => true,
];
$wgMinervaOverflowInPageActions = [
	"base" => false,
	"beta" => false,
	"amc" => true,
];

== Problems
1) It's not possible to disable the menu changes via the feature flag without disabling the entirety of AMC
2) Currently toggling any of these features on ships the CSS and JS for all the features using modules that mix responsibilities - these are skins.minerva.amc.styles (contains all styles for AMC), wikimedia.ui (contains majority of icons) and skins.minerva.mainMenu.icons (mixes icons for stable which are loaded via JavaScript and AMC which require JS - ideally this should be its own icon pack)

QA steps (exploratory)

This change should not have made any visible changes to the AMC mode. Some exploratory testing of the beta cluster e.g. https://en.m.wikipedia.beta.wmflabs.org/wiki/Spain would be useful to confirm that is the case. ie. all the AMC features should be displaying as they did before. Specifically, check for any icons that might be missing and any UI regressions inside the personal menu (user icon in the top right of screen) and main menu (when you click the hamburger icon in top left of header)

  • Check

https://he.m.wikipedia.beta.wmflabs.org to confirm RTL is also working as before

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 30 2019, 12:23 AM

Change 526285 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] AMC features should default to true in desktop

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

Jdlrobson renamed this task from Minerva desktop should reflect AMC enabled state to [EPIC] Cleanup AMC after release.Jul 30 2019, 7:18 PM
Jdlrobson updated the task description. (Show Details)

Change 526285 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] AMC features should default to true in desktop

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

Jdlrobson updated the task description. (Show Details)Aug 5 2019, 5:11 PM
Jdlrobson renamed this task from [EPIC] Cleanup AMC after release to Cleanup AMC feature flagging.Aug 6 2019, 2:36 PM
Jdlrobson triaged this task as High priority.
Jdlrobson updated the task description. (Show Details)
Jdlrobson moved this task from Product Owner Backlog to Upcoming on the Readers-Web-Backlog board.
ovasileva set the point value for this task to 5.Aug 6 2019, 4:26 PM

Change 531756 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Separate AMC icons from non-AMC icons

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

Change 531755 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Feature flag overhaul

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

Change 531755 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Feature flag overhaul

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

Jdrewniak removed Jdrewniak as the assignee of this task.Aug 26 2019, 11:13 PM
Jdrewniak assigned this task to Edtadros.
Jdrewniak added a subscriber: Jdrewniak.

Change 531756 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Separate AMC icons from non-AMC icons

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

Jdlrobson updated the task description. (Show Details)Aug 27 2019, 12:27 AM
Jdlrobson removed a project: Patch-For-Review.
ovasileva reassigned this task from Edtadros to pmiazga.Aug 27 2019, 5:08 PM
ovasileva added a subscriber: Edtadros.
Edtadros claimed this task.Aug 29 2019, 1:34 PM
Edtadros added a subscriber: pmiazga.
Edtadros reassigned this task from Edtadros to pmiazga.Aug 29 2019, 2:29 PM

@pmiazga ... it's all yours!

QA all possible configs, everything looks good, each config works well both standalone, and with other configs.

pmiazga removed Edtadros as the assignee of this task.Aug 29 2019, 3:35 PM

The only missing thing are translations (only if we want to promote those things into beta mode) - but I'm not sure if this is something we might do.

Sample view how it looks like when all AMC features get promoted to Beta mode

/cc @ovasileva

ovasileva closed this task as Resolved.Aug 30 2019, 4:38 PM
ovasileva claimed this task.

The only missing thing are translations (only if we want to promote those things into beta mode) - but I'm not sure if this is something we might do.
Sample view how it looks like when all AMC features get promoted to Beta mode


/cc @ovasileva

Looks good. Not worried about the translations as we won't be adding AMC features to beta. Resolving.