Use Vuetify's lazy attribute to delay mounting of DOM nodes#3483
Use Vuetify's lazy attribute to delay mounting of DOM nodes#3483rtibbles merged 2 commits intolearningequality:unstablefrom
Conversation
|
@MisRob I tried profiling this like you but I couldn't quite get the views you had looked at for tracking memory usage. I'm curious of your thoughts on this. The |
|
@bjester That's great. I think this could be one of the things having an impact on #3367. I replied to your message on Slack in regards to performance charts. I'd be curious to see whether we can observe something on before/after performance profiles, but even without them, I think this will be a good direction. |
!!! |
| :query="{ ids: [userId] }" | ||
| /> | ||
| <VMenu offsetY> | ||
| <Menu> |
There was a problem hiding this comment.
I can see in the notes that you updated <Menu /> to use the lazy setting (and can see the prop added there). Is lazy not an option for VMenu which is why you switched just to <Menu/>? Or is this for another reason?
There was a problem hiding this comment.
lazy is indeed an option for VMenu. Our Menu inherits VMenu, and so I updated this since it also defaults the offsetY attribute-- I'm sure we'll want that 99% of the time for our menus anyways. There were many usages of Menu, so by adding lazy there, I took advantage of that abstraction layer requiring a single change. We can do the same when we migrate off Vuetify and to KDS for things like this.
marcellamaki
left a comment
There was a problem hiding this comment.
looks fine to me - previous comments were mostly just curiosity, no outstanding concerns.
Summary
Description of the change(s) you made
<VTooltip>usages to uselazysetting which delays their mount to the DOM until they need to be<Menu>component to also defaultlazysettingManual verification steps performed
Screenshots (if applicable)
Below are screenshots of the Chrome performance monitor for the same channel on production vs locally with these changes. I left the channel open at the root to get a baseline on each. You can see there are less DOM nodes in the Elements tab.
Does this introduce any tech-debt items?
Reviewer guidance
How can a reviewer test these changes?
Are there any risky areas that deserve extra testing?
References
Comments
Contributor's Checklist
PR process:
CHANGELOGlabel been added to this PR. Note: items with this label will be added to the CHANGELOG at a later timedocslabel has been added if this introduces a change that needs to be updated in the user docs?requirements.txtfiles also included in this PRStudio-specifc:
notranslateclass been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages,components, andlayoutsdirectories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarnandpip)