Page MenuHomePhabricator

Convert TextWithIconWidget to Codex
Closed, ResolvedPublic3 Estimated Story Points

Description

TextWithIconWidget is currently built as an OOUI widget. It should be rewritten using CSS-only Codex (likely a thin wrapper around the Icon component), which would help us with:

  • Special page transclusion bugs due to the OOUI theme being global state (T388427)
  • The widget being completely broken in Monobook and Timeless, and possibly other skins (T386102)
  • Our general desire to move towards Codex

Event Timeline

From my initial look, this seems much harder than I thought due to how icon works in the CSS-only version... Maybe not, but for the time being this is blocked on the subtask.

ifried set the point value for this task to 3.Mar 20 2025, 9:56 PM

@Daimona Would you share a link and/or a screenshot of the projected use case?

@Daimona Would you share a link and/or a screenshot of the projected use case?

Yup, we use this component in a few places to present unlabeled information (i.e., the icon, or its accessibility text, are the label). Some example links are:

Change #1130127 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] TextWithIconWidget: return string instead of widget instance

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

Change #1130127 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] TextWithIconWidget: return string instead of widget instance

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

Change #1131847 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Convert TextWithIconWidget to Codex

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

Change #1131847 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Convert TextWithIconWidget to Codex

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

Daimona added a subscriber: vaughnwalters.

On second thought, I think it would make sense to have this go through QA. @vaughnwalters There isn't much to test here: we've updated the widgets used in Special:AllEvents and in the event page registration header to use Codex. They should look exactly the same as before, except for minor spacing and colour differences that are expected. Everything else should be the same (content, icons, accessibility text).

To clarify: the widgets in question are the ones I circled in red in this hi-tech, AI-powered, quantum-computing-assisted image:

TextWithIconWidget.png (750×1 px, 122 KB)

Change #1133930 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Stop loading OOUI icons previously used with TextWithIconWidget

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

Change #1133930 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Stop loading OOUI icons previously used with TextWithIconWidget

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

Problems with OOUI's IconWidget reported as result.

Created T391381 because of icons missing in Special:AllEvents

MHorsey-WMF changed the task status from Open to In Progress.EditedApr 10 2025, 3:00 PM
MHorsey-WMF changed the task status from In Progress to Stalled.
MHorsey-WMF subscribed.

Blocked due to T391296

Change #1136759 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Re-apply "Convert TextWithIconWidget to Codex"

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

Change #1136759 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Re-apply "Convert TextWithIconWidget to Codex"

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

On second thought, I think it would make sense to have this go through QA. @vaughnwalters There isn't much to test here: we've updated the widgets used in Special:AllEvents and in the event page registration header to use Codex. They should look exactly the same as before, except for minor spacing and colour differences that are expected. Everything else should be the same (content, icons, accessibility text).

To clarify: the widgets in question are the ones I circled in red in this hi-tech, AI-powered, quantum-computing-assisted image:

TextWithIconWidget.png (750×1 px, 122 KB)

This is working as expected now. Also, I appreciate your quantum computing / MS-Paint mashup @Daimona . Quick check, I didn't see cdx-icon as a class for any of these, is that expected? If so, this is good to go to design sign off

Special:EventDetailsSpecial:AllEventsEvent page
Screenshot 2025-04-16 at 2.19.38 PM.png (1×2 px, 238 KB)
Screenshot 2025-04-16 at 2.46.44 PM.png (388×704 px, 30 KB)
Screenshot 2025-04-16 at 2.46.59 PM.png (272×2 px, 51 KB)
Screenshot 2025-04-16 at 2.47.11 PM.png (1×1 px, 133 KB)

Quick check, I didn't see cdx-icon as a class for any of these, is that expected?

I think so. According to the docs, there are no standard classes for icons. Is there some documentation page suggesting otherwise?

Quick check, I didn't see cdx-icon as a class for any of these, is that expected?

I think so. According to the docs, there are no standard classes for icons. Is there some documentation page suggesting otherwise?

I'm not sure, but just inspecting the icons and looking at the example code

Quick check, I didn't see cdx-icon as a class for any of these, is that expected?

I think so. According to the docs, there are no standard classes for icons. Is there some documentation page suggesting otherwise?

I'm not sure, but just inspecting the icons and looking at the example code

I'm not sure if that's standard, at least for the CSS-only version. None of the examples linked above have it, and I'd be leaning towards following those examples.

Quick check, I didn't see cdx-icon as a class for any of these, is that expected?

I think so. According to the docs, there are no standard classes for icons. Is there some documentation page suggesting otherwise?

I'm not sure, but just inspecting the icons and looking at the example code

I'm not sure if that's standard, at least for the CSS-only version. None of the examples linked above have it, and I'd be leaning towards following those examples.

Okay thanks, I'll mark this as done / resolved then.