Page MenuHomePhabricator
Create Task
Maniphest T346162

Icons rendered via JavaScript in Minerva should use Codex mixin
Closed, ResolvedPublic3 Estimated Story PointsBUG REPORT
Actions

Description

Related: T346184

Minerva has various modules for providing icon packs. Given the limitations with the selector and variants, it is sometimes confusing which icon pack to put it in.

There are few benefits of using icon packs for modules loaded via JS (for render blocking styles they can be useful given they support use of URLs rather than data URIs) so it's suggested we remove the modules in favor of the Codex icon mixin.

TODO

  • For any icon rendered via JavaScript, use Codex mixins. This applies to the following modules:
  • skins.minerva.icons.images.scripts.misc
  • skins.minerva.icons.page.issues.uncolored
  • skins.minerva.icons.page.issues.default.color
  • skins.minerva.icons.page.issues.medium.color
  • Any icons with the mw-ui-icon-minerva prefix have the minerva-icon- prefix (if needed)
  • The following should remain icon packs, since they use "useDataURI: falsE" (or are loaded on page load):
  • skins.minerva.content.styles.images
  • skins.minerva.overflow.icons
  • skins.minerva.icons.wikimedia
  • skins.minerva.mainMenu.icons
  • skins.minerva.personalMenu.icons
  • The following modules should be updated to use useDataURI to reduce bundle size:
  • skins.minerva.mainMenu.advanced.icons
  • skins.minerva.personalMenu.icons [done in https://gerrit.wikimedia.org/r/c/mediawiki/skins/MinervaNeue/+/979964]

QA

Manually review any Pixel "regressions" and confirm they are improvements/acceptable.

Details

SubjectRepoBranchLines +/-
Icons rendered via JavaScript in Minerva should use Codex mixinmediawiki/skins/MinervaNeuemaster+0 -3
Update modules to not use data URIs to reduce bundle sizemediawiki/skins/MinervaNeuemaster+5 -4
Customize query in gerrit

Related Objects

Event Timeline

Jdlrobson created this task.Sep 12 2023, 3:59 PM
Restricted Application added subscribers: Masumrezarock100, Aklapper. · View Herald TranscriptSep 12 2023, 3:59 PM
Jdlrobson mentioned this in T344022: Codex: Clean up Minerva's use of icons.Sep 12 2023, 4:00 PM
Jdlrobson triaged this task as High priority.Sep 12 2023, 6:23 PM
Jdlrobson updated the task description. (Show Details)Sep 12 2023, 9:27 PM
ovasileva edited projects, added Web-Team-Backlog (Web Team FY2023-24 Q2 Sprint 2); removed Web-Team-Backlog (Web Team FY2023-24 Q2 Sprint 1).Oct 11 2023, 3:42 PM
ovasileva edited projects, added Web-Team-Backlog (Web Team FY2023-24 Q2 Sprint 3); removed Web-Team-Backlog (Web Team FY2023-24 Q2 Sprint 2).Oct 19 2023, 5:52 PM
ovasileva edited projects, added Web-Team-Backlog (Web Team FY2023-24 Q2 Sprint 4); removed Web-Team-Backlog (Web Team FY2023-24 Q2 Sprint 3).Nov 2 2023, 5:53 PM
Jdlrobson updated the task description. (Show Details)Nov 9 2023, 6:24 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson set the point value for this task to 3.Nov 9 2023, 6:26 PM
Jdlrobson edited projects, added Web-Team-Backlog (FY2023-24 Q2 Sprint 5); removed Web-Team-Backlog (Web Team FY2023-24 Q2 Sprint 4).Nov 20 2023, 6:38 PM
ovasileva moved this task from Incoming to Ready for Development on the Web-Team-Backlog (FY2023-24 Q2 Sprint 5) board.Nov 30 2023, 10:07 AM
• Osps7 subscribed.Nov 30 2023, 10:11 AM

Will it be usable via mobile phone? The problem with codes or programming is that it is sometimes limited to browsing my desktop

Jdlrobson mentioned this in T351569: Search bars should have magnifying glasses on mobile.Dec 4 2023, 6:56 PM
• Mabualruz claimed this task.Dec 5 2023, 5:35 PM
• Mabualruz moved this task from Ready for Development to Doing on the Web-Team-Backlog (FY2023-24 Q2 Sprint 5) board.
Jdlrobson added a comment.Dec 5 2023, 5:53 PM

Mo will start by addressing FIXMEs in the codebase.

gerritbot added a comment.Dec 5 2023, 9:32 PM

Change 979964 had a related patch set uploaded (by Mabualruz; author: Mabualruz):

[mediawiki/skins/MinervaNeue@master] Icons rendered via JavaScript in Minerva should use Codex mixin

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

gerritbot added a project: Patch-For-Review.Dec 5 2023, 9:32 PM
• Mabualruz moved this task from Doing to Code Review on the Web-Team-Backlog (FY2023-24 Q2 Sprint 5) board.Dec 5 2023, 9:42 PM
Jdlrobson updated the task description. (Show Details)Dec 6 2023, 12:23 AM
Jdlrobson moved this task from Code Review to Needs More Work on the Web-Team-Backlog (FY2023-24 Q2 Sprint 5) board.
gerritbot added a comment.Dec 6 2023, 12:44 AM

Change 979964 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] Update modules to not use data URIs to reduce bundle size

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

