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

cause of jumping when reaching the top and having a lower limit #172

Closed
tylkomat opened this issue Jul 18, 2017 · 20 comments
Closed

cause of jumping when reaching the top and having a lower limit #172

tylkomat opened this issue Jul 18, 2017 · 20 comments

Comments

@tylkomat
Copy link

tylkomat commented Jul 18, 2017

The Line 166 in src/modules/viewport.js causes the jumping.

When having a limit of index 1 as the top then the mentioned line forces a padding to be added to viewport.scrollTop, since paddingHeight is negative. This scrolls the viewport down again, while the user scrolls up, which causes the jumping.

When this line is removed everything works as expected when having a lower limit. On the other hand without a lower limit, ui-scroll will fetch new items indefinitely.

There should be a check which only executes that line when the beginning (buffer.bof === false) was not reached.

tylkomat added a commit to tylkomat/ui-scroll that referenced this issue Jul 27, 2017
only adjust viewport when beginning of buffer is not reached angular-ui#172
@dhilt
Copy link
Member

dhilt commented Aug 16, 2017

@tylkomat Thanks for your participation! I'm trying to reproduce the issue and I can't. We have a demo which emulates remote server and provides start index progammable option (demo, sources) – it would be great if you could break it or give some another repro. I need to see wrong behaviour to be able to cover it with tests.

@mamacasc
Copy link

mamacasc commented Aug 16, 2017 via email

@dhilt
Copy link
Member

dhilt commented Aug 17, 2017

@mamacasc Let's say we have the log

requested index = 1, count = 10, resolved 10 items
requested index = 11, count = 10, resolved 10 items
requested index = -9, count = 10, resolved 0 items

As the scroller is initialized (or reloaded) it starts requesting items starting from the item with index = 1. First and second requests were to fill the viewport from top (1) to bottom (20). Let's say that 20 items have height = H1 while the viewport has height = H2 and H1 - H2 < paddingHeight, where paddingHeight is internal ui-scroll parameter to provide a viewport buffer space. See the doc related to padding parameter. And the third request was to fill the top viewport buffer space which height is also equal to paddingHeight. But still our dataset has no negative indexes, third request returns no data.

You don't need to do any special things with this. If your dataset has items with negative indexes, scroller will take them at the right time.

@tylkomat
Copy link
Author

tylkomat commented Aug 17, 2017

@dhilt The jumping happens in the repo I gave you before. On the main page (/list) just scroll down some items until the buffer fetches the next bunch and scroll up slowly and you'll notice a jumping (your scroll gets pushed down a little bit while you are scrolling up).

It may be due to the datasource that I use. I haven't yet been able to produce a simpler example, but I'm not the first one to report such kind of problem.

My solution does not break other examples. It only uses what is already there. When scrolling down this behaviour is already in place, when reaching the end (datasource gives less items back than requested) buffer.eof is set to true and no more items are requested. This was not in place for scrolling up, while buffer.bof is set, but not used. The merge request implements that behaviour and disables loading additional items when the top (datasource gives less items than requested) was reached.

@dhilt
Copy link
Member

dhilt commented Aug 17, 2017

@tylkomat The ui-scroll directive gives a number of ways of usage, it does not push you into some strict boundaries, but also it requires to be more careful. Your code is too complicated and I'm still not able to identify the issue with your demo. For example, I replaced

<div ui-scroll="item in listDatasource" adapter="adapter">
  <a ui-sref=".single({index: item})" ng-bind="item" persist-scroll-position="getSetStartIndexCallback(item)"></a>
</div>

with just

<div ui-scroll="item in listDatasource" adapter="adapter"> {{item}} </div>

and all looks fine. Then I went further and returned a container with ui-sref and removed margin: 5px from layout.css. And again it looks ok. So I incline to the opinion that the cause of your problem is so-called margin collapsing. We have an issue on the GitHub: #39. Please try to reskin your grid without inner margins, hope it helps.

@mamacasc
Copy link

mamacasc commented Aug 17, 2017 via email

@dhilt
Copy link
Member

dhilt commented Aug 17, 2017

@mamacasc

The doc (Readme). uiScroll directive. Parameters:

padding - expression, optional - extra height added to the visible area for the purpose of determining when the items should be created/destroyed. The value is relative to the visible height of the area, the default is 0.5 and the minimal value is 0.3.

So you may not set this parameter, it has the default value. But this is important part of the engine.

Post somewhere the repro of your issue (plunker, jsfiddle, whatever), I will take a look. Or just take one of our examples and adjust it step by step to satisfy your needs.

@mamacasc
Copy link

mamacasc commented Aug 17, 2017 via email

@dhilt
Copy link
Member

dhilt commented Aug 17, 2017

@mamacasc The demos code is in the repository: https://github.com/angular-ui/ui-scroll/tree/master/demo. Clone the repo and try it via npm start on localhost:5005.

@mamacasc
Copy link

mamacasc commented Aug 18, 2017 via email

@dhilt
Copy link
Member

dhilt commented Aug 19, 2017

@mamacasc Plunker is great, it saves our time! I forked it and made a short fix: https://plnkr.co/edit/EUznlF0ynxJK1ptkAZEG?p=preview. You had a problem with the viewport's height, which must be constrained according to the documentation.

@mamacasc
Copy link

mamacasc commented Aug 21, 2017 via email

@mamacasc
Copy link

mamacasc commented Aug 21, 2017 via email

@mamacasc
Copy link

mamacasc commented Aug 23, 2017 via email

@dhilt
Copy link
Member

dhilt commented Aug 23, 2017

@mamacasc Lokks like something's not ok with datasource implementation. Also it may be stylesheets issue. Anyway we can't help you without repro. Try to extend working plunker to satisfy your app needs. (Maybe your issue will go away during this process.)

@mamacasc
Copy link

mamacasc commented Aug 23, 2017 via email

@dhilt
Copy link
Member

dhilt commented Aug 31, 2017

@tylkomat I've made a fix for #171 issue. This may relate to your problem too. This will be included in the next release, but it would be great if you could try fixed distributive: https://raw.githubusercontent.com/angular-ui/ui-scroll/effective-height/dist/ui-scroll.js.

@tylkomat
Copy link
Author

tylkomat commented Sep 1, 2017

@dhilt Just using it does not change anything. Do I need to configure something?

@dhilt
Copy link
Member

dhilt commented Sep 1, 2017

@tylkomat No, just try branch' distributive (instead of npm/bower latest) in your project. Btw, the problem you described here may still be caused by margin collapsing issue and could be avoid through grid template's redesign.

But the change you suggested in PR #173 is very close to my changes I mentioned yesterday. They both relate to scroll-top adjustement in case of negative values, but your change does not allow to pass new test that I created for jumping problem in the branch.

@dhilt
Copy link
Member

dhilt commented Sep 29, 2017

1.7.0-rc.2 is published, it includes discussed changes. If it does not solve any question mentioned here, please open new issue.

@dhilt dhilt closed this as completed Sep 29, 2017
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