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

Scrollbar recalculation does always move scrollbar to the top #84

Open
crudo1f opened this issue Jul 25, 2017 · 6 comments
Open

Scrollbar recalculation does always move scrollbar to the top #84

crudo1f opened this issue Jul 25, 2017 · 6 comments

Comments

@crudo1f
Copy link
Contributor

crudo1f commented Jul 25, 2017

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
Copy link
Contributor

@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
Copy link
Contributor Author

crudo1f commented Jul 28, 2017

@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
Copy link

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
Copy link

@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
Copy link
Contributor

@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 😃

@billdami
Copy link

@alexander-alvarez sounds good! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants