prefer new .jp-icon-mono class for monochromatic icons

[dlqqq]

References

• Weird colors for tool icons in certain themes #12041

Code changes

• prefer new .jp-icon-mono class for all monochromatic action icons

  • identical to existing .jp-icon3 class

• adds new .jp-icon-mono-inverse class to handle inverse colors (e.g. the shell prompt within the terminal icon).

• replaces Python icon with one more consistent with Python's style guide (from Simplify R and Python icon svgs elyra-ai/elyra#2705)

• replaces Markdown icon with icon from dcurtis/markdown-mark

User-facing changes

• new Markdown icon is used, along with a more standard "sky-blue" color shared by VS Code.
• new Python icon is used with correct styling according to Python style guide
• standardizes color for monochromatic action icons
• updated documentation to encourage theme developers to prefer .jp-icon-mono over .jp-iconX classes, which have a much more limited scope as most icons are monochromatic

Before:

After:

Backwards-incompatible changes

The terminal and console icons are now set to a monochromatic icon. Following the discussion in the original issue, it seems that the terminal and console icons are more of "action" icons (typically monochromatic) rather than "document" icons (typically colored to be both consistent with a language/filetype's branding and to be easily identifiable). Definitely open to suggestions here, as this is a significant visual change. This is also an opportunity to change the terminal and console icons altogether.

Most action icons had their previous CSS class removed and replaced with .jp-icon-mono. This will break theme extensions that rely on icons having the old CSS classes. However, standardizing the CSS class we are using for monochromatic icons will also make it significantly easier for theme developers to maintain consistent icon theming throughout the JupyterLab UI.

[jupyterlab-probot]

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link:

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.

You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

[ellisonbg]

One quick question - are there any other colored icons that are getting their colors from a generic JupyterLab theme variable? JSON? YML? Anything else?

[dlqqq]

@ellisonbg Good question. There were a few icons that I chose not to touch because they were not monochromatic, so these should probably be reviewed on a case-by-case basis on how we should replace the classes associated with them. The scope of this PR is mainly for monochromatic icons and to fix the most glaring icon CSS issues visible from the JL launcher; a separate issue and PR should be made to address the larger issue of an excessive number of CSS classes being misused throughout JL.

Icons still using old CSS classes

Uses .jp-icon0:

• packages/ui-components/style/debug/bad.svg
• packages/ui-components/style/icons/filetype/html5.svg

Uses .jp-icon1:

• packages/ui-components/style/icons/filetype/vega.svg
• packages/ui-components/test/bar.svg
• packages/ui-components/test/foo.svg

Uses .jp-icon2:

• packages/ui-components/style/icons/jupyter/jupyterlab-wordmark.svg

Uses .jp-icon3:

• packages/ui-components/style/icons/jupyter/jupyter.svg
• packages/ui-components/style/unused/text-editor-alt.svg

Uses .jp-icon4:

None.

Uses .jp-icon-accent*:

• packages/ui-components/style/unused/text-editor-alt.svg

Uses .jp-icon-brand*:

• packages/ui-components/style/icons/filetype/image.svg
• packages/ui-components/style/icons/filetype/react.svg

Uses .jp-icon-warn*:

• packages/ui-components/style/icons/filetype/json.svg
• packages/ui-components/style/icons/filetype/notebook.svg
• packages/ui-components/style/icons/jupyter/jupyter-favicon.svg
• packages/ui-components/style/icons/jupyter/jupyter.svg
• packages/ui-components/style/icons/jupyter/jupyterlab-wordmark.svg

Uses .jp-icon-contrast*:

• packages/ui-components/style/icons/filetype/r-kernel.svg
• packages/ui-components/style/icons/filetype/spreadsheet.svg
• packages/ui-components/style/icons/filetype/yaml.svg

Key observations

• Many classes (like .jp-icon4) aren't even being used at all by any icon.
• These classes are not being used contextually, i.e. .jp-icon-warn is not being used in "warning" icons, but is instead used to just color icons orange. Same with .jp-icon-contrast0 for coloring icons purple.

Open question

Can we remove some (if not all) of these CSS classes and replace them with ones that are directly related to the icon?

• Right now, a theme developer changing the color of .jp-icon-warn* would expect only warnings to be changed, when in fact it actually just changes the color of the Jupyter logo! This is very confusing.
• Instead of having several poorly-named and frequently unused .jp-icon* classes, we can instead create new classes for each special icon that requires its own color palette. For example, we add the .jp-icon-jupyter-logo class to the Jupyter SVG, and then apply our desired colors via that class. This way, theme developers don't accidentally end up changing the color of icons that are intended to have their own theme-independent color.

I personally fail to see a reason we should continue exposing so many .jp-icon* classes and variables that don't seem to be getting much usage. I think it's worth considering migrating away from this approach and have developers explicitly specify which icons they wish to override rather than forcing disparate icons to share the same CSS classes.

[fcollonval]

Thanks for tackling this @dlqqq

Can we remove some (if not all) of these CSS classes and replace them with ones that are directly related to the icon?

* Right now, a theme developer changing the color of `.jp-icon-warn*` would expect only warnings to be changed, when in fact it actually just changes the color of the Jupyter logo! This is very confusing.

* Instead of having several poorly-named and frequently unused `.jp-icon*` classes, we can instead create new classes for each special icon that requires its own color palette. For example, we add the `.jp-icon-jupyter-logo` class to the Jupyter SVG, and then apply our desired colors via that class. This way, theme developers don't accidentally end up changing the color of icons that are intended to have their own theme-independent color.

I'm 💯 to reduce the number of generic classes and to use more semantic names for specific icons.

I'm also wondering if we could not simply drop using a generic class for most icons and set fill or stroke to currentColor to simplify the handling of icon next to text (e.g. dealing with selectable icon); it brings the ease of web font icons.

[dlqqq]

I'm also wondering if we could not simply drop using a generic class for most icons and set fill or stroke to currentColor to simplify the handling of icon next to text (e.g. dealing with selectable icon); it brings the ease of web font icons.

@fcollonval Hm, I did some initial testing and it looks like the most obvious issue with your suggestion is that in most cases we don't want the icon to inherit its parent color. See what happens when I set the fill and stroke to currentColor:

The icons get set to pure black because that's the default color of text in light mode. We want the icon to be a different color while unselected and have the icon be the same color while selected.

Rather than attaching another class to each icon, I suggest that we attach a class to the parent and use a separate selector to target icon descendants of the parent instead. We can provide a generic .jp-icon-override class for elements that need inline icons to always match color. In icons.css:

.jp-color-override > .jp-icon-mono[fill] {
  fill: currentColor;
}

.jp-color-override > .jp-icon-mono[stroke] {
  stroke: currentColor;
}

.jp-color-override > .jp-icon-mono-inverse[fill] {
  opacity: 0.0;
}

.jp-color-override > .jp-icon-mono-inverse[stroke] {
  opacity: 0.0;
}

It's ambiguous as to how mono inverse color should be handled (e.g. the shell prompt of the terminal icon). I suggest making it transparent as above by default. If greater specificity is needed, developers can forego the .jp-color-override class and write their own overriding parent class to control both mono and mono inverse colors.

TL;DR: delegate responsibility of control to the parent, rather than trying to implement the functionality in the icon styles itself.

[dlqqq]

Another action item to add to the wider goal of eliminating excessive generic classes: as we have gathered the styles for monochromatic icons under the .jp-icon-mono and .jp-icon-mono-inverse selectors, we can actually get rid of .jp-icon-selectable as well from all the icons. I've tested this locally and can't notice any obvious differences. This will have to come after if/when we migrate away from the existing icon classes, since some icons (as mentioned above) still use the old .jp-iconX classes.

[dlqqq]

Small change that had to be made: it looks like the way we're currently rendering SVG icons means that we can't directly attach a class attribute to the root SVG element. The fix for this was pretty simple, just had to nest child elements under a new <g class="..." /> element.

[dlqqq]

List of CSS hacks being used that can be fixed with the .jp-icon-override class I suggested earlier in this thread (noticed by fixing UI regressions):

• packages/launcher-extension/style/base.css:12 (create launcher icon)
• packages/toc-extension/style/base.css:10 (TOC icon)
• packages/apputils/style/commandpalette.css:105 (command palette search icon)
• packages/ui-components/style/icons.css:{291,320} ("busy" unsaved changes icon beneath close tab icon)
• packages/debugger/style/icons.css:25 (debugger icon)

[fcollonval]

Thanks for all the tests and report

The icons get set to pure black because that's the default color of text in light mode. We want the icon to be a different color while unselected and have the icon be the same color while selected.

Thanks for trying.

Rather than attaching another class to each icon, I suggest that we attach a class to the parent and use a separate selector to target icon descendants of the parent instead. We can provide a generic .jp-icon-override class for elements that need inline icons to always match color. In icons.css:
It's ambiguous as to how mono inverse color should be handled (e.g. the shell prompt of the terminal icon). I suggest making it transparent as above by default. If greater specificity is needed, developers can forego the .jp-color-override class and write their own overriding parent class to control both mono and mono inverse colors.

As you demonstrate, if we are to use a single jp-icon-mono class, it becomes easy to switch to currentColor by using a parent selector. So I would not add the additional jp-icon-override and let developer switch the style where they see fit. So adding your above snippet in the documentation sounds good enough.

TL;DR: delegate responsibility of control to the parent, rather than trying to implement the functionality in the icon styles itself.

💯

Small change that had to be made: it looks like the way we're currently rendering SVG icons means that we can't directly attach a class attribute to the root SVG element. The fix for this was pretty simple, just had to nest child elements under a new element.

Did you try setting className instead of class? Class is never used in JS because of its ambiguity with the language keyword class.

List of CSS hacks being used that can be fixed with the .jp-icon-override class I suggested earlier in this thread

I don't get how introducing jp-icon-override will simplify cover those cases. Would you mind providing the code snippet?

Comment on the code change

You should not commit docs/source/developer/ui_components.rst as it is generated.

[dlqqq]

I would not add the additional jp-icon-override and let developer switch the style where they see fit. So adding your above snippet in the documentation sounds good enough.
I don't get how introducing jp-icon-override will simplify cover those cases.

Yeah, after thinking about this more, it does seem unnecessary as all it does is really just save a few lines and requires the developer to know that the class exists before using it. I think to keep our default CSS classes minimal, we should strike off this idea.

I can briefly explain the approach I was thinking of. The idea is to add the .jp-icon-override class to the parent element of the icon, and then set color: whatever on styles attached to the parent element. Thus we are delegating the responsibility of controlling icon color to the parent rendering the icon rather than the icon itself. But again, we can just have the component owners write the CSS selectors themselves.

TL;DR: the fix for these chained selector hacks is simply to add some class to the parent element that allows us to select icon children to apply style overrides.

You should not commit docs/source/developer/ui_components.rst as it is generated.

@fcollonval Are you sure about this? The docs/source/developer has other autogenerated files underneath it as well, so I followed the convention by committing them to the repo. 🤔

[dlqqq]

Reviewed all failing UI snapshots, and there are no remaining unintentional regressions.

[dlqqq]

please update snapshots

[ajbozarth]

I'll take a deep look at this as soon as I have time (prob not until next Monday though)

[fcollonval]

You should not commit docs/source/developer/ui_components.rst as it is generated.

@fcollonval Are you sure about this? The docs/source/developer has other autogenerated files underneath it as well, so I followed the convention by committing them to the repo. thinking

@dlqqq you are indeed closer to the solution - the script copying the file from ui-components package needs to be updated:

jupyterlab/packages/ui-components/docs/build.sh

Line 10 in 989683c

MONOREPO_DEVDOC=$(realpath $PKG_ROOT/../../docs/source/developer)

The path is not source/developer but source/extension. So the existing file should be modified and not brand new as for now.

[HaudinFlorence]

Hi, I am wondering if the logics of monochromatic icons could be extended to cases when the background elements are colored. Like on this example snapshot:

This may be useful for creating new themes.