Page MenuHomePhabricator

Re-introduce a core icon pack with the icons necessary to render the built-in widgets
Closed, DeclinedPublic

Description

We've recently removed the core icon pack in OOjs UI (https://gerrit.wikimedia.org/r/#/c/238324/) and we're in the process of updating everything not to rely on it (T166730). But I just realized that we've made a huge omission – no one looked at which icons we use on OOjs UI code ourselves. I have made this analysis now: here's which icon packs are required by the four parts of the library:

  • core: content, alerts, interactions
    • FieldLayout uses 'info' (if 'help' or 'notices' config option is given)
    • FieldLayout uses 'alert' (if 'errors' config option is given)
    • FieldsetLayout uses 'info' (if 'help' config option is given)
    • PopupWidget uses 'close' (if 'head' config option is given)
    • TextInputWidget uses 'search' (when 'type' is 'search', deprecated)
    • SearchInputWidget uses 'search'
  • widgets: interactions, content, editing-advanced, movement, moderation
    • NumberInputWidget uses 'add', 'subtract'
    • SearchWidget uses 'search'
    • SelectFileWidget uses 'upload', 'attachment', 'close'
    • OutlineControlsWidget uses 'add', 'collapse', 'expand', 'trash'
  • toolbars: movement
    • ListToolGroup uses 'collapse', 'expand'
  • windows: movement
    • ProcessDialog uses 'previous' (in mobile mode, if it has a 'back' action)

Counted together, the now-required icon packs (alerts, interactions, content, editing-advanced, movement, moderation) are far larger than the old core pack. Even if we look at the core widgets/layouts only (content, alerts, interactions), the newly required packs are still larger than the old one. So it looks like we've made a huge mistake. (As a side note, it seems that the grouping of icons into packs does not reflect the use cases very well.)

I think we need to re-introduce a core icon pack with the icons necessary to render the built-in widgets. I don't think we can require our users to keep track of this bookkeeping to add the 'interactions' pack or whatever every time they use a PopupWidget (or use another widget that uses PopupWidget…).

Event Timeline

Change 357011 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] Add appropriate OOjs UI icon pack dependencies for OOjs UI itself

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

matmarex triaged this task as Unbreak Now! priority.Jun 3 2017, 12:16 PM
matmarex added subscribers: Volker_E, Jdforrester-WMF.

“far larger“ is fairly strong words. What size increase do we talk about gzipped?

Change 357011 merged by jenkins-bot:
[mediawiki/core@master] Add appropriate OOjs UI icon pack dependencies for OOjs UI itself

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

This has been open almost a month, and the only patch was merged. Is this still UBN? What needs to be done?

It's Volker's call as PO. UBN for the project, but it has no impact in production.