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

Improve v-icon performance #16511

Merged
merged 2 commits into from Nov 28, 2022
Merged

Improve v-icon performance #16511

merged 2 commits into from Nov 28, 2022

Conversation

azrikahar
Copy link
Contributor

Description

Currently this method is being called everytime a <v-icon> component is mounted:

link to library.add(...iconDefinitions) in Font Awesome's docs

This in turn causes some performance hit as <v-icon> is being used throughout the app. However the most noticable impact is the <select-icon> interface, where now that same method is being ran for every icon in the whole dropdown list, which was why the dropdown takes a while to appear.

Changes

  • Moved the library.add(fab) call into the <social-icon> so that it will only be called when using social icons.

  • Turned socialIcons and components into a separate imported file (I assume this way they will only be declared once? Not 100% sure about this) which felt like it helped as well, but not sure if it's placebo.

  • Tweaked filteredIcons computed property in <select-icon> a little bit.

Before

(gray circle to indicate mouse click to see how long does it take to appear/disappear)

PowerToys_Djg2uWDLfF.mp4

After

There's still a tad delay when it's dissappearing due to Vue unmounting the icons in the dropdown/v-menu (still room for improvement here), but it is appearing near instantly now.

PowerToys_gHsKnvupCY.mp4

Type of Change

  • Bugfix
  • Improvement
  • New Feature
  • Refactor / codestyle updates
  • Other, please describe:

Requirements Checklist

  • New / updated tests are included
  • All tests are passing locally
  • Performed a self-review of the submitted code

If adding a new feature:

  • Documentation was added/updated. PR:

Copy link
Member

@br41nslug br41nslug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Im seeing a noticeable performance improvement compared to main.

@rijkvanzanten rijkvanzanten merged commit d1483b1 into main Nov 28, 2022
@rijkvanzanten rijkvanzanten deleted the v-icon-performance branch November 28, 2022 19:39
@rijkvanzanten rijkvanzanten added this to the Next Release milestone Nov 28, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants