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

Content stuck after anchorscroll #508

Closed
CoenWarmer opened this issue Feb 3, 2014 · 47 comments
Closed

Content stuck after anchorscroll #508

CoenWarmer opened this issue Feb 3, 2014 · 47 comments
Milestone

Comments

@CoenWarmer
Copy link

I'm using anchorscroll to scroll to an element on the page.

After clicking the link to scroll to a location, the content view seems to get stuck. You can scroll down further than the element, but you can't scroll up past the element.

I've tried reproducing this on vanilla Angular, and it doesn't seem to occur there. Is this a bug?

Ionic: http://plnkr.co/edit/68V0hX5hji9KedZ3gWg9
Vanilla Angular: http://plnkr.co/edit/PCO051UJS8EHbdkmFV40

Also referenced on:
http://forum.ionicframework.com/t/content-stuck-after-anchorscroll/776

@adamdbradley adamdbradley added this to the 0.9.24 milestone Feb 3, 2014
@ajoslin
Copy link
Contributor

ajoslin commented Feb 3, 2014

$anchorScroll still uses normal scroll functionality, but Ionic uses javascript-transform-scrolling.

We will either remove $anchorScroll in favor of something similar, or rewrite it to work with our scrolling.

@CoenWarmer
Copy link
Author

Is there an alternative for anchor scrolling that can be used now while we wait for an ionic specific implementation?

@mlynch
Copy link
Contributor

mlynch commented Feb 5, 2014

Sure, set overflow-scroll="true" on the content area and then try it and see if it works for ya.

On Feb 5, 2014, at 6:40 AM, Coen Warmer notifications@github.com wrote:

Is there an alternative for anchor scrolling that can be used now while we wait for an ionic specific implementation?


Reply to this email directly or view it on GitHub.

@CoenWarmer
Copy link
Author

Yup, this works! Thanks.

@ajoslin
Copy link
Contributor

ajoslin commented Feb 9, 2014

Added in c2bbd9e. Lemme know how it works for you @CoenWarmer!

@CoenWarmer
Copy link
Author

Hey @ajoslin, thanks for this!

Just tried to drop this in, but I'm seeing the same behavior as before: the content snaps back when you try to scroll up before the hash that you jumped to.

This is how I tried using it:

$scope.gotoDay = function(id){
$location.hash(id);
$ionicScrollDelegate.anchorScroll();
}

Is that how it's supposed to work?

@ajoslin
Copy link
Contributor

ajoslin commented Feb 9, 2014

Yes. I'll test it out some more.

On Sun, Feb 9, 2014 at 3:13 PM, Coen Warmer notifications@github.comwrote:

Hey @ajoslin https://github.com/ajoslin, thanks for this!

Just tried to drop this in, but I'm seeing the same behavior as before:
the content snaps back when you try to scroll up before the hash that you
jumped to.

This is how I tried using it:

$scope.gotoDay = function(id){
$location.hash(id);
$ionicScrollDelegate.anchorScroll();
}

Is that how it's supposed to work?

Reply to this email directly or view it on GitHubhttps://github.com//issues/508#issuecomment-34584745
.

@ajoslin
Copy link
Contributor

ajoslin commented Feb 9, 2014

OK, fixed it in 4c9a4c0! There were a couple problems.

@CoenWarmer
Copy link
Author

Alright, this is looking really good- nice work @ajoslin. Now we can scroll up after using anchorscroll, opening up cool stuff like an alfabetized list jumper. :)

I do see one issue that I can't quite explain. I have four anchors on my page, I can scroll just fine to id1, id3 and id4. But when I try to scroll to id2, the page scrolls a bit past id2. This didn't happen before. Any ideas?

@ajoslin
Copy link
Contributor

ajoslin commented Feb 10, 2014

Another bug is that the first time the browser sets window.location.hash, it tries to scroll to it. I'm not exactly sure how we can fix this - we can decorate $location.hash and set scroll to 0 the first time it is set?

TODO:

  • fix problem with browser and $location.hash()
  • use document.getElementById instead of querySelector (to allow number IDs)

@ajoslin
Copy link
Contributor

ajoslin commented Feb 10, 2014

@CoenWarmer can you post a plunkr/codepen of the scroll problem?

@ajoslin
Copy link
Contributor

ajoslin commented Feb 10, 2014

