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

tree expand and collapse #1069

Closed
jbomer opened this issue Jan 15, 2015 · 2 comments
Closed

tree expand and collapse #1069

jbomer opened this issue Jan 15, 2015 · 2 comments

Comments

@jbomer
Copy link

jbomer commented Jan 15, 2015

Sometime parent container don't resize on tree expant or collapse (with OnDemandGrid). This modification in Tree.expand() it seem perform properly:

if (hasTransitionend && !noTransition) {
//on.once(container, hasTransitionend, this._onTreeTransitionEnd);
on(container, hasTransitionend, this._onTreeTransitionEnd);
}

@zub2
Copy link

zub2 commented Jul 16, 2015

I have just been bitten by the same issue. It's occurring in dgrid v0.4.0 but also in master (as of a3577b6).

For the issue to manifest, there needs to be a node that has at least two levels of children. The behavior is as if the height of the expanded node (including its children) was determined once and upon subsequent folds/unfolds, the height stays fixed. This behavior breaks rendering when a child node is expanded/collapsed. This would explain the fix jbomer is proposing above. I can confirm that his fix works for me.

It can be reproduced using dgrid/test/Tree.html, in the first grid ("Lazy-loading tree grid, with store functionality") like this:

  1. expand "Africa"
  2. expand "Egypt"
  3. collapse "Africa"
  4. expand "Africa"
  5. collapse "Egypt"
    -> at this moment a gap between "Sudan" and "Asia" occurs; I'd expect "Asia" to move so that there is no gap

Similarly (after reloading):

  1. expand "Africa"
  2. collapse "Africa"
  3. expand "Africa"
  4. expand "Egypt"
    -> at this moment "Sudan" disappears; I'd expect "Asia" to move one row down so that "Sudan" stays visible.

This is in fact reproducible with all the test grids in test/Tree.html where there are at least 2 levels of children.

I tested this using Google Chrome 43.0.2357.134 (64-bit).

@bogool
Copy link

bogool commented Dec 10, 2015

This fix works fine when tree (or node) is initially collapsed by default.
I have the tree based on memory store, that is fully expanded:

shouldExpand: function(row, level, previouslyExpanded){
    return true;
}

In this case the fix doesn't work. So I improved it a little, and now it works for me.
Improved version of this fix may look like this:

//dgrid/Tree.js
//line 144
    if (hasTransitionend && !noTransition) {
        //on.once(container, hasTransitionend, this._onTreeTransitionEnd);
        on(container, hasTransitionend, this._onTreeTransitionEnd);
    }
    else {
        this._onTreeTransitionEnd.call(container);
        //fix for node expanded by default:
        if (hasTransitionend){
            on(container, hasTransitionend, this._onTreeTransitionEnd);
        }
    }

kriszyp added a commit to kriszyp/dgrid that referenced this issue Dec 11, 2015
container only gets created once, so we need to listen for more than the
first transitionend event, in order to remove the fixed height that breaks
scrolling, fixes dojo#1069
kfranqueiro pushed a commit that referenced this issue Dec 11, 2015
The container only gets created once, so we need to listen for more than the
first transitionend event, in order to remove the fixed height which could
interfere with inner parent expansion.

Additionally, the immediate call to `_onTreeTransitionEnd` is unnecessary,
since the no-transition path skips height adjustment and already handles
updating the display property elsewhere.

Thanks @jbomer, @bogool, and @kriszyp for poking at this and helping to
arrive at the solution.

(cherry picked from commit 1323e9f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants