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

Scrolling not possible, as we can't get any closer to the destination, pageScrollFinish doesn't fires #50

Closed
martinmanzo opened this issue Oct 18, 2016 · 3 comments
Labels

Comments

@martinmanzo
Copy link

Hi, thanks for this directive. I'm having just one problem.

My project has a type of vertical news feed, each feed is a panel that have some buttons in the footer. Those buttons change the way the information is displayed thus causing the height of the panel to change.

In order to the user to always see the correct feed I needed to scroll up to the top of the panel when the information changed. This was really easy to do with this directive.

The thing is that if I want to change the way the information is displayed for a second (or third...) time, and there was no scrolling (the offset to the top hadn't changed) I get this message

"Scrolling not possible, as we can't get any closer to the destination"

and the pageScrollFinish event never fires.

For the moment I managed this issue by assigning a different offset to each anchor (differing by 1px was enough) what I think that the correct thing would be to the event to fire.

Am I missing something?

Thanks,
Martín

@Nolanus
Copy link
Owner

Nolanus commented Oct 19, 2016

Hi Martin,

glad to hear the this library helps you with your project.
Unfortunately I didn't quite get whether you're reporting one or two problems.

  1. pageScrollFinish does not fire in case we can't get any closer to the destination
    That's correct and I agree with you that it should fire whenever a "scroll" has been started. I possibly change the emitted value into something else than a boolean to indicate the reason the scrolling stopped in greater detail (successful finish, interrupted, can't get closer to destination, can't find target)

  2. no scrolling when "the way information is displayed changes for second time"
    Is it desired that no scrolling takes place? Does "changing the way info is displayed" means the positioning changes and scrolling up further would be possible?

@Nolanus Nolanus added the bug label Oct 19, 2016
@martinmanzo
Copy link
Author

Hi @Nolanus, thanks for the response.

It's one problem, the first one you wrote.
"Changing the way info is displayed" may change the height of the element but not the position. Imagine this situation, a news feed with various items (i.e: Facebook or Twitter) with an option to expand or contract each individual item by using a set of button at the items footer. Initially the height of the items makes scrolling needed so that the footer can be seen. The user scrolls down and clicks a button to change what is displayed in the item. This new view form has a smaller height so the panel adapts as needed. Without auto-scrolling the user would have to scroll up again and search for the wanted item. That's where your library comes into play. Once the user clicks the button the page scrolls to the start of the corresponding item and changes the height of the item accordingly.

Now the user want to change again the visualization without having to scroll down (because the new height made that the buttons were available without having to scroll down). As position hasn't changed the plugin shows "Scrolling not possible, as we can't get any closer to the destination" and the event isn't fired.

The solution you describe in 1) is excellent but maybe it hasn't has to be so complex. I was reading your code and by changing the if starting at line 187:

if (pageScrollInstance.distanceToScroll === 0) {
      // We're at the final destination already
      // OR we need to scroll down but are already at the end
      // OR we need to scroll up but are at the top already
      if (_angular_core.isDevMode()) {
           console.log('Scrolling not possible, as we can\'t get any closer to the destination');
      }
      return;
}

with

if (pageScrollInstance.distanceToScroll === 0) {
      // We're at the final destination already
      // OR we need to scroll down but are already at the end
      // OR we need to scroll up but are at the top already
      if (_angular_core.isDevMode()) {
           console.log('Scrolling not possible, as we can\'t get any closer to the destination');
      }
      pageScrollInstance.fireEvent(true);
      return;
}

This wouldn't change the meaning of what the event name declares (that the scroll ended) and by sending true as a parameter you are saying that the scroll position is where the dev want's it to be.

Am I right?

Sorry for my English but it isn't my first language :)

@martinmanzo
Copy link
Author

Thanks!

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

No branches or pull requests

2 participants