visjs / vis-timeline

📅 Create a fully customizable, interactive timelines and 2d-graphs with items and ranges.
https://visjs.github.io/vis-timeline/
Other
1.87k stars 313 forks source link

Vertical scroll position should persist after setGroups call #172

Open kodemi opened 4 years ago

kodemi commented 4 years ago

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

yotamberk commented 4 years ago

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

kodemi commented 4 years ago

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

kodemi commented 4 years ago

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

kodemi commented 4 years ago

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 commented 4 years ago

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;
    ...
  2. set group height in css:
    .vis-panel.vis-left .vis-label .vis-inner {
    height: 40px;
    }
  3. 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
mckindd commented 3 years ago

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.

TomHiller-swd commented 2 years ago

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

marcortw commented 12 months ago

Four years down the road, I'd also like to thank you @kodemi 😄 For me it did the trick with just the added call to _didResize. I didn't have to set any of the height specific values. But that could as well be because I don't use any height settings at all, might be why it works for me without adding one of the three other options.

marcortw commented 3 months ago

Added PR #1814