@RangerRick try out the latest commit in your plunkr with nightly build in 30 minutes when the CDN refreshes itself (or do bower install driftyco/bower-ionic#master and try it locally)

ajoslin added a commit that referenced this issue Feb 10, 2014
@CoenWarmer
Copy link
Author

@ajoslin Sure, here you go: http://plnkr.co/edit/XrGitDamL2hqPlEIwNAK?p=preview

I'm using the nightly in this plunkr, but somehow the content seems to stick in this version again?

@ajoslin
Copy link
Contributor

ajoslin commented Feb 10, 2014

The problem there is the browser still: it sets scrollTop to the element matching window.location.hash the first time you set window.location.hash.

I added something small in your plunkr and it works: http://plnkr.co/edit/bV2zlqL79G8yBEd3c1gV?p=preview (obviously not a final solution :-))

I think we just need to add a decorator to $location.hash() to set all scroll areas' scrollTop to 0 when it is set.

@CoenWarmer
Copy link
Author

In your Plunkr all links scroll to id="foo1"?

@ajoslin
Copy link
Contributor

ajoslin commented Feb 10, 2014

http://plnkr.co/edit/bV2zlqL79G8yBEd3c1gV?p=preview - there was a missing closing brace on the first p: <p - so clicking any of them counted as the first one.

@CoenWarmer
Copy link
Author

Ok great @ajoslin, the example works.

I've done some further research on my case, and I seem to have found what's the issue. Check: http://www.youtube.com/watch?v=QyUATm4U7R4

It seems the content that's below Day2 is not high enough so the scrollposition doesn't set exactly to the top of Day2. However, if you scroll to Day3 or 4 that doesn't seem to matter: the content area exactly positions to to the anchor. Is it possible to make the content area always line up with the anchor, regardless of the height of the content below it?

@RangerRick
Copy link

@ajoslin FYI, I rearranged my plunkr from #572 to use the latest nightly build and even adding your test hack setting scrollTop it still sticks:

http://embed.plnkr.co/shyXuk/preview

Note also that the behavior is different in the plunkr edit preview than it is when you actually go to the full embed link above. In the preview on the edit page, it works as expected, but when you go to plunkr's full-screen embed link like above, it still sticks.

@CoenWarmer
Copy link
Author

@RangerRick Your Plunkr doesn't seem to stick. Looks good from here?

@RangerRick
Copy link

@CoenWarmer It does in Chrome for Mac on my system. I checked to make sure it wasn't a caching issue, I'm getting Ionic, v0.9.24-alpha-728 from the CDN. Should that have the fix?

@RangerRick
Copy link

And on Firefox for Mac, my plunkr still does the bounce thing, where it shoots to 500 and goes back to 0.

@CoenWarmer
Copy link
Author

@RangerRick I'm on Chrome / Mac as well.

Your Plunkr uses the nightly. I'm just opening your Plunkr and clicking the clock. The content scrolls to item 500. After that I can scroll upwards and down without problems.

@CoenWarmer
Copy link
Author

Also I don't think you need the setTimeout function at all. Just having
$scope.doScroll = function() {
$location.hash('foo-500');
$ionicScrollDelegate.anchorScroll();
} should do the trick.

@RangerRick
Copy link

@CoenWarmer I'm just refreshing my plunkr and clicking the clock too, and it sticks to 500. With or without the timeout.

http://ranger.befunk.com/temp/ionic-plunkr.mov

@CoenWarmer
Copy link
Author

@ajoslin Hey Andy, did you see my previous message? There is a small issue remaining with the scroll when there is too little content below it. Unless there is no content below it, then it suddenly works as expected. More detail here: http://www.youtube.com/watch?v=QyUATm4U7R4

@ajoslin
Copy link
Contributor

ajoslin commented Feb 11, 2014

I'll look at this when I start today, sorry I switched to something else
yesterday :-)
On Feb 11, 2014 5:30 AM, "Coen Warmer" notifications@github.com wrote:

@RangerRick https://github.com/RangerRick I don't know what to tell
you... I'm just clicking your Plunkr and I can't get it to stick... Really
strange!

Reply to this email directly or view it on GitHubhttps://github.com//issues/508#issuecomment-34742199
.

@CoenWarmer
Copy link
Author

@ajoslin Amazing, thanks

@ajoslin
Copy link
Contributor

ajoslin commented Feb 11, 2014

@CoenWarmer we currently use this algorithm to get the position of an element within its scroller container: https://github.com/driftyco/ionic/blob/master/js/utils/dom.js#L19-L31

I don't see how we would fix that for your use-case; any ideas?

