Page MenuHomePhabricator

Clean up language button related code on mobile web (instrumentation + old feature flagged code)
Closed, ResolvedPublic3 Story Points

Description

Remove instrumentation language switching

The language button instrumentation relies on a scroll event handler and on mw.viewport.isElementInViewport(), which forces a style recalc. The simplest way to fix this would be to remove the instrumentation, if we have accumulated enough data.

Furthermore, once we've collected sufficient data, we won't need the modal eventing anymore, either.

[Done] Remove feature MinervaUsePageActionBarV2

This supports the old page actions bar which didn't have the language icon.
Supporting 2 modes of page actions is no longer necessary.

  • Remove checks for MinervaUsePageActionBarV2 and need for variable
  • Remove use of feature-page-action-bar-v2 class

Remove feature MinervaBottomLanguageButton

The bottom language button is feature flagged using that variable. Remove the HTML generation in SkinMinerva.php, the related CSS rules to #page-secondary-actions .language-selector, and related JS code that enables this button to work (is kind of shared with the page-actions one, so maybe there's not that much to remove).

Acceptance criteria

  • Language button related event logging is disabled
  • Language button related schemas are marked as inactive
  • Action bar styles and feature flags are unflagged and stabilized (grep for T130849 to find FIXMEs)

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
dr0ptp4kt updated the task description. (Show Details)Apr 4 2016, 4:52 PM
dr0ptp4kt set the point value for this task to 1.
bmansurov updated the task description. (Show Details)Apr 4 2016, 4:57 PM

Based on the timing of rollout, I think this now belongs in sprint 71. Moving there.

Jdlrobson changed the task status from Open to Stalled.Apr 12 2016, 4:50 PM
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptApr 19 2016, 9:55 PM
Jhernandez renamed this task from Language button instrumentation performs poorly to Remove instrumentation on language switcher button on mobile web.Jun 29 2016, 4:42 PM
Jhernandez changed the task status from Stalled to Open.
Jhernandez added subscribers: gerritbot, StudiesWorld.
Jhernandez renamed this task from Remove instrumentation on language switcher button on mobile web to Remove instrumentation on language switcher on mobile web.Jun 30 2016, 4:40 PM
dr0ptp4kt renamed this task from Remove instrumentation on language switcher on mobile web to Remove instrumentation language switching on mobile web.Jun 30 2016, 4:42 PM
dr0ptp4kt updated the task description. (Show Details)
dr0ptp4kt changed the point value for this task from 1 to 2.
dr0ptp4kt changed the point value for this task from 2 to 3.
Krinkle moved this task from Inbox to Radar on the Performance-Team board.
Krinkle removed a subscriber: Performance-Team.
Jhernandez renamed this task from Remove instrumentation language switching on mobile web to Clean up language button related code on mobile web (instrumentation + old feature flagged code).Jul 20 2016, 12:50 PM
Jhernandez updated the task description. (Show Details)
Jhernandez updated the task description. (Show Details)Jul 29 2016, 9:10 AM
Jdlrobson updated the task description. (Show Details)Sep 27 2016, 8:35 PM

Change 313070 had a related patch set uploaded (by Jdlrobson):
Remove MinervaUsePageActionBarV2 config variable

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

Jdlrobson updated the task description. (Show Details)Sep 27 2016, 8:38 PM
ovasileva updated the task description. (Show Details)Oct 11 2016, 3:15 PM
ovasileva added a subscriber: ovasileva.

removed "Bottom language button code and feature flag are removed from the code" from a/c as is tracked in T143829: Remove unnecessary "Read in another language" button except for on Main pages

Jdlrobson moved this task from To Do to Doing on the Reading-Web-Sprint-83-Y? board.
Jdlrobson updated the task description. (Show Details)

Change 315318 had a related patch set uploaded (by Jdlrobson):
Remove MinervaBottomLanguageButton

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

Jdlrobson updated the task description. (Show Details)Oct 11 2016, 6:52 PM

Change 315325 had a related patch set uploaded (by Jdlrobson):
Remove language overlay instrumentation

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

I've reviewed and +1'd rEMFR4ad702303fc6: Remove language overlay instrumentation. The following are a tidied up version of the comment I left on the change:

With $wgMinervaAlwaysShowLanguageButton = true; added to LocalSettings.php, I went through the following scenarios:

  • On a page with no languages, clicking the switch language page action should trigger a toast
  • On a page with languages, clicking the switch language page action should show the language switcher overlay and I should be able to switch language
  • On the Main Page, clicking the "Read in another language" secondary page action should show the language switcher overlay and I should be able to switch language
phuedx changed the task status from Open to Stalled.Oct 13 2016, 10:13 AM

Change 315318 merged by jenkins-bot:
Remove MinervaBottomLanguageButton

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

Change 315325 merged by jenkins-bot:
Remove language overlay instrumentation

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

phuedx changed the task status from Stalled to Open.Oct 20 2016, 9:48 AM
phuedx updated the task description. (Show Details)

IMO this needs sign off from an engineer – more likely @bmansurov – while T143829: Remove unnecessary "Read in another language" button except for on Main pages needs QA and then sign off from @ovasileva.

phuedx reassigned this task from Jdlrobson to bmansurov.Oct 20 2016, 9:55 AM
bmansurov removed bmansurov as the assignee of this task.Oct 20 2016, 8:23 PM
bmansurov moved this task from Ready for Signoff to To Do on the Reading-Web-Sprint-83-Y? board.

wgMFSchemaMobileWebLanguageSwitcherSampleRate and wgMinervaUsePageActionBarV2 should be removed from mediawiki-config. Everything else looks good.

@Jdlrobson do you think we should clean-up the config files in the next sprint?

Yes, next sprint. Wwe can't change them until they've written the train anyway. I was planning to do a swat next week. I've created a card to capture this work and complete the epic: T148853.

bmansurov closed this task as Resolved.Oct 21 2016, 6:44 PM
bmansurov claimed this task.