Skip to content
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

Post 2.0 improvements to LabIcon: work with all svg loaders, improve performance, fix issue with menus from extensions #8125

Merged
merged 13 commits into from Mar 30, 2020

Conversation

telamonian
Copy link
Member

@telamonian telamonian commented Mar 29, 2020

References

jupyter/nbdime#512

Some improvements to LabIcon.

@vidartf With these changes you should be able to use LabIcon/@jupyterlab/ui-components in just about any project regardless of how svg loading is configured.

Code changes

  • LabIcon should now work with all commonly-used svg loaders/loading schemes

    • previously, LabIcon expected svgs to be loaded using a raw loader (such as Webpack's raw-loader) that directly loaded the raw data from an .svg file as a string
    • accomplished by shimming the input svgstr
  • 15-20x speedup of icon creation (on the javascript side)

    • accomplished by caching the intermediates produced while parsing LabIcon.svgstr into actual svg nodes
    • results in an overall 2x speedup of icon creation + render
    • profiling of creating/rendering ~1500 filebrowser items
      • old code:

        image

      • new code (compare cloneNode to resolveSvg above):

        image

  • Fix icons in menus added to main menubar by extensions, and ensure that correct renderer is used for all submenus

    • done via fixes to the implementation of MenuSvg

User-facing changes

  • improved performance related to icons (especially when opening dirs with 1000+ files in the filebrowser)

  • icons should now work correctly in menus added to main menubar and in all submenus (including context submenus)

Backwards-incompatible changes

None

@jupyterlab-dev-mode
Copy link

@jupyterlab-dev-mode jupyterlab-dev-mode bot commented Mar 29, 2020

Thanks for making a pull request to JupyterLab!

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

telamonian added 2 commits Mar 30, 2020
- unlike previous approach (setting the renderer for the context menu), ContextMenuSvg ensures that all context submenus use the appropriate renderer
- will correctly override menu renderer regards of the order in which the submenus get added to/inserted in one another
@telamonian telamonian changed the title [WIP] Post 2.0 improvements to LabIcon: work with all svg loaders, improve performance, fix issue with menus from extensions Post 2.0 improvements to LabIcon: work with all svg loaders, improve performance, fix issue with menus from extensions Mar 30, 2020
@telamonian
Copy link
Member Author

@telamonian telamonian commented Mar 30, 2020

This PR is now ready for review.

I already went through and checked everything, but ideally whoever reviews this will build it locally and double check that there's no regressions in the menu icons.

@saulshanabrook
Copy link
Member

@saulshanabrook saulshanabrook commented Mar 30, 2020

The summary from this meeting that for 3.0 we should fix some of these things upstream in lumino, but for now we can fix them here without breaking changes.

@saulshanabrook saulshanabrook merged commit a73cf0c into jupyterlab:master Mar 30, 2020
45 of 49 checks passed
@lock lock bot added the status:resolved-locked label May 5, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug enhancement pkg:application pkg:mainmenu pkg:ui-components status:resolved-locked tag:Performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants