Page MenuHomePhabricator

Remove the category overlay feature and output html-categories
Closed, ResolvedPublic3 Estimated Story Points

Description

NOTE: tldr: We are failing to maintain this component. The health of this code is below our normal standards.
  1. the open product questions about the future of this feature in T24660 (should we show to anons) T204834 (should we show on pages without categories) or should we redesign it (T142124).
  2. its lack of unit tests (T205126)
  3. confusing behaviour (T224820)
  4. neglected longstanding bugs (T224819, T225556)

We should remove it.

In AMC mode we make categories available to users. A blue categories button shows at the bottom of the page and clicking it opens a category overlay that lists categories on the page in two tabs "content based" and "organizational"

This is a pleasant way to view categories on a mobile browser. On long lists however it can be hard to tell whether appropriate categories are available.

That said it has various problems and we are doing a bad job of maintaining it.

Removing this feature reduces our maintenance burden and adds support for editor gadgets that do a far better job than us.

TODO

Minerva changes

  • Remove the code resources/skins.minerva.options/categories.js and the dependency to MobileFrontend
  • The browser test tests/selenium/specs/category.js is removed (and associated code)
  • The getCategoryButton code is removed inside includes/Skins/SkinMinerva.php
  • Drop the call to hasCategoryLinks inside getSecondaryActions - $buttons['categories'] should never be set.
  • SkinMinerva::getTemplateData should set the template value html-categories to empty string is hasCategoryLinks is true.
  • Add {{{html-categories}}} to skin.mustache underneath the element #page-secondary-actions

MobileFrontend changes (phase 2)

  • Remove src/mobile.startup/categoryOverlay.js
  • Remove src/mobile.categories.overlays/ folder and associated code
  • Remove associated i18n messages.

QA

Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Spain
In logged in / AMC: There is a category box at the bottom of the page
In logged in with AMC disabled: There are no category box at the bottom of the page.
When logged out: There are no category box at the bottom of the page.

QA Results - Beta

ACStatusDetails
1T246049#7163795
2T246049#7163795
3T246049#7163795

QA Results - Beta

ACStatusDetails
1T246049#7168407
2T246049#7168407
3T246049#7168407

QA Results - Prod

ACStatusDetails
1T246049#7174093
2T246049#7174093
3T246049#7174093

Related Objects

Mentioned In
T284763: Review category gadgets for mobile usage
T212465: [EPIC] None of our View's should exhibit 2 levels of inheritance
T152199: Display categories for logged-in users on mobile web stable
T270580: "Add article to category" doesn't display well for smaller screens/long translations
T258846: Editor should not use OverlayManager.replaceCurrent
T204834: [Needs input] Show the category button on pages without any categories
T205126: [EPIC] Separate CategoryOverlay responsibilities and add unit tests
T142124: [EPIC] Redesign category pages and overlay for mobile
T225556: [Bug] CategoryLookupInputWidget only highlights letters in results when user types "Category:" (or substring) first
T224820: Autocompletion in the "Add to category" screen on mobile web is confusing
T224819: CategoryLookupInputWidget search box shows <strong> tags in search results
T216182: Make CategoryTabs a dumb component
Mentioned Here
T152199: Display categories for logged-in users on mobile web stable
T142124: [EPIC] Redesign category pages and overlay for mobile
T24660: Display the categories on the mobile site for everyone
T204834: [Needs input] Show the category button on pages without any categories
T205126: [EPIC] Separate CategoryOverlay responsibilities and add unit tests
T224819: CategoryLookupInputWidget search box shows <strong> tags in search results
T224820: Autocompletion in the "Add to category" screen on mobile web is confusing
T225556: [Bug] CategoryLookupInputWidget only highlights letters in results when user types "Category:" (or substring) first

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

It would be nice to understand the strategic direction with this featureegiven the many open bugs ^

So as this ticket is now open for one year without any activity: Would it be possible at least to fix the bug with the submit button leaving the view port? Or should I open a new ticket for that? I don't really like that I have to switch to desktop mode every time I want to edit image categories on mobile.

Possible solution (quick fix, not perfect):

.overlay-header {
  position: relative;
  max-width:100vw
}
.header-action {
  position: absolute !important;
  right: 0;
}
.overlay-title {
  max-width: 50vw;
}
.oo-ui-buttonElement-button {
  max-width: 90vw;
}

Result:

grafik.png (711×463 px, 11 KB)

@Discostu the problem here is the code is not a high priority.
I'd happily review any fixes for the category feature if they were to be uploaded, but this bug is unlikely to get focus without a volunteer patch. I think changing the label from "Add to category" to "Add" would be the easiest solution here.

Jdlrobson renamed this task from Remove the category feature or give it some love to Remove the category feature and replace with Vector's HTML representation.Apr 27 2021, 6:06 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson renamed this task from Remove the category feature and replace with Vector's HTML representation to Remove the category feature and output html-categories.Apr 27 2021, 6:06 PM
Jdlrobson removed ovasileva as the assignee of this task.
Jdlrobson added a subscriber: ovasileva.

Change 683441 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/MinervaNeue@master] Replace categories button with HTML rendering

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

