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

Async expand computes incorrect scrollHeight leading to rows not displaying #904

Closed
mvaerle opened this issue Apr 23, 2014 · 4 comments
Closed

Comments

@mvaerle
Copy link

mvaerle commented Apr 23, 2014

This is related to #739. Thanks a lot for fixing that issue.

Unfortunately somehow this feature is not really usable anymore (using dgrid v0.3.14). The tests for the async expand are passing however when looking at the browser you can clearly see some rows are missing.

The test in question is intern/plugins/tree-expand-promise.js.

What happens is that at the end the assert is that there are 5 rows displaying. That assert is true, however only 3 rows are visible in the browser. When hovering the parent row that was just expanded ("Node 2") suddenly the missing rows appear.

This has something to do with 236 / 237 of dgrid/tree.js:

if(grid._expanded[row.id] && hasTransitionend){
    var scrollHeight = container.scrollHeight;
    container.style.height = scrollHeight ? scrollHeight + "px" : "auto";
}

Initially the scrollHeight of the container is set to the height of just the container row. When it is expanded the parent row doesn't update to the full height, the container under that does, but the parent node is still not high enough, which means that the other rows that are below it are not rendered, due to overflow:hidden being set.

As soon as the parent node is hovered the fixed height is removed from the node in the dom and the rows are displayed.

My current workaround is monkeypatching tree.js to make sure the code above is never reached, I'm not sure what implications that has though.

Thanks a lot for looking into this.

@kfranqueiro
Copy link
Member

I'm curious how you're going about reproducing this. It's worth noting that the promise returned from expand resolves once data is retrieved - it doesn't wait for the transition (if any) to finish. Could that be related to what you're seeing? Or are you seeing issues even in manual tree use?

@pmgbps
Copy link

pmgbps commented Apr 24, 2014

I'm seeing the issue as well. The problem I'm having is when I use nested trees.

If I have a tree set up like this:

A
-- B
---- C
---- D
---- E
---- F
-- G
---- H
---- I
---- J
---- K

I have all parent rows set with shouldExpand = true.

The initial query will come back with data for A, telling it to expand, which calls grid.store.getChildren(row.data, options). That request returns B and E, which both then auto exapnd. A's expand promise is then resolved, and the snippet of code in the first comment is run.

Since B has shouldExpand = true, the grid now goes and fetches B's children and puts them in. However, since A's promise has already resolved, it already sized the dgrid-tree-container for A's decendents, before its grandchildren (B's children) were rendered. This chops off some of B's child rows, with G and its children on top of them.

Perhaps the expand promise should be grouped with or dependent upon with any promises it's children may have?

Thanks.

@kfranqueiro
Copy link
Member

Can you try this again against dgrid master and see if it's resolved? I think this is the same issue as #874.

@kfranqueiro
Copy link
Member

Given no further response, I'm going to close this since I believe we've resolved it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants