Added optional className param to JLIcon constructor

[ajbozarth]

References

This follows on @telamonian work with #7700

Code changes

Currently JLIcons are initialized with a generated className based on the name. This means all classNames are prepended with jp-, but this precludes the idea of extensions creating their own JLIcons with project unique CSS selectors.

User-facing changes

N/a

Backwards-incompatible changes

N/A

[jupyterlab-dev-mode]

Thanks for making a pull request to JupyterLab!

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

[ajbozarth]

@telamonian I'm not 100% sure this addition fits with your design and usage of JLIcon. While I was working on moving our extensions onto 2.0b3 I switched to using JLIcon. I found that I had to change our icon's css to jp- instead of our own to make it work and felt this could be easily addressed.

[blink1073]

@telamonian, thoughts on this change?

[telamonian]

I'd prefer not to have this in (sorry, @ajbozarth!) The basic idea is that an icon having both a .name and a .className would be redundant, so that's why there's no public .className property on the LabIcon instances (the name JLIcon was changed to => LabIcon as part of #7767).

Instead, there's a private ._className prop, that, as you mentioned, is generated with a leading "jp-". This was being used as part of an automagic backwards compatibility shim, but that's been almost entirely removed as of #7767. As soon as I figure out how to remove the last couple of uses of the static LabIcon.UNSTABLE_getReact method, I plan on removing the private ._className prop entirely.

[telamonian]

If you do need a CSS selector for one of the new icons, my advice is:

• don't use CSS directly for styling the new icons, instead pass style props into the element or react methods directly.

• if you really do need a CSS selector, the use the icon dataset property (which will be set to the icon's name): [data-icon='some-icon-name']

[telamonian]

I found that I had to change our icon's css to jp- instead of our own to make it work

@ajbozarth Where did you run into trouble, exactly? Even before #7767, the ._className property was never used in any way while creating the DOM nodes associated with an icon (eg, the className attribute of icon nodes was NOT being set to the ._className prop of the corresponding LabIcon instance).

[ajbozarth]

@telamonian I took look through #7767 and I believe I'll just have to wait for you to finish and update my usage to match the final version. I'm pretty sure we're leveraging some of the remaining usage of LabIcon.UNSTABLE_getReact. We're adding a command with a iconClass param that in this case has to be jp-<IconName>Icon for the JLIcon instance I made with name icon-name. I would assume this will probably eventually change as part of removing LabIcon.UNSTABLE_getReact. I will close this, but feel free to comment any more clarifying info here if you think it will help (we have a 2.0 dev branch for our extensions that we're updating regularly to work with the latest beta)