Page MenuHomePhabricator

Kill badgeicons.json and use OOUI's icons-alerts.json instead
Closed, ResolvedPublic

Description

Now that OOUI's "alerts" icon pack includes all the icons we need, we can use it and get rid of the badgeicons module.

  • Replace uses of the ext.echo.badgeicons module with oojs-ui.styles.icons-alerts
  • Delete the ext.echo.badgeicons module definition in extension.json
  • Delete modules/icons/badgeicons.json and the icon files it refers to (badge.svg and tray.svg)

Requires

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJul 8 2016, 6:15 PM

As the implementer of this hack, I fully agree with the "Ewwww". It was a hack I was pushed into doing at the time for performance reasons, because the icons we needed were scattered across many icon packs, and pulling in all of them would be wasteful. Now that OOUI's icon packs have been reorganized, the situation is hopefully less bad, so I should look at this again.

Volker_E triaged this task as Low priority.Apr 18 2017, 12:44 AM
Volker_E added subscribers: Pginer-WMF, Volker_E.EditedDec 5 2017, 2:28 AM

In order to get rid of badgeicons.json and use OOUI's icons, we'd also need to upstream 'double-check' icon. @Pginer-WMF I guess you are in support of this (it's then also missing in M229 and hasn't been overhauled yet)?

Volker_E updated the task description. (Show Details)Dec 5 2017, 2:30 AM
Volker_E updated the task description. (Show Details)

In order to get rid of badgeicons.json and use OOUI's icons, we'd also need to upstream 'double-check' icon. @Pginer-WMF I guess you are in support of this (it's then also missing in M229 and hasn't been overhauled yet)?

The double-check icon is used to communicate the action affects multiple elements, and it is used in "Mark all as done/read" buttons on the notifications page and the New filters beta on the Watchlist. So I think it is useful as its own concept, complementing the single check with a key distinction that helps anticipating the effect of the action. Similar to the done-all icon in Material design.

Regarding the visual style it needs to be adjusted, since we want it to be aligned with the single check. @Volker_E, can you tell me which is the best place to update the icon? Since we are in the middle of the icon update process I want to avoid adding it to an outdated place.

@Pginer-WMF The best place would right now still be M229. It's not perfect from a process perspective coming in later, but it doesn't hurt either to add it there to move forward.

Quiddity removed a subscriber: Quiddity.Dec 5 2017, 6:55 PM

@Pginer-WMF The best place would right now still be M229. It's not perfect from a process perspective coming in later, but it doesn't hurt either to add it there to move forward.

I added an icon for "mark all as done" in the design source file for M229 (it will become visible when updated). That should make it ready to be available in the icon repo at some point. Let me know if anything else is needed.

I don't think that T160690: Be able to request a custom icon pack on-the-fly without bloating RL module registry is strictly required for this, but it would be a performance improvement if it ever happens.

Upstream 'double-check' icon to OOUI 'interactions' pack

I assume this is 'checkAll', and therefore done

Upstream 'double-check' icon to OOUI 'interactions' pack

I assume this is 'checkAll', and therefore done

Yes, indeed!

That's not on OOUI any more, is ready to be done in Notifications.

Restricted Application added a project: Growth-Team. · View Herald TranscriptJul 14 2018, 3:42 AM

@Catrope will ask @Mooeypoo if this could be a good GSoC project. Staying in "To Triage" for now until that conversation.

kostajh moved this task from Needs Discussion/Analysis to Upcoming Work on the Growth-Team board.
kostajh added a subscriber: kostajh.

We'll get to this at some point in Q2/Q3/Q4 but in the meantime, volunteer contributions are very welcome!

JTannerWMF moved this task from FY 2019-20 to Q2 2019-20 on the Growth-Team board.Jan 3 2019, 3:57 PM
JTannerWMF moved this task from Q2 2019-20 to Upcoming Work on the Growth-Team board.
Catrope updated the task description. (Show Details)Mar 25 2019, 8:03 PM
kostajh removed Catrope as the assignee of this task.Mar 29 2019, 2:22 AM

Hi @kostajh @Catrope, I will like to work on this issue.
I am a bit unclear as what needs to be done for the first point Replace uses of the ext.echo.badgeicons module with oojs-ui.styles.icons-alerts.
On using grep I can only find two places where this module has been used i.e. in /includes/EchoHooks.php and /includes/special/SpecialNotifications.php.
Do I need to change only those statements for this task?

Do I need to change only those statements for this task?

Yes, that sounds right.

JTannerWMF moved this task from Inbox to External on the Growth-Team board.Apr 2 2019, 6:04 PM

Hi @kostajh, can I work on this?

Hi @kostajh, on deleting tray.svgand bell.svg the tray and bell icon from the header also disappears as they are used in /modules/nojs/mw.echo.badge.less. So, should we delete them?

Change 504404 had a related patch set uploaded (by Kosta Harlan; owner: shivanshbindal9):
[mediawiki/extensions/Echo@master] add ooui icons

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

Hi @kostajh @Volker_E @Esanders, on deleting tray.svg and bell.svg the tray and bell icon from the header also disappears as they are used in /modules/nojs/mw.echo.badge.less. So, should we delete them and move it in some other folder or is there some other way around?

Change 510211 had a related patch set uploaded (by Shivanshbindal9; owner: shivanshbindal9):
[mediawiki/extensions/Echo@master] add ooui icons and deletes tray.svg and bell.svg from modules

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

Change 504404 abandoned by Kosta Harlan:
add ooui icons

Reason:
Abandoning in favor of I8218530ed2cdd2d81c1fc24509f36ea2b6742bd9

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

Change 510211 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Remove badgeicon module, use OOUI icon instead.

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

matmarex closed this task as Resolved.Jun 11 2019, 7:39 PM
matmarex added a subscriber: matmarex.

I assume this is fixed now. Thank you for working on it!