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

Section will be snapped even if section height is larger than viewPort #96

Closed
arijoon opened this issue Jan 27, 2016 · 9 comments
Closed
Labels
Milestone

Comments

@arijoon
Copy link

arijoon commented Jan 27, 2016

Surprisingly it will snap to the second section even if the current section is larger than view port and the second section is not in view! So assuming -> is a section and x indicates which section is viewed

1-> x
2->

If I scroll down but still stay within 1's view port it'll snap to section 2

1->
2-> x

Also if scrolled amount is less than directionalThresh it will snap back to the top of the section (so content at the bottom of a section can never be viewed if it's larger than viewPort). I tried modifying the library to support sections bigger than view port but no luck yet.

@arijoon
Copy link
Author

arijoon commented Jan 27, 2016

Actually found the issue (line 208: getPanelsInViewPort):

var viewport = { top: self.$snapContainer.scrollTop() };
viewport.bottom = viewport.top + self.$snapContainer.height();

That is not really getting the correct panels. Try printing it to console and you'll see the issue. It get's all the panels in container from top of viewport to bottom of the container instead of viewport. I suggest replacing that with this:

 var viewport = { top: $(window).scrollTop() };
 viewport.bottom = viewport.top + $(window).height();

@guidobouman
Copy link
Owner

Actually, it's not that easy, the plugin can run on other elements in the document as well. I'll have a look at it later today.

@guidobouman guidobouman added the bug label Feb 1, 2016
@guidobouman guidobouman added this to the v0.16.0 milestone Feb 1, 2016
@arijoon
Copy link
Author

arijoon commented Feb 1, 2016

Are you sure? As the variable name suggests I thought viewport is the section in view. That will get exactly the top and bottom of the section in view.

@guidobouman
Copy link
Owner

Yes that's true. But your solution doesn't work when you initialise it on a div instead of body. that's what the scrollContainer variable if for.

@arijoon
Copy link
Author

arijoon commented Feb 5, 2016

ah I see my bad, I totally forgot this plugin is awesome enough to work on any container. So we'll need to grab min(viewTop,containertop) and max(viewBottom,containerBottom)

@Smashr
Copy link

Smashr commented Feb 14, 2016

It would be great if the plugin would also snap to the bottom of panels that are larger than the viewport/container. So basically if the panel is as high as the viewport/container, snap like it normally does (to the top) ... but if the panel is taller than the viewport/container, it should have two snap points, one at the top of the panel and one at the bottom.

Assuming there are two panels containing a large amount of content, and both are taller than the viewport/container :

When scrolling from panel 1 to panel 2, the scrolling will first snap to the top of Panel 1 (first snap point), then scroll normally until its near the bottom of panel 1, then snap to the bottom of panel 1 (second snap point). If the user continues scrolling, then scrolling would snap to the top of panel 2.

When scrolling back up from panel 2 to panel 1, the scrolling would snap to the bottom of panel 1, instead of scrolling all the way to the top of Panel 1. Also if there are two snap points for larger panels, a user will be able to scroll to the bottom end of Panel 1, before moving onto the top of panel 2.

I wish I was fluent enough in js/jQuery to do a solution for this awesome plugin, but alas :(.

@Smashr
Copy link

Smashr commented Feb 14, 2016

Here's a pen derived from the panel snap demo to illustrate the issue. Its almost impossible to keep the bottom of the top two panels in view, let alone click/tap the links at the bottom of the panels.

Also, when navigating back from panel 2 to panel 1, instead of snapping to the bottom of panel 1 first, it snaps all the way to the top of panel 1.

http://codepen.io/anon/pen/yeZZgw

@guidobouman
Copy link
Owner

guidobouman commented Feb 14, 2016

Thanks @Smashr, that's a painfully accurate point you're making. This issue is actually turning into a feature overhaul.

@guidobouman
Copy link
Owner

guidobouman commented May 8, 2018

This feature has gotten an overhaul for v1.0. Watch the releases closely.

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

3 participants