progress on icons: added inline svg icon support to toolbar buttons

[telamonian]

References

followup to #6034

commit 0d59934 included some related follow-up to #7182:
fixes: #7189
fixes: jupyterlab/jupyterlab-git#411
fixes: QuantStack/jupyterlab-drawio#45

Code changes

Toolbar buttons now support inline SVG icons. Here's how would add a new toolbar button icon to Jlab core, using a foo-bar.svg file:

• copy foo-bar.svg to ui-components/style/icons/toolbar
• When you create a new ToolbarButton, enter the icon name in the iconClassName parameter of the constructor. Given an icon file named foo-bar.svg, either of the following will be valid:

return new ToolbarButton({
      iconClassName: 'foo-bar',
      ...
    });

or

return new ToolbarButton({
      iconClassName: 'jp-FooBarIcon',
      ...
    });

I also improved/unified fallback to old icon handling behavior in case of missing icon.

User-facing changes

Adds (optionally themeable) inline svg icons for toolbars.

Backwards-incompatible changes

[jupyterlab-dev-mode]

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link:

[telamonian]

This is now ready to go

[jasongrout]

Just checking - is this (should this be) backwards compatible with 1.0?

[telamonian]

Just checking - is this (should this be) backwards compatible with 1.0?

It definitely should be. I am sure that this PR doesn't regress anything in terms of the fixes made in #7182

[telamonian]

This PR now also includes some followup to #7182 in the form of more workarounds to deal with newly discovered more and different issues arising from Jlab 1.0.x installations bringing in Jlab 1.1.x packages when rebuilding against extensions.

[telamonian]

@jasongrout Can you please take a look and sign off on this PR? It should fix the remaining reported UI bugs that were caused by package version mismatches.

[jasongrout]

It's in my queue for today :).

[jasongrout]

Depending on how we define our public API, this could be seen as a breaking change for some packages, in that it moves CSS classes and variables from one package to another. The end result is JupyterLab as a whole is not incompatible, but individual packages are, if we consider the CSS classes/variables part of a package's public API.

Still, I think fixing the breakage we are seeing in 1.0.2 is worth merging this. I think even more we need a really clear story about exactly what we are promising when we say we are backwards compatible.

[jasongrout]

@blink1073 - if you have some time, I'd love to have a second review of this before backporting to 1.1.x.

[telamonian]

we need a really clear story about exactly what we are promising when we say we are backwards compatible.

I think that's a large part of what caused the bugs fixed in #7182 and this PR. Like you say, there seem to be two types of backwards compatibility that we're having to deal with here:

1. Compatibility wrt Jupyterlab as a whole
2. Compatibility wrt individual packages in @jupyterlab

When I wrote the icon handling code in #6034 in the first place, I tried to make it backwards compatible in the sense of 1. I think that part I was successful at. I wasn't even thinking about 2.

We shouldn't need to consider the backwards compatibility of a core package wrt other core packages (I suppose we're stuck doing that until 2.0.0, in any case). So I think all of the tricky cases that we need to consider are for non-Jlab applications that depend on a subset of the core packages.

For example, for the sake of non-Jlab applications should we add "resolutions" to all of the core packages to ensure against Jlab package version mismatch?

[jasongrout]

For example, for the sake of non-Jlab applications should we add "resolutions" to all of the core packages to ensure against Jlab package version mismatch?

I think a more acceptable solution would be to change all of our inter-dependencies to ~ instead of ^. But at that point, are we really following semver anymore?

[jasongrout]

(resolutions won't work because they are ignored except at the top level, IIRC. They work in JLab because we control the top-level compilation)

[vidartf]

We shouldn't need to consider the backwards compatibility of a core package wrt other core packages

You might get away with this for extensions, but please do not do this for library packages. One of the core principles for lab has been to be more modular so that others can reuse part of the code, and to do it safely by following semver.

[vidartf]

Also, there might be custom lab apps (e.g. phoila) that only use a subset of the core extensions (or replacing a core extension with a custom version), so reshuffling of features can quickly break those.

[blink1073]

LGTM. As part of this release I'd like to test against jlab 1.0 + some pinned extensions (which will be added to the release script).

[jasongrout]

Also fixes #7221

[jasongrout]

@meeseeksdev backport to 1.1.x

[jasongrout]

@meeseeksdev backport to 1.x

[jasongrout]

@meeseeksdev backport to 1.1.x

[lumberbot-app]

Something went wrong ... Please have a look at my logs.