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

Scroller is still updating in hidden tabs / states #83

Closed
moosi opened this issue Apr 20, 2016 · 14 comments
Closed

Scroller is still updating in hidden tabs / states #83

moosi opened this issue Apr 20, 2016 · 14 comments

Comments

@moosi
Copy link

moosi commented Apr 20, 2016

I use your scroller plenty times in my app. It is nested in different tabs that can contain multiple states (some states include a scroller, some do not). The problem is all states stay active in background. When I change from one tab to another the scroller is still reacting to scroll events. That means if I open 3 tabs containing a scroller, all 3 scrollers are still reacting to scroll events simultaneously, even if the opened tab does not contain an own scroller.

Here is a simple plnkr with 2 tabs and 1 scroller per tab:
https://plnkr.co/edit/MUgEKBOnQng170YLtEvs

@dhilt
Copy link
Member

dhilt commented Apr 20, 2016

I have investigated the issue, thank you!

The cause of this issue is that no viewport directive is being used in your demo and the global "window" acts as a viewport for both scrollers. And thus we have a scroll-event subscription which works for both scrollers: viewport.bind('scroll', resizeAndScrollHandler);

We need a little time to think of how this could be fixed.

@mfeingold
Copy link
Contributor

A simple fix for this problem would be to use ui-scroll-viewport directive to introduce separate viewports to each scroller

@moosi
Copy link
Author

moosi commented Apr 21, 2016

Is it possible to use the ui-scroll-viewport directive without specifiying a fixed height? For example:

<div ui-scroll-viewport ></div> instead of <div ui-scroll-viewport style="height: 400px"></div>

If the directive will introduce a seperate scroll area (not using the browsers scroll area) this will not work for my usecase.

@mfeingold
Copy link
Contributor

The viewport height has to be constrained

@dhilt
Copy link
Member

dhilt commented Apr 22, 2016

I did a commit on this issue. And prepared a demo: http://rawgit.com/angular-ui/ui-scroll/master/demo/disabled/disabled.html. With it you can subscribe on selectedIndex changes and call .disable() on the adapter when selectedIndex does not match scroller instance. Something like

$rootScope.$watch('selectedIndex', function (value) {
  if(value !== 0) scrollerAdapter.disable();
  else scrollerAdapter.enable();
});

for first tab controller and

$rootScope.$watch('selectedIndex', function (value) {
  if(value !== 1) scrollerAdapter.disable();
  else scrollerAdapter.enable();
});

for second tab controller.

@mfeingold
Copy link
Contributor

would it be better to have a property (call it enabled) instead? Among other things this would allow for checking the property.

@moosi
Copy link
Author

moosi commented Apr 27, 2016

First of all thank you for the possiblity to disable the scroller. I thought a lot about how I could detect if a specific scroller becomes invisible/visible. For my plunker it is easy and works as you have suggested: Both tabs are active and I can simply switch the tab index. But in my case I have a bottom navigation and a top navigation. The first tab of the bottom navigation can contain none scroller, one scroller or multiple scrollers. From this point the user can navigate to an 'endless' chain of views within the tab (and can also navigate back to previous views). For example
"Profile - Post - Hashtag List - Another Profile - Another Post - Like List - ....". I don't have a solution for this yet. Do you think it will be possible for the future that the scroller can recognize itself if it is visible or not? Because I noticed some other problems related to this. For example when I open a bottom sheet (which will push the scroller into background like a modal dialog) the scroller won't stop requesting new data.

@dhilt
Copy link
Member

dhilt commented Apr 29, 2016

Per Mike's request I replaced Adapter.disable() logic with disabled attribute. With it you may solve the issue this way:

<md-card ui-scroll="post in feed.posts"" disabled="selectedIndex !== 0">
...
<md-card ui-scroll="post in profile.posts" disabled="selectedIndex !== 1">

Here you can obtain full demo of this feature: http://rawgit.com/angular-ui/ui-scroll/master/demo/disabled/disabled.html. This will be included in next release.

Regarding to your last comment, if I understood it correctly, just have a property (or set of properties) on scope that reflects the navigation state of your app and bind it to "disabled" attribute in appropriate relation.

@moosi
Copy link
Author

moosi commented May 9, 2016

