Page MenuHomePhabricator

Image Type dropdown does not change icon color when selected
Closed, ResolvedPublic1 Story Points

Description

As reported here, in the VisualEditor's advanced media setting dialog, the color of the icons in the dropdown does not change:

Desired color scheme:
Current color scheme:

Specifically, the class oo-ui-image-progressive needs to be added when a row is selected.

Event Timeline

Cirdan created this task.May 31 2018, 9:34 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 31 2018, 9:34 AM
Esanders triaged this task as Low priority.Jun 1 2018, 2:08 PM
Esanders added subscribers: Volker_E, Esanders.
This comment was removed by Esanders.
Volker_E removed a project: OOUI.Jun 1 2018, 2:09 PM
Volker_E added subscribers: matmarex, Jdforrester-WMF.

@matmarex @Jdforrester-WMF As far as I remember that's an issue with VE code and not OOUI…?

@Esanders Getting rid of progressive icons?

It's an OOUI issue:

DropdownWidgets are really supposed to have icons though, as in Apex we use a checkmark to show the selected item, which overrides any icon:

@Esanders Getting rid of progressive icons?

Yeah?

Cirdan added a comment.Jun 1 2018, 2:19 PM

The problem really is the missing class, I think. For my mockup I just went in and manually added the class, resulting in the correct colors.

Volker_E added a project: OOUI.EditedJun 1 2018, 2:19 PM

@Esanders There was once the idea, instead of stupidly generating an extra icon file for each color variant to make use of fill CSS attribute. But that would mean at best a large architectural change.
For now, it's more reasonable to stay with variants.

In this case, I thought we don't generate the color variants, those are non OOUI icons.

@Esanders There was once the idea, instead of stupidly generating an extra icon file for each color variant to make use of fill CSS attribute. But that would mean at best a large architectural change.
For now, it's more reasonable to stay with variants.
In this case, I thought we don't generate the color variants, those are non OOUI icons.

We do, via ResourceLoaderImageModule. The only issue here is in OOUI.

We either need to

  • Not allow icons in dropdown options
  • Style them correctly

Change 437371 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[oojs/ui@master] WikimediaUI theme: Use 'progressive' icons for pressed/selected MenuOptionWidget

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

@matmarex @Jdforrester-WMF As far as I remember that's an issue with VE code and not OOUI…?

VE's custom icons used to not have colorized variants, but we fixed that a while ago with rEVEDda422b18d5ae: Load icons where we can via RLIM.

Volker_E moved this task from Backlog to Reviewing on the OOUI board.Jun 5 2018, 9:45 PM
Jdforrester-WMF moved this task from Reviewing to OOUI-0.27.2 on the OOUI board.
Jdforrester-WMF edited projects, added OOUI (OOUI-0.27.2); removed OOUI.
Jdforrester-WMF removed a project: Patch-For-Review.

Change 437371 merged by jenkins-bot:
[oojs/ui@master] WikimediaUI theme: Use 'progressive' icons for pressed/selected MenuOptionWidget

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

Jdforrester-WMF closed this task as Resolved.Jun 5 2018, 11:25 PM
Jdforrester-WMF set the point value for this task to 1.
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptJun 5 2018, 11:25 PM

This was released in OOUI v0.27.2, which will roll out with MediaWiki 1.32.0-wmf.8.

Vvjjkkii renamed this task from Image Type dropdown does not change icon color when selected to fxbaaaaaaa.Jul 1 2018, 1:07 AM
Vvjjkkii reopened this task as Open.
Vvjjkkii removed matmarex as the assignee of this task.
Vvjjkkii raised the priority of this task from Low to High.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed the point value for this task.
Vvjjkkii removed subscribers: gerritbot, Aklapper.
CommunityTechBot renamed this task from fxbaaaaaaa to Image Type dropdown does not change icon color when selected.Jul 2 2018, 8:39 AM
CommunityTechBot closed this task as Resolved.
CommunityTechBot assigned this task to matmarex.
CommunityTechBot lowered the priority of this task from High to Low.
CommunityTechBot set the point value for this task to 1.
CommunityTechBot updated the task description. (Show Details)
CommunityTechBot added subscribers: gerritbot, Aklapper.