rowHeight not refreshing while dragging

[svenadlung]

trafficstars

We are working with the rowHeight-function to give the nodes different heights depending on the nested level of each node. This is working well so far.

But we ran into problems while dragging a node. The calculated heights need to be recalculated while dragging, but they don't. We noticed this is working well with isVirtualized is false, but not if it's set to true.

Anyone got an idea?

[svenadlung]

To be more precise I think height is not re-set at all until toggling visibilities or removing and adding nodes. Even updating the state (changing sorting) is causing the issue.

Can anybody provide help please?

[jcalebj]

I came across the same problem as @svenadlung

I have rowHeight set to:

  rowHeight = ({ node }) => {
    console.log(node);
    if (!node.parent) {
      return 116;
    } else {
      return 46;
    }
  }

When isVirtualized = false, after initial load, I can see the rowHeight func firing in the console and correctly setting the height as I drag around a node, but when isVirtualized = true the rowHeight func does not fire at all (after initial load) when dragging the node around.

Any ideas?

[aviov]

Nice component:) and here is the same thing about rowHeight being invoked only at onVisibilityToggle, if isVirtualized={true}. @checklist was going to resolve this via custom NodeRendererDefault in issue #192 Does not seem to be shortcut solution. Would be cool to invoke rowHeight programmatically.

[jcalebj]

Thanks for the comment @asynchub! I think the issue lies outside of having a custom NodeRendererDefault. In react-sortable-tree.js it seems like the renderRow function's style prop is different when isVirtualized vs !isVirtualized. When isVirtualized is false, the style uses rowHeight func to calculate height (as seen here on line 684). However, when isVirtualized is true, the style does not seem to be using the rowHeight func I pass in. As seen here on line 666

[jcalebj]

So what if at this line there was:

style: {
            ...rowStyle,
            height:
              typeof rowHeight !== 'function'
                ? rowHeight
                : rowHeight({
                    index,
                    treeIndex: index,
                    node: row.node,
                    path: row.path,
                  }),
          },

[aviov]

nice hack @jcalebj 👍 instead of row.node and row.path apply rows[index].node and rows[index].path there. rowHeight invoked now, node height is changed, but next nodes are "not moved down" when node height button is toggled. setState does not affect other rows movement either before or after. experimented with transpiled code:

style: Object.assign({}, rowStyle, {
                                          height: typeof rowHeight !== 'function' ? rowHeight : rowHeight({
                                            index: index,
                                            treeIndex: index,
                                            node: rows[index].node,
                                            path: rows[index].path
                                          })
                                        }),

[aviov]

to ScrollZoneVirtualList, which is actually withScrolling from 'react-dnd-scrollzone' there lines like theese are passed: height={height} estimatedRowSize={ typeof rowHeight !== 'function' ? rowHeight : undefined } interesting, if they can affect another rows to move down on toggling the target row? need to check that later today.

[jcalebj]

@asynchub I spent the majority of the day yesterday trying to track down the reason for this behavior 😒 I traced it down through ScrollZoneVirtualList to react-virtualized and found this issue this morning. It seems that the second to last comment references this in the docs and may be the solution we've been looking for 😻 I'll report back with my results

[aviov]

thanks a lot for your find @jcalebj 👍 having same feeling to deal with react-virtualized hopefully you are right, that recomputeRowHeight can resolve this. will try this later in the evening as well :)

[svenadlung]

Yeah, I was thinking the issue is with react-virtualized. I didn't dig deeper so far. Did you progress your work?

[jcalebj]

Yeah @asynchub @svenadlung , I found a solution for resetting the height correctly after moving/dragging a node. To be clear, the issue is not with react-virtualized.

react-sortable-tree.js is missing a call to List.recomputeRowHeights(). See docs here and an example here.

Right now I have that call within moveNode function (which happens after drag is finished) which updates the row heights correctly after a move happens.

The rowHeights update while dragging is still not working though. I don't know where to call the recomputeRowHeights function to update the rowHeights while dragging. I've tried several different places in the lifecylce, but there's a point between a row of height X and a row of height Y that it sits between "slideRegion" of both rows, so it shifts back and forth from below to above the hovered row. You can see this behavior in the attached gif. I have a debounce in place to slow it down, else it happens so often you can't see it and it nearly crashes. You can see that if I leave that particular "middle ground" slide region and go lower or higher than the row, that it resolves and sets the correct rowHeight both when it's above and below the row with the larger rowHeight.

I can open up a PR on what I have that fixes the rowHeight issue after moving a node, but will need some more time and/or assistance resolving the problem while dragging (detailed above). What do you guys think?

[aviov]

hi @svenadlung have found how to invoke the function recomputeRowHeights inside ScrollZoneVirtualList. need to find out, where to call this function exactly to make clicked node height and offset position of next rows. working on this. any ideas @svenadlung, @jcalebj update:) just seen previous post from @jcalebj, thanks a lot, will study this.

[jcalebj]

@asynchub see my last comment for my findings

[aviov]

