-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Removed build step to add new icon classes (#10906) #10908
base: main
Are you sure you want to change the base?
Conversation
Thanks for making a pull request to JupyterLab! To try out this branch on binder, follow this link: |
Thanks for making it a separate PR. This definitely needs to be discuss at a developer meeting as it will impact extensions. |
I believe this should not remove any classes already added to |
I acknowledge that. But for homogeneity reasons, I would remove the |
This is definitely a breaking change w.r.t. 3rd party extensions. The last time I checked (admittedly a year ago), more extensions were using our css icons than were using LabIcon. So the removal of said css icons can go in for v4.0, but if we're going to yet again break a bunch of extensions I'd prefer it to be for something more significant. In any case, there's some argument to be made that, rather than remove them, we should simply un-deprecate the css icons. Since I originally added the deprecation mark I've come to see that there are things that css icons can do that LabIcon cannot. The issue is discussed in detail in #8237, which is about some possible ways to improve icon handling. In short, css icons have two advantages:
At the moment, there are a few possible use cases for LabIcon will either have unacceptable performance or require ugly hacks (see everything under packages/ui-components/src/icon/widgets). As discussed in #8237, there are some alternative icon rendering techniques that could confer similar advantages on LabIcon. Ideally we'd tackle this first before deeply considering removing css icons |
References
This PR fixes #10906
Code changes
deprecated.css
fileUser-facing changes
None
Backwards-incompatible changes
None