Oh damn, somehow my response was not been sent :( Here again:

I use ui-router and ui-router-extras to handle the states of my application. I tried to save the state that belongs to the controller on initialization and also added an event listener:

function initScrollerListener () {
    feedVm.state = $state.current.name + JSON.stringify($state.params);

    // Disable scroller if not visible
    $rootScope.$on('$stateChangeSuccess', function(event, toState, toParams, fromState, fromParams) {
        var nextState = toState.name + JSON.stringify(toParams);
        feedVm.scroller.disabled = feedVm.state !== nextState;
     });
}

I am using the deep state redirect feature of ui-router extras (https://christopherthielen.github.io/ui-router-extras/#/dsr). Because of that I can not use '$stateChangeStart' and have to wait for '$stateChangeSuccess' to access the final state transition (for example: If you hit the feed tab 'home.feed' is getting redirected to the last active state of the tab 'home.feed.likes'). The problem is during the time span where the state change is happening ui-scroll is already requesting new data and before the '$stateChangeSuccess' event the view gets rendered -> the scroller is setting y-pos to 0.

Besides that I am currently using a routing service that takes care about everything related to states. If I can find a solution for the first problem I am still mixing controller logic with states.

@dhilt
Copy link
Member

dhilt commented May 10, 2016

  1. Mike commited a change so "disabled" is not an attribute of the directive, it is a property of the Adapter. I see you got it. But there is also some new restriction: "disabled" can't be set before the directive is initialized.

  2. This feature ("disabled") prevents only user scroll/resize data fetching. May we should go a bit further, and prevent any fetching if "disabled" is true... But with p.1 this becames not so easy to implement it. I mean a prevention of initial data load, because of the moment of scroller initialization is undefined. We need to think about it. By the way here is some workaround I see:

$rootScope.isScrollerInit1 = false;
$rootScope.isScrollerInit2 = false;

$rootScope.$on('$stateChangeSuccess', function (...) {
  if(/* state is relevant to scroller1 tab */)
    $rootScope.isScrollerInit1 = true;
  if(/* state is relevant to scroller2 tab */)
    $rootScope.isScrollerInit2 = true;
});
<div ui-scroll="row in datasource1" ng-if="isScrollerInit1">
...
<div ui-scroll="row in datasource2" ng-if="isScrollerInit2">

This will prevent initialization process itself while your state doesn't match necessary tab.

@moosi
Copy link
Author

moosi commented May 20, 2016

If I understand you correctly the current solution should fit my router service behaviour:

  1. On state change start the scroll position for the exiting state is getting stored by the router service
  2. On successful state change the scroller of the inactive state is getting disabled by its controller
  3. On successfull state change to an previous active state (sticky) the router service restores the scroll position ($window.scrollTo(x, y))
  4. On success state change to an previous active state the scroller is getting re-enabled by its controller

I interpret from your response that the scroller should be able to react to the '$window.scrollTo' action even if it is disabled. But for some reason each time I go back to an previous active state the scroll position is different. In combination with the simple infinite scroll plugin I used previously it worked as expected. If you confirm my interpretation the problem should be my own router logic.

For the problem where the scroller is still active during state transition and requests new data I still couldn't find a solution as well.

@dhilt
Copy link
Member

dhilt commented May 20, 2016

If disabled is set to true then scroller should ignore $window.scrollTo. The only problem is that the first data fetch occures without scroll event. To prevent it I suggested to ng-hide ui-scroll directive until user had come to relevant screen for the 1st time.

@moosi
Copy link
Author

moosi commented May 23, 2016

$window.scrollTo: I played around with this scenario and found a problem when using this function. It works as expected if I scroll to a position within the first loaded dataset. For example I have a button to switch between position 0px and 600px. It will always scroll to the correct position. But if I use some bigger ranges (0px and 9000px) the scroller needs to render new datasets. Using the button multitple times will generate something like this: '0px - 2279px - 28px - 5444px - 28px - 7467px - 19px - 9000px - 19 px - 9000px - ...' That is the reason why my scroll-position cache always restored a different position. If you need example code for this please let me know.

Data-fetch: I got you. The first time data fetch is fine. The problem is the data fetch when changing from one tab to another but this is a problem with my own code. I need to disable the scroller somehow before state transitions take place.

@dhilt
Copy link
Member

dhilt commented May 23, 2016

Ok! So there's no problem with "disable" itself as I can see and we are close to finish this issue. Regarding to $window.scrollTo, I'm still not sure if I understood you properly, but if you think we can help you from the side of the directive, you may open a new issue and a repro would be preferably...

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