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

[Feature Request] [VDialog] [Performance] Don't create ThemeProvider when dialog is lazy and not visible #8712

Closed
ascott18 opened this issue Aug 23, 2019 · 1 comment · Fixed by #8823, skyYaga/skdvin-webapp#22, skyYaga/skdvin-webapp#23 or anyulled/reactivewebflux#5
Assignees
Labels
C: VDialog VDialog C: VMenu VMenu performance The issue involves performance T: enhancement Functionality that enhances existing features

Comments

@ascott18
Copy link
Contributor

Problem to solve

VDialog components are creating a ThemeProvider even when the dialog is hidden.

This is an extra component allocation, and it also causes extra dependency subscriptions against $vuetify theme props. On a page that has a large number of lazy dialogs (in my case, a table with multiple button-activated dialogs on each row), this causes a significant performance hit due to vuejs/vue#10435

I can work around this by taking my dialogs out of my table and manually providing data and activating, but it would be nice if this wasn't needed.

Proposed solution

Include the ThemeProvider (and maybe even its containing div(s)) in the set of elements that aren't rendered when the dialog is hidden.

@ghost ghost added the S: triage label Aug 23, 2019
@KaelWD KaelWD added C: VDialog VDialog C: VMenu VMenu performance The issue involves performance T: enhancement Functionality that enhances existing features and removed S: triage labels Aug 24, 2019
@KaelWD KaelWD self-assigned this Sep 1, 2019
@KaelWD
Copy link
Member

KaelWD commented Sep 1, 2019

It's probably best to have a single external dialog anyway tbh.

KaelWD added a commit that referenced this issue Jan 31, 2020
* fix(VDialog): include ThemeProvider in the lazyContent

fixes #8712

* fix(VMenu): include ThemeProvider in the lazyContent

fixes #8712

* test: update snapshots

* fix(bootable): don't generate lazy content until it's needed

* refactor(bootable): avoid rendering content wrappers before boot

* test: use eager

* fix(detachable): wait for content to render before detaching

* test(VCarouselItem): remove snapshots

* refactor: split up render functions

* refactor: store content in variable

* fix(VDialog): render transition

* test: update snapshots
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment