New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
progress on icons: added inline svg icon support to toolbar buttons #7192
progress on icons: added inline svg icon support to toolbar buttons #7192
Conversation
also added some docstrings to the icon-related interfaces
Thanks for making a pull request to JupyterLab! To try out this branch on binder, follow this link: |
This is now ready to go |
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 |
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 |
@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. |
It's in my queue for today :). |
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. |
@blink1073 - if you have some time, I'd love to have a second review of this before backporting to 1.1.x. |
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:
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 For example, for the sake of non-Jlab applications should we add |
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? |
(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) |
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. |
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. |
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). |
Also fixes #7221 |
@meeseeksdev backport to 1.1.x |
… support to toolbar buttons
@meeseeksdev backport to 1.x |
… support to toolbar buttons
…2-on-1.x Backport PR #7192 on branch 1.x (progress on icons: added inline svg icon support to toolbar buttons)
@meeseeksdev backport to 1.1.x |
Something went wrong ... Please have a look at my logs. |
… support to toolbar buttons
… support to toolbar buttons (cherry picked from commit 0a989e6)
Backport PR #7192: progress on icons: added inline svg icon support to toolbar buttons
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:foo-bar.svg
toui-components/style/icons/toolbar
ToolbarButton
, enter the icon name in theiconClassName
parameter of the constructor. Given an icon file namedfoo-bar.svg
, either of the following will be valid:or
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