@jcalebj, have you seen this plunker https://plnkr.co/edit/lTFmKOQlJLIehL1LIY47?p=preview ? got it from 4th post by @bvaughn from this issue: https://github.com/bvaughn/react-virtualized/issues/307 there is one more recomputeRowHeights call and that is called in <VirtualScroll> with comment: // Check if any heights have been recalculated before the ref was set In your case: have you tried to call recomputeRowHeights from both: <ScrollZoneVirtualList> and from it's rowRenderer - 'renderRow' props? renderRow is actually rst's prop, which you can call from component, that you return by renderRow function.

[jcalebj]

@asynchub I'll test that out today

[aviov]

@jcalebj, thanks a lot for your fix, it helps 👍 from there, got working solution for challenge with row height on tree node click: ref line is the same and at the same place as you have: ref={el => { this.scrollZoneVirtualListComponent = el }} and the recomputeRowHeights call is just before you return from render, like this: if (isVirtualized) { this.scrollZoneVirtualListComponent && this.scrollZoneVirtualListComponent.wrappedInstance.recomputeRowHeights(); } and of course, style in rowRenderer at if (isVirtualized): style: Object.assign({}, rowStyle, { height: typeof rowHeight !== 'function' ? rowHeight : rowHeight({ index: index, treeIndex: index, node: rows[index].node, path: rows[index].path }) }), from here, it is the point attempt passing these things across components of both react-sortable-tree and react-virtualized and limiting calls:rowHeight and recomputeRowHeights accordingly, passing exact indexes of tree nodes. by the way, have you noticed?:, that react-sortable-tree has a good prop reactVirtualizedListProps, worth of trying to pass things to the List / Grid of react-virtualized how is your challenge with moveNode so far?

[jcalebj]

@asynchub I believe if you call recomputeRowHeights within the render that that's an anti-pattern. Are you not getting warnings in your console from that?

To be clear, did you get a working solution in place? Or is what you wrote above a work in progress?

I've solved the problem of having incorrect row heights after a move, it's in PR 269. The remaining problem is to get the rowHeights to updated correctly while dragging/moving. Have you had any progress on that front?

[jcalebj]

@wuweiweiwu Is there a reason for labelling this issue 'enhancement' and not 'bug'? This is a known problem with current documented functionality. Re-sorting a node when isVirtualized and rowHeight set to a func resulting in variable heights does not work correctly.

[aviov]

@jcalebj, there is warning about rener() purity: Warning: Cannot update during an existing state transition (such as within render or another component's constructor). agree, this is antipattern. this is just solution in place for now, to go on with other things :), and it works for dragging/moving feature too. Definitely, component needs responsive row heights onMove, onDrag and onClick at node.

[svenadlung]

Any news on this? In my understanding there is a PR which is fixing this on drop but not while dragging. Anyways, the PR is not ready to merge? Please correct me if I am wrong! It would be great if someone finds a solution even while dragging (besides the hack to put it in the render-Function ;)).

[wuweiweiwu]

@svenadlung seems that @jcalebj is working on a PR right now. But please feel free to investigate and/or open a PR as well :)

[jcalebj]

Hi @svenadlung, I opened a PR that fixes one part of the problem (on drop, but not while dragging). But the PR is lacking some test coverage at the moment. I don't have the bandwidth right now to tackle the 'while dragging' issue (I sunk a good bit of time into it to no avail), but it seems like a fix for both problems is wanted in a single PR before they'll merge it in.

[developerpoidi]

Are there any updates regarding this issue? I am calculating the height based on the text so I can have multiple lines, but with this bug my list is not really usable.

Btw. you did a great job with this component!

[developerpoidi]

There also seams to be a problem with the height recalculation if you move an item between a parent-child relation (where the blue arrow is shown and moves the item below it).

The problem occurs also after using the change proposed in https://github.com/frontend-collective/react-sortable-tree/pull/269 (which at least fixes the calculation after the move, if not the mentioned case happens)

[xgzl87]

@developerpoidi Have you solved your problem?I had a similar problem.

[dcporter44]

i hacked around this by forcing calling recomputeRowHeights on drag change. On drag I set an interval at 250ms to recompute row heights. When stopped dragging, I clear the interval. I originally didnt use an interval and called recomputeRowHeights within the onChange prop, but that caused me to call recomputeRowHeights hundreds of times (for every pixel i moved my cursor while dragging). Feel free to change 250ms to a lower number. But calling recomputeRowHeights too often will hurt performance. This method isnt perfect, but it does the job:

<SortableTree>
...
reactVirtualizedListProps={{
   autoHeight: true,
   ref: ref => this.list = ref;
}}
onDragStateChanged={({ isDragging }) => {
     if (this.list) {
       const recompute = () => {
          this.list.wrappedInstance.recomputeRowHeights();
        };
        if (isDragging) {
          this.dragInterval = setInterval(recompute, 250);
        } else {
          clearInterval(this.dragInterval);
          this.list.wrappedInstance.recomputeRowHeights();
        }
    }
}}
</SortableTree>

[dilvane]

i hacked around this by forcing calling recomputeRowHeights on drag change. On drag I set an interval at 250ms to recompute row heights. When stopped dragging, I clear the interval. I originally didnt use an interval and called recomputeRowHeights within the onChange prop, but that caused me to call recomputeRowHeights hundreds of times (for every pixel i moved my cursor while dragging). Feel free to change 250ms to a lower number. But calling recomputeRowHeights too often will hurt performance. This method isnt perfect, but it does the job:

<SortableTree>
...
reactVirtualizedListProps={{
   autoHeight: true,
   ref: ref => this.list = ref;
}}
onDragStateChanged={({ isDragging }) => {
     if (this.list) {
       const recompute = () => {
          this.list.wrappedInstance.recomputeRowHeights();
        };
        if (isDragging) {
          this.dragInterval = setInterval(recompute, 250);
        } else {
          clearInterval(this.dragInterval);
          this.list.wrappedInstance.recomputeRowHeights();
        }
    }
}}
</SortableTree>

Works fine @dcporter44, I had to use this.list.wrappedInstance.current.recomputeRowHeights(); instead, probably some react-virtualized update. Thanks.

[mx2323]

i was able to get the correct behavior for refreshing row height while dragging on my own local fork of react-sortable-tree. the changes probably are not usable by anyone else, nor are they the 'right' way to do things, but i'm happy enough with the resulting behavior that it's usable for us for now.

the issue is that if you modify the code to call recomputeRowHeights() more frequently like everyone is doing above, a larger and smaller row will end up toggling in the tree infinitely while you hover over a fixed location with your mouse pointer. by calling recompute row heights every time the state is changed on hover() in react-sortable-tree.js, it just exposes a problem in the library where moving variable height rows around with the current logic can cause infinite toggling of the draggedNode location.

for instance, this happens whenever a smaller height node A is hovered over the bottom portion of a larger drop target node B, A is inserted in the tree above B by the library, but on redraw, if the currently hovering node A is over B, it will try to move A down and B up. But, then on the next redraw, it will detect the node is over B, and then will try to again insert A above B, which leaves you in that infinite loop of redrawing because you are recomputing row heights

i made a change to the library in dnd-manager.js so that if it is hovering over a lower portion over a component, it will try to insert it after instead of always trying to place it above the target node.

if people are interested, i can work on a diff. it's probably not the the right way to do this, but appears to be working good enough for our purposes.

[svenadlung]

i was able to get the correct behavior for refreshing row height while dragging on my own local fork of react-sortable-tree. the changes probably are not usable by anyone else, nor are they the 'right' way to do things, but i'm happy enough with the resulting behavior that it's usable for us for now.

the issue is that if you modify the code to call recomputeRowHeights() more frequently like everyone is doing above, a larger and smaller row will end up toggling in the tree infinitely while you hover over a fixed location with your mouse pointer. by calling recompute row heights every time the state is changed on hover() in react-sortable-tree.js, it just exposes a problem in the library where moving variable height rows around with the current logic can cause infinite toggling of the draggedNode location.

for instance, this happens whenever a smaller height node A is hovered over the bottom portion of a larger drop target node B, A is inserted in the tree above B by the library, but on redraw, if the currently hovering node A is over B, it will try to move A down and B up. But, then on the next redraw, it will detect the node is over B, and then will try to again insert A above B, which leaves you in that infinite loop of redrawing because you are recomputing row heights

i made a change to the library in dnd-manager.js so that if it is hovering over a lower portion over a component, it will try to insert it after instead of always trying to place it above the target node.

if people are interested, i can work on a diff. it's probably not the the right way to do this, but appears to be working good enough for our purposes.

Exactly. We were facing the same problem, thats why we ended up only using onMoveNode and recompute heights only after instead of doing it while dragging.

It would be amazing to see your sources. Wasn't able to find your fork :(

[ZirkleTsing]

Yeah @asynchub @svenadlung , I found a solution for resetting the height correctly after moving/dragging a node. To be clear, the issue is not with react-virtualized.

react-sortable-tree.js is missing a call to List.recomputeRowHeights(). See docs here and an example here.

Right now I have that call within moveNode function (which happens after drag is finished) which updates the row heights correctly after a move happens.

The rowHeights update while dragging is still not working though. I don't know where to call the recomputeRowHeights function to update the rowHeights while dragging. I've tried several different places in the lifecylce, but there's a point between a row of height X and a row of height Y that it sits between "slideRegion" of both rows, so it shifts back and forth from below to above the hovered row. You can see this behavior in the attached gif. I have a debounce in place to slow it down, else it happens so often you can't see it and it nearly crashes. You can see that if I leave that particular "middle ground" slide region and go lower or higher than the row, that it resolves and sets the correct rowHeight both when it's above and below the row with the larger rowHeight.

I can open up a PR on what I have that fixes the rowHeight issue after moving a node, but will need some more time and/or assistance resolving the problem while dragging (detailed above). What do you guys think?

How did you use debounce to solve the problem of moving nodes back and forth?