@ovasileva: The description of this task and @Jdlrobson's (demo?) patch seem very-nearly-almost complete. Should this be moved across the board?

@ovasileva: The description of this task and @Jdlrobson's (demo?) patch seem very-nearly-almost complete. Should this be moved across the board?

We're still discussing what the alternative design should be. There's currently some concerns from myself and @alexhollender on displaying the entire list if we were to do T152199: Display categories for logged-in users on mobile web stable as well. We could potentially:

@alexhollender - would you be okay with showing the full list for AMC only just for the time being? If so, I think we can go ahead with this one.

Following Olga's recommendation in T152199 this is ready for dev.

Jdlrobson renamed this task from Remove the category feature and output html-categories to Remove the category overlay feature and output html-categories.May 25 2021, 2:38 PM
Jdlrobson added a subscriber: Whatamidoing-WMF.

Change 683441 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Replace categories button with HTML rendering

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

Edtadros added a subscriber: Edtadros.

@Jdrewniak Can you provide some test steps please?

Change 699238 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/MobileFrontend@master] Remove categories feature

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

@Edtadros I forgot I have some follow up work here :)

Change 699238 merged by jenkins-bot:

[mediawiki/extensions/MobileFrontend@master] Remove categories feature

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

@Jdlrobson I don't like so much the deletion of the Mobile Frontend Categories Feature, but i like so much.

Test Result - Beta

Status: ❌ Fail
Environment: beta
OS: macOS Big Sur
Browser: Chrome
Device: MBP
Emulated Device: NA

Test Artifact(s):

QA Steps

Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Spain
✅ AC1: In logged in / AMC: There is a category box at the bottom of the page

Screen Recording 2021-06-19 at 12.53.43 PM.mov.gif (674×312 px, 442 KB)

❌ AC2: In logged in with AMC disabled: There is no category box at the bottom of the page.

Screen Recording 2021-06-19 at 12.54.50 PM.mov.gif (674×312 px, 293 KB)

✅ AC3: When logged out: There is no category box at the bottom of the page.

Screen Recording 2021-06-19 at 12.55.58 PM.mov.gif (674×312 px, 238 KB)

@ovasileva looks like this was also enabled for logged in users 😬.
Should I revert this and hot fix, or given this has been in production 2 weeks, would you rather prioritize looking into T152199 to avoid editors questioning why it was removed?

Change 700700 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/MinervaNeue@master] Category feature not ready for logged in users

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

Change 700700 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Category feature not ready for logged in users

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

We talked about this in standup and agreed to go ahead with reverting the change. This will go out with the train but I can backport earlier if there's urgency here (Olga let me know!). If anyone complains we will prompt them to opt into the advanced mobile mode and direct them to T152199.

Thanks @Edtadros for catching in QA! This should pass now.

Test Result - Beta

Status: ✅ Pass
Environment: beta
OS: macOS Big Sur
Browser: Chrome
Device: MBP
Emulated Device: NA

Test Artifact(s):

QA Steps

Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Spain
✅ AC1: In logged in / AMC: There is a category box at the bottom of the page

en.m.wikipedia.beta.wmflabs.org_wiki_Spain(iPhone 11 Pro Max) (4).png (2×1 px, 363 KB)

✅ AC2: In logged in with AMC disabled: There is no category box at the bottom of the page.

en.m.wikipedia.beta.wmflabs.org_wiki_Spain(iPhone 11 Pro Max) (3).png (2×1 px, 319 KB)

✅ AC3: When logged out: There is no category box at the bottom of the page.

en.m.wikipedia.beta.wmflabs.org_wiki_Spain(iPhone 11 Pro Max) (5).png (2×1 px, 361 KB)

We talked about this in standup and agreed to go ahead with reverting the change. This will go out with the train but I can backport earlier if there's urgency here (Olga let me know!). If anyone complains we will prompt them to opt into the advanced mobile mode and direct them to T152199.

Thanks @Edtadros for catching in QA! This should pass now.

Train is okay

Test Result - Prod

Status: ✅ Pass
Environment: testwiki
OS: macOS Big Sur
Browser: Chrome
Device: MBP
Emulated Device: NA

Test Artifact(s):

QA Steps

Visit https://test.m.wikipedia.org/wiki/User:Lijealso/Esplanada/Nome_do_t%C3%B3pico_teste
✅ AC1: In logged in / AMC: There is a category box at the bottom of the page

test.m.wikipedia.org_wiki_User_Lijealso_Esplanada_Nome_do_t%C3%B3pico_teste(iPhone 11 Pro Max).png (2×1 px, 291 KB)

✅ AC2: In logged in with AMC disabled: There is no category box at the bottom of the page.

test.m.wikipedia.org_wiki_User_Lijealso_Esplanada_Nome_do_t%C3%B3pico_teste(iPhone 11 Pro Max) (1).png (2×1 px, 272 KB)

✅ AC3: When logged out: There is no category box at the bottom of the page.

test.m.wikipedia.org_wiki_User_Lijealso_Esplanada_Nome_do_t%C3%B3pico_teste(iPhone 11 Pro Max) (2).png (2×1 px, 259 KB)