Page MenuHomePhabricator

Add CSS-only (or non-client-side-JS) icon
Closed, ResolvedPublic5 Estimated Story Points

Description

We need an icon that works without client-side JS. This may or may not be truly "CSS-only."

There are 2 approaches we could take.

Approach 1: Icons as CSS background-images

This is how it currently works in OOUI: you can add the class .oo-ui-icon-article to get the style background-image: url(themes/wikimediaui/images/icons/article-ltr.svg);. Progressive, destructive, and inverted (white) icons are also supported via separate classes and svg files.

Considerations:

  • This has historically been difficult to maintain and isn't flexible enough. However, we could consider using modern tools like mask-image to enable us to set the icons to any color without needing separate svgs (see here for more info on this technique)
  • This is simple to use and people are already used to it
  • We'd have to make the svgs available for use in this way

Approach 2: insert icon SVG into the markup on the server-side

We could instruct people to basically do what Icon.vue does in their back-end JS or PHP. This might look like:

import { resolveIcon, shouldIconFlip, cdxIconAlert } from '@wikimedia/codex-icons';

const resolvedIcon = resolveIcon( cdxIconAlert, 'en', 'ltr' );
const svg = typeof resolvedIcon === 'string' ? resolvedIcon : `<path d="${resolvedIcon.path}" />`;
const flipClass = shouldIconFlip( cdxIconAlert, 'en' ) ? 'cdx-icon--flipped': '';

const icon = `
    <span class="cdx-icon ${flipClass}">
        <svg ...attrs...>
            <g>${svg}</g>
        </svg>
    </span>
`;

Considerations:

  • This is a lot more work than just slapping a CSS class or two on a span
  • We could update our util functions to consolidate them into a single exportable function that provides all the icon data needed to make this a little easier
  • This is not CSS-only, it's server-side

Event Timeline

Change 870711 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] [PoC no mergey] Server renderable icon demo

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

As discussed with @Volker_E and @Catrope, we will consider the following:

  • Support background image SVGs with mask-image for color for icons commonly used inside other components, since it would be a burden to have to copy/paste or generate all the svg markup in these cases. This especially applies to a component like Message which supports 4 different icons depending on the message type.
  • For all other icon usage, decide whether we should support background image SVGs (which would require us to generate styles for only the icons being used, to avoid including a bloated CSS file with all the background image styles), support approach #2, or possibly both

Looking at caniuse browser support of CSS mask-image closer and comparing our browser support matrix, we would end up with non-colored icons for the CSS-only component in some of our basic supported browsers.

It looks like mask-image itself has decent support. Firefox 39-52 would be an issue.

egardner changed the task status from Open to In Progress.Dec 21 2022, 10:09 PM

Change 870711 abandoned by Anne Tomasevich:

[design/codex@main] [PoC no mergey] Server renderable icon demo

Reason:

This was only for demonstration purposes

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

Change 875965 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] Icon: Add limited set of CSS-only icons

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

In the patch, I created a mixin for icon background images that includes a param for the icon color, so mask-image isn't needed in this implementation path

Change 875965 merged by jenkins-bot:

[design/codex@main] Icon: Add Less mixin for CSS-only icons

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

I'd like to bring in one quest before resolving the task, currently we're splitting background-image application with a specific Less mixin and have other background properties on the icon mixin, not consistently applied.
This is a bit impractical as background-repeat: no-repeat is common for all icon applications, background-size is directly connected to the icon size, but should better be applied on the same element like image, and background-position the same.

We'd have to

  • accept either duplication of rules, on applications like Message types or Select's :enabled/:disabled elements when moving those rules out of icon and to the background-image mixin or
  • add a new mixin which carries all properties besides image and pulls image in via mixin. We should then probably also rename the mixins once more:
    • cdx-mixin-icon, cdx-mixin-css-icon. It seems confusing to have both mixins output styles and both featuring a very different scope.

Change 885071 had a related patch set uploaded (by VolkerE; author: VolkerE):

[design/codex@main] styles: Add `.cdx-mixin-icon-background()` mixin

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

Change 869852 had a related patch set uploaded (by Catrope; author: Catrope):

[mediawiki/core@master] Update Codex from v0.4.3 to v0.5.0

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

Change 885450 had a related patch set uploaded (by Catrope; author: Catrope):

[mediawiki/core@master] Update Codex from v0.4.3 to v0.5.0

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

Change 885450 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.4.3 to v0.5.0

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

Change 887407 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] Icon: refactor CSS icon mixins and introduce sizes

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

egardner set the point value for this task to 5.Feb 8 2023, 5:22 PM

Change 885071 abandoned by VolkerE:

[design/codex@main] styles: Add `.cdx-mixin-icon-background()` mixin

Reason:

Superseded by I26077c8119b2

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

With CSS mixin patch still reviewed, moving this back to code review col for now.

Change 887407 merged by jenkins-bot:

[design/codex@main] Icon: refactor CSS icon mixins and introduce sizes

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

@bmartinezcalvo it has to do with demonstrating how to use the Less mixin when you have one, two, or three parameters - if you look at the code sample, you can see how the 3 icons are all created in a slightly different way via the mixin.

What if we changed the text of the third icon to "Arrow next (extra-emall, @color-progressive fill color)" to make it more clear that this was intentional? We could also make these 3 icons the same color, then demonstrate how to make an icon a different size and color in a separate demo.

What if we changed the text of the third icon to "Arrow next (extra-emall, @color-progressive fill color)" to make it more clear that this was intentional? We could also make these 3 icons the same color, then demonstrate how to make an icon a different size and color in a separate demo.

@AnneT yes please, let's add the color indicative so the user understand why this icon is blue. Moving the task to Ready for development so you can update it.

Change 889230 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[mediawiki/core@master] Update Codex from v0.5.0 to v0.6.0

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

Test wiki created on Patch demo by ATomasevich (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/66d018802f/w

Change 889230 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.5.0 to v0.6.0

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

I'd suggest to remove the blue treatment in this demo completely, we already feature a different color in the demo above.

Change 890868 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[design/codex@main] docs: Change CSS-only icon demo to remove size/color combo

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

I've updated the demo page to remove the size/color combo; I agree that it's more confusing than helpful

Change 890868 merged by jenkins-bot:

[design/codex@main] docs: Change CSS-only icon demo to remove size/color combo

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

Change 893051 had a related patch set uploaded (by VolkerE; author: VolkerE):

[mediawiki/core@master] Update Codex from v0.6.0 to v0.6.2

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

Change 893051 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v0.6.0 to v0.6.2

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

It LGTM. Although icons are not well aligned with labels (specially the 12px one) I guess this will be solved in another task. Moving this task to QTE sign-off.

Volker_E assigned this task to AnneT.
Volker_E removed a project: Patch-For-Review.

Resolving this, the icons are already in use in the CSS-only components and have gone through design sign-off.
We'll file new tasks if we identify shortcomings in esoteric browsers.

Test wiki on Patch demo by ATomasevich (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/66d018802f/w/