The other solution to try, I guess, is to try using el.getBoundingClientRect(). Could you edit ionic.js (search for getPositionInParent) in your app and play with the function (try using getBoundingClientRect().{left,top}) and lemme know if you have any luck?

@ajoslin
Copy link
Contributor

ajoslin commented Feb 11, 2014

Hi @RangerRick - this works: http://plnkr.co/edit/7oqi9I?p=preview

I will commit something which causes $location.hash to set scrollTop to 0 of scroll elements when it is set.

@ajoslin
Copy link
Contributor

ajoslin commented Feb 11, 2014

..Also, anchorScroll should allow animating! I'll let you pass (true) to it.

@CoenWarmer
Copy link
Author

@ajoslin Will try that, thanks!

Perhaps it might actually not be such a big issue as there will be more items under Day2 in the final version of the app.

@RangerRick
Copy link

@ajoslin That plunk still bounces on my mac chrome browser, but it's inconsistent. When it sticks to 500, though, it does actually let me scroll. Haven't had a chance to try it in a phonegap install yet.

@ajoslin
Copy link
Contributor

ajoslin commented Feb 11, 2014

@RangerRick try this guy: http://plnkr.co/edit/3Uvoan?p=preview

I made anchorScroll be sure to make sure the scrollView fits the content before it goes.

@RangerRick
Copy link

@ajoslin yup, that did it, woot!

@RangerRick
Copy link

Heh, must have just been timing. Still bounced. :P

Also, I've tried reinstating my code to do anchorScroll in my real app, and it actually scrolls down 100 pixels or so too far in iOS. Not sure how to debug why... Just tried your plunk in safari for iOS and it goes to the right spot.

@ajoslin
Copy link
Contributor

ajoslin commented Feb 11, 2014

OK. I have a feeling getPositionInParent is returning wrong values sometimes for some reason ... I'll see the results @CoenWarmer gets with getBoundingClientRect(), or try it later today.

@adamdbradley adamdbradley modified the milestones: 0.9.25, 0.9.24 Feb 12, 2014
@ajoslin
Copy link
Contributor

ajoslin commented Feb 13, 2014

Looking into this some more.

@ajoslin
Copy link
Contributor

ajoslin commented Feb 13, 2014

Ok @RangerRick, your example is messing up on iOS because of the tap-topbar-to-scroll-top functionality. Usually when your button is pressed it registers as a topbar-tap too.

Right now there's no way to disable tap-topbar functionality - we probably should add that as an option on the headerBar directive.

@CoenWarmer - I'm closing this for now, but if you do find a better algorithm for finding an element's scrollHeight in our fake-scrolling paradigm, please reopen!

🎱

@ajoslin ajoslin closed this as completed Feb 13, 2014
@RangerRick
Copy link

@ajoslin So that's what causes the wrong-offset stuff? Or just the bounce?

@ajoslin
Copy link
Contributor

ajoslin commented Feb 13, 2014

The wrong-offset stuff was only happening on desktop for me; I could not reproduce it on mobile (Android or iOS).

My theory for that is that perhaps the mouse was sticking and messing up the scroll.

If you clone the latest master, I included your test in js/ext/angular/test/anchorScroll.html. Hook up a mobile device with a static server and try to reproduce it and lemme know :-)

@RangerRick
Copy link

The plot thickens. In my app, at least, it works in Safari on iOS, but it's wrong in the Cordova app version. Sigh. Not sure how to even put together a reasonable test case. Unless you've got an iOS device and want me to add you to my beta... :)

@ajoslin
Copy link
Contributor

ajoslin commented Feb 13, 2014

Ha, ok I'll give it a run.

Send it to the email on my gh profile.
On Feb 13, 2014 12:42 PM, "Benjamin Reed" notifications@github.com wrote:

The plot thickens. In my app, at least, it works in Safari on iOS, but
it's wrong in the Cordova app version. Sigh. Not sure how to even put
together a reasonable test case. Unless you've got an iOS device and want
me to add you to my beta... :)

Reply to this email directly or view it on GitHubhttps://github.com//issues/508#issuecomment-35004630
.

@RangerRick
Copy link

I wonder if it's some kind of weird interaction with the PhoneGap status bar plugin or something. Otherwise I can't imagine how it'd be different, really...

@RangerRick
Copy link

FYI, for anyone else running into this issue thread, my problem with offsets being wrong was related to there being intermediate objects inside my list, which was throwing the calculations off. Still a bug, I suppose, but a different bug. :)

@ionitron-bot
Copy link

ionitron-bot bot commented Sep 6, 2018

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants