vis-timeline icon indicating copy to clipboard operation
vis-timeline copied to clipboard

Vertical scroll position should persist after setGroups call

Open kodemi opened this issue 4 years ago • 8 comments

Hi! I receive data for Timeline dynamically and have to call setItems and setGroups every time I get new data. But setGroups resets vertical scroll position (I set the parameter height in options). You can see it here: https://codesandbox.io/s/vis-timeline-vertical-scroll-reset-bmoc1

kodemi avatar Nov 07 '19 09:11 kodemi

Try doing this:

document.getElementById("update-button").onclick = () => {
  const scrollTop = document.getElementsByClassName("vis-vertical-scroll")[0].scrollTop;
  timeline.setGroups(groups);
  setTimeout(() => {
    document.getElementsByClassName("vis-vertical-scroll")[0].scrollTop = scrollTop ;
  }, 0);
};

If this isn't good enough for what you're looking for, feel free to reopen the issue

yotamberk avatar Nov 09 '19 20:11 yotamberk

Hi! I tried to set timeout and restore scrollTop, but it leads to flicker. I have updated codesandbox example to demonstrate it.

kodemi avatar Nov 09 '19 21:11 kodemi

Should be reopened but I have no rights to do it.

kodemi avatar Nov 12 '19 10:11 kodemi

Hi! I tried to set timeout and restore scrollTop, but it leads to flicker. I have updated codesandbox example to demonstrate it.

vis-timeline

kodemi avatar Nov 17 '19 14:11 kodemi

I found that the problem appeared in v6.0.0. Comparing v6.0.0 with v5.1.0 and digging through code I found traces of problem:

  1. setGroups call
  2. Core.js _redraw() call and redraw of ItemSet
  3. ItemSet redraws every group to calculate the sum of the height of the groups.
  4. Group during redraw sets it's height to 0 and ItemSet sets it's height prop and frame height style to minHeight
  5. _redraw() calls _updateScrollTop() and because this.props.center.height = 30 (minHeight) resets scrollTop to 0.

So group is failed to calculate its height in _calculateHeight because this.visibleItems = [] and this.props.label.height = 0. this.props.label.height will be updated to actual value only in next steps, after height calculation.

Quick fix is preset this.props.label in redraw() function in Group.js:

redraw(range, margin, forceRestack, returnQueue) {
    let resized = false;
    const lastIsVisible = this.isVisible;
    let height;
    const queue = [
      () => {
        forceRestack = this._didMarkerHeightChange.call(this) || forceRestack;
      },
      
      // recalculate the height of the subgroups
      this._updateSubGroupHeights.bind(this, margin),

      // calculate actual size and position
      this._calculateGroupSizeAndPosition.bind(this),

      () => {this._didResize.bind(this)(resized, this.height)}, // <----- adding this function call

      () => {
        this.isVisible = this._isGroupVisible.bind(this)(range, margin);
      },
...

And one of three:

  1. comment these lines in _updateItemsInRange:
_updateItemsInRange(orderedItems, oldVisibleItems, range) {
   const visibleItems = [];
    const visibleItemsLookup = {}; // we keep this to quickly look up if an item already exists in the list without using indexOf on visibleItems

    //if (!this.isVisible && this.groupId != ReservedGroupIds.BACKGROUND) {
    //  for (let i = 0; i < oldVisibleItems.length; i++) {
    //    var item = oldVisibleItems[i];
    //    if (item.displayed) item.hide();
    //  }
    //  return visibleItems;
    //} 

    const interval = (range.end - range.start) / 4;
    ...
  1. set group height in css:
.vis-panel.vis-left .vis-label .vis-inner {
  height: 40px;
}
  1. set timeline option groupHeightMode: 'fixed'

Demos:

  1. Combination of redraw() and _updateItemsInRange() changes: https://codesandbox.io/s/vis-timeline-jg9o6
  2. Combination of redraw() and group height style: https://codesandbox.io/s/vis-timeline-hycf1
  3. Combination of redraw() and option groupHeightMode: 'fixed': https://codesandbox.io/s/vis-timeline-ou1o0

kodemi avatar Dec 01 '19 01:12 kodemi

Is there any updates on a bug fix for this ticket?

I am running into the same issue in my current project. We had tried the original suggested fix to change the element a while back and that did not work or flickered depending on where it was used in React. The issue recently came back into discussion.

Any updates would be appreciated.

mckindd avatar Jun 11 '21 19:06 mckindd

@kodemi Thank you for this, I used the first option with patch-package to fix it.

TomHiller-swd avatar Sep 09 '22 19:09 TomHiller-swd