Scrollbar recalculation does always move scrollbar to the top

[crudo1f]

In case of calling the update() or recalculate() action the scrollbar is not only resized it is always moved to the top. This happens on resizing the page as well.

[alexander-alvarez]

@crudo1f yes, nice find... It seems the problem is through the myriad of function calls that we set the scrollOffset to 0 here and here. It should be straightforward to leverage https://github.com/alphasights/ember-scrollable/blob/master/addon/components/ember-scrollable.js#L193..L200 instead of 0. Although I'm not sure how this would affect other behavior.

Do you have the ability to take a look at it perchance?

[crudo1f]

@alexander-alvarez sorry for the late reply. I am not able to look into that the upcoming weeks because of holidays. In case this remains open I will try to take care of it afterwards.

[billdami]

I came across this issue in my project when I try to restore an ember-scrollable's previous vertical scroll position by binding to the scrollToY property. When the scrollable is rendered with a non-zero scrollToY the element is scrolled correctly, but the scrollbar still appears at the top, and when you go to manually scroll, it "jumps" to where it should be.

It seems that instead of hardcoding this value as zero: https://github.com/alphasights/ember-scrollable/blob/master/addon/components/ember-scrollable.js#L279

It should be using the current scrollToY value?

[billdami]

@alexander-alvarez @crudo1f I tried the solution in my previous comment by overriding the ember-scrollable component in my project, and it appears to work, although I'm not sure if there may be any unintended side-effects/corner cases (I'm only using vertical scroll in my case):

createScrollbar() {
  if (this.get('isDestroyed')) {
      return [];
  }
  const scrollbars = [];

  this.resizeScrollContent();

  if (this.get('vertical')) {
      const verticalScrollbar = new Vertical({
          scrollbarElement: this.$(`${scrollbarSelector}.vertical`),
          contentElement: this._contentElement
      });
      this.set('verticalScrollbar', verticalScrollbar);
      this.updateScrollbarAndSetupProperties(this.get('scrollToY'), 'vertical');
      scrollbars.push(verticalScrollbar);
  }
  if (this.get('horizontal')) {
      const horizontalScrollbar = new Horizontal({
          scrollbarElement: this.$(`${scrollbarSelector}.horizontal`),
          contentElement: this._contentElement
      });
      this.set('horizontalScrollbar', horizontalScrollbar);
      this.updateScrollbarAndSetupProperties(this.get('scrollToX'), 'horizontal');
      scrollbars.push(horizontalScrollbar);
  }
  return scrollbars;
}

[alexander-alvarez]

@billdami this is in line with what I was expecting. I don't have push access to this repo but if we get a PR out I can bug @offirgolan to merge it and cut a version :smiley:

[billdami]

@alexander-alvarez sounds good! 👍