ReleaseTaggerBot added a project: MW-1.42-notes (1.42.0-wmf.9; 2023-12-12).Dec 6 2023, 1:00 AM
Maintenance_bot removed a project: Patch-For-Review.Dec 6 2023, 1:10 AM
• Mabualruz updated the task description. (Show Details)Dec 6 2023, 4:15 PM
  • skins.minerva.content.styles.images
  • skins.minerva.overflow.icons
  • skins.minerva.icons.wikimedia
  • skins.minerva.mainMenu.icons
  • skins.minerva.personalMenu.icons

All have "useDataURI": false

• Mabualruz updated the task description. (Show Details)Dec 6 2023, 4:16 PM
• Mabualruz added a comment.Dec 6 2023, 4:19 PM

can't find mw-ui-icon-minerva in the code base seems it was cleared up previously

• Mabualruz added a comment.Dec 6 2023, 7:10 PM
  • skins.minerva.icons.images.scripts.misc
  • skins.minerva.icons.page.issues.uncolored
  • skins.minerva.icons.page.issues.default.color
  • skins.minerva.icons.page.issues.medium.color

Those modules seems to be removed/replaced by skins.minerva.icons.wikimedia and it is using mixins with the "useDataURI": false option

• Mabualruz updated the task description. (Show Details)Dec 6 2023, 7:11 PM
• Mabualruz moved this task from Needs More Work to Code Review on the Web-Team-Backlog (FY2023-24 Q2 Sprint 5) board.
Jdlrobson updated the task description. (Show Details)EditedDec 6 2023, 7:12 PM
Jdlrobson moved this task from Code Review to Needs More Work on the Web-Team-Backlog (FY2023-24 Q2 Sprint 5) board.

Mo I am not seeing a patch for AC1. To be clear the AC is to get rid of those modules by using the Codex mixins. It is preferable to get icons from Codex as inline SVGs in our JavaScript code rather than relying on the OOUI icon pack.

gerritbot added a comment.Dec 6 2023, 10:19 PM

Change 980830 had a related patch set uploaded (by Mabualruz; author: Mabualruz):

[mediawiki/skins/MinervaNeue@master] Icons rendered via JavaScript in Minerva should use Codex mixin

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

gerritbot added a project: Patch-For-Review.Dec 6 2023, 10:19 PM
• Mabualruz added a comment.Dec 7 2023, 3:22 PM
In T346162#9387993, @Jdlrobson wrote:

Mo I am not seeing a patch for AC1. To be clear the AC is to get rid of those modules by using the Codex mixins. It is preferable to get icons from Codex as inline SVGs in our JavaScript code rather than relying on the OOUI icon pack.

I cannot find the modules mentioned in AC1

In T346162#9387990, @Mabualruz wrote:
  • skins.minerva.icons.images.scripts.misc
  • skins.minerva.icons.page.issues.uncolored
  • skins.minerva.icons.page.issues.default.color
  • skins.minerva.icons.page.issues.medium.color

Those modules seems to be removed/replaced by skins.minerva.icons.wikimedia and it is using mixins with the "useDataURI": false option

If I am missing something can you give me on example and I will carry on

Jdlrobson reassigned this task from • Mabualruz to Edtadros.Dec 7 2023, 3:54 PM
Jdlrobson moved this task from Needs More Work to QA on the Web-Team-Backlog (FY2023-24 Q2 Sprint 5) board.
Jdlrobson added a subscriber: • Mabualruz.

Talked to Mo and the AC1 is indeed done (my mistake).
The remaining result is a FIXME that can't be removed just yet: https://codesearch.wmcloud.org/search/?q=mw-ui-icon-&files=&excludeFiles=&repos=#Skin:MinervaNeue

Jdlrobson updated the task description. (Show Details)Dec 7 2023, 3:55 PM
Edtadros added a comment.Dec 18 2023, 2:35 AM

@Jdlrobson, I'm not sure what to do here. Looks like Mo did the QA already

ovasileva reassigned this task from Edtadros to Jdlrobson.Dec 18 2023, 6:12 PM
ovasileva moved this task from QA to Ready for Signoff on the Web-Team-Backlog (FY2023-24 Q2 Sprint 5) board.
ovasileva added a subscriber: Edtadros.
Jdlrobson closed this task as Resolved.Dec 19 2023, 5:40 AM

Yep this looks good to me. According to Pixel the only changes here were the 15 relating to T346670.

[1] https://pixel.wmcloud.org/archive/origin/wmf/1.42.0-wmf.9/mobile/

Jdlrobson added a project: Verified.Jan 4 2024, 1:00 AM
Jdlrobson mentioned this in T352463: Verify 1.42.0-wmf.9 deployment.Jan 4 2024, 8:57 PM
gerritbot added a comment.Mar 18 2024, 7:52 AM

Change 980830 abandoned by Mabualruz:

[mediawiki/skins/MinervaNeue@master] Icons rendered via JavaScript in Minerva should use Codex mixin

Reason:

We still are waiting for the results to be empty we will create a new patch once it is

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

Maintenance_bot removed a project: Patch-For-Review.Mar 18 2024, 8:30 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits