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

[Enhancement] Allow disable transition for VDataTable expand row #4764

Closed
pwang2 opened this issue Jul 31, 2018 · 11 comments
Closed

[Enhancement] Allow disable transition for VDataTable expand row #4764

pwang2 opened this issue Jul 31, 2018 · 11 comments
Assignees
Labels
C: VDataTable VDatatable T: enhancement Functionality that enhances existing features

Comments

@pwang2
Copy link

pwang2 commented Jul 31, 2018

What problem does this feature solve?

As requested here #2890, expand all is quite helpful to implement some treeview rows.

The current transition of expand rows makes workaround solution like https://codepen.io/anon/pen/aEJeVo perform very slow. I saw 300ms Recalculate Style in performance profiler.
image

What is your proposed solution?

Expose the transition as an props to opt in by user.

@KaelWD KaelWD added the wontfix The issue is expected and will not be fixed label Jul 31, 2018
@KaelWD
Copy link
Member

KaelWD commented Jul 31, 2018

Use css for this.

Also, I'm not seeing any performance issues in your example.

@KaelWD KaelWD closed this as completed Jul 31, 2018
@pwang2
Copy link
Author

pwang2 commented Aug 1, 2018

@KaelWD , Thanks for replying. Could you help clarify what do you mean by use css for this?

In my case for a nested table with 200 rows(maximum 3 levels, 20 columns), use dropdown select to re-grouping the rows takes 500ms to display the transitions.
image

in the code here,

setTimeout(() => (el.style.height = `${el.scrollHeight}px`), 100)
. scrollHeight access will reflow the whole DOM again and again. This is a huge performance issue even in a not so extreme scenario.

ref: https://gist.github.com/paulirish/5d52fb081b3570c81e3a

@KaelWD KaelWD reopened this Aug 1, 2018
@KaelWD KaelWD added the pending review The issue is still pending disposition label Aug 1, 2018
@pwang2
Copy link
Author

pwang2 commented Aug 1, 2018

To be clear, single row expand will not be an issue. The bottleneck comes with the expandAll workaround code here https://codepen.io/anon/pen/aEJeVo.

  for (let i = 0; i < this.items.length; i += 1) {
      const item = this.items[i];
      this.$set(this.$refs.dTable.expanded, item.name, true)
    }

image

@pwang2
Copy link
Author

pwang2 commented Aug 1, 2018

More information here FYI.

Besides vuetify's transition, vue's global update lifecyle method also adds a unconditional getTransitionInfo getComputedStyle hard query for all browser better than IE9 which aggravated the situation here. In short, we can not overburden it by adding transition too much element here.

https://github.com/vuejs/vue/blob/b7445a2b945dcded287601ace8e711ab5cf35ab5/src/platforms/web/runtime/transition-util.js#L124

image

@husayt
Copy link

husayt commented Aug 16, 2018

This is affecting us too, is there any workaround for time being?

@jacekkarczmarczyk
Copy link
Member

@pwang2 expand transition has been refactored in 1.3.10, is the issue still valid?

@KaelWD
Copy link
Member

KaelWD commented Dec 4, 2018

New codepens with more rows:
0.17: https://codepen.io/anon/pen/GwLMEX?editors=1010
1.3: https://codepen.io/anon/pen/VVNMzX?editors=1010

Seems the new transition is even slower, because it has to reflow the page twice for each element. I wonder if we could batch the reflows with requestAnimationFrame somehow so it only has to be done once for the whole page.

@KaelWD KaelWD added T: enhancement Functionality that enhances existing features and removed pending review The issue is still pending disposition wontfix The issue is expected and will not be fixed labels Dec 4, 2018
@KaelWD KaelWD self-assigned this Dec 4, 2018
@MajesticPotatoe MajesticPotatoe changed the title Allow disable transition for VDataTable expand row [Enhancement] Allow disable transition for VDataTable expand row Dec 7, 2018
@pwang2
Copy link
Author

pwang2 commented Dec 31, 2018

@jacekkarczmarczyk, sorry for the so late response. I quick checked the code and it seems the performance issue is still there.
@KaelWD , please let me know if you need any help from my side. I finally have some change to working on a related project again.

@MajesticPotatoe MajesticPotatoe added the C: VDataTable VDatatable label Apr 10, 2019
@amanbolat
Copy link

I have same problem. My application have VTable with 100 rows. Even expanding a single row will create some lag.

@AakifMushtaq
Copy link

@amanbolat This is what I did to get rid of the transition because we didn't really need that effect. Not sure if this is the right solution but it worked for me.
.v-datatable__expand-content { transition: unset !important; }

@jacekkarczmarczyk
Copy link
Member

v-data-table doesn't have transition on expanded row in 2.x, see #8197

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: VDataTable VDatatable T: enhancement Functionality that enhances existing features
Projects
None yet
Development

No branches or pull requests

7 participants