lazd / iNoBounce

Stop your iOS webapp from bouncing around when scrolling
http://blog.lazd.net/iNoBounce/
BSD 2-Clause "Simplified" License
1.34k stars 400 forks source link

iNoBounce breaks horizontal scrolling #50

Open JosephShenton opened 6 years ago

JosephShenton commented 6 years ago

Hi, I have a DIV purposely set to scroll horizontally however with iNoBounce it breaks this feature.

You can see the CSS and HTML here.

<div class="scrolls">
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
                <img src="http://placehold.it/50x50" />
              </div>
.scrolls {
    overflow-x: scroll;
    overflow-y: hidden;
    height: 80px;
    white-space:nowrap
}
.imageDiv img {
    box-shadow: 1px 1px 10px #999;
    margin: 2px;
    max-height: 50px;
    cursor: pointer;
    display:inline-block;
    *display:inline;/* For IE7*/
    *zoom:1;/* For IE7*/
    vertical-align:top;
 }
lazd commented 6 years ago

Duplicate of #2?

lazd commented 6 years ago

@JosephShenton ?

monokee commented 6 years ago

I ran into the same limitation and ended up writing my own micro tool for this with direction detection and a few perfomance optimizations. It allows for both horizontal and vertical scrolling: https://gist.github.com/monokee/685fb5eccaf4cc1d01cf4661cefcc84c See comments for usage.

lazd commented 6 years ago

@monokee a pull request would have been nicer, it's pretty clear you based your work on iNoBounce.

monokee commented 6 years ago

tbh I've never done a pull request before and don't really know how that works. feel free to adopt anything you might find useful (that's why I'm sharing it here)

monokee commented 6 years ago

I refactored and improved the approach a little by moving most of the pre-selection logic into the touchstart listener to greatly simplify the touchmove handler and further optimize performance . All that is needed for reliable results should be these two document-level event handlers (set {passive: false} in supported environments!)

// Defined outside of continuous input handlers to tame GC
let finger, el, style, overflowY, overflowX, initX, initY, atTop, atBtm, targetScrollAxis;

// document.addEventListener('touchstart', touchStart);
const touchStart = e => {
    el = e.target;
    initX = e.changedTouches[0].pageX;
    initY = e.changedTouches[0].pageY;
    atTop = false;
    atBtm = false;
    // Find scrollable element
    while (el !== document.body) {
        style = getComputedStyle(el)
        if (!style) break;
        overflowY = style.getPropertyValue('overflow-y');
        overflowX = style.getPropertyValue('overflox-x');
        // Detect scrollable elements scroll axis
        if (el.scrollHeight > el.offsetHeight && (overflowY === 'auto' || overflowY === 'scroll')) {
            targetScrollAxis = 'y';
            // Top or bottom end
            if (el.scrollTop < 1) {
                atTop = true;
            } else if (el.scrollTop + el.offsetHeight === el.scrollHeight) {
                atBtm = true;
            }
        } else if (el.scrollWidth > el.offsetWidth && overflowX !== 'hidden') {
            targetScrollAxis = 'x';
        } else {
            targetScrollAxis = '';
        }
        if (targetScrollAxis) {
            return;
        } else {
            el = el.parentNode;
        }
    }
};

// document.addEventListener('touchmove', touchMove, {passive: false});
const touchMove = e => {
    finger = e.changedTouches[0];
    switch (targetScrollAxis) {
        case 'y':
            if ((atTop && initY < finger.pageY) || (atBtm && initY > finger.pageY))
                e.preventDefault();
            return;
        case 'x':
            if (Math.abs(finger.pageY - initY) > 5)
                e.preventDefault();
            return;
        default:
            e.preventDefault();
            return;
    }
};

Tested on iOS 11.3 Safari & Chrome Expected Behavior: all iNobounce features + unrestricted horizontal scrolling + no css requirements + performance improvements

This is so trivial that I feel like I'm missing something. Would be interested in more tests and suggestions for futher improvement!

lazd commented 6 years ago

@monokee does it scroll horizontally when you're scrolled to the top or bottom of the element?

monokee commented 6 years ago

@lazd I've tested this on elements that are either horizontally or vertically scrollable, not both at the same time. I should test for that and report back.

monokee commented 6 years ago

In case an element is scrollable on both X and Y axes here's what will happen:

If we're at the top or bottom end of a Y axis scrollable, that information, along with the initial touch coordinates is passed from the touchstart to the touchmove:

targetScrollAxis = 'y';
atTop || atBottom = true;

If we're at the top or bottom of a Y axis scrollable element the touchmove handler additionally checks if we're attempting to swipe away from the top and prevents the move only if we are in fact pulling down, thus preventing the default behavior. If we were to strictly pan along the x axis the check for initY < finger.pageY would fail because a strict x-pan move would not alter the current y coordinate and thus the default behavior (panning horizontally) would not be prevented. This is expected and exactly what we want and the same is true for the bottom end.

In real world usage it can happen that an intended x-pan on an element that is scrollable on both axes also slightly moves on the y axis. If this unintended y-move is in the direction away from the elements current top or bottom boundary the x-move will inevitably be prevented and there is nothing that can be done to circumvent this. Again, this only applies if element is scrollable on both axes simultaneously && element is at the top or bottom boundary && if we unintentionally move along the y-axis when we actually want to pan-x.

Definitely something to keep in mind as a potential edge case but probably quite rare in real world situations. Everything else should be working exactly as expected according to my tests.

Edit: Actually I'm checking for either X or Y scrollables in the above implementation, the handler would need a slight modification to allow for XY scrollable elements (if + if instead of if + else if essentially). I'll patch that up in the morning.

monokee commented 6 years ago

Edit: Actually I'm checking for either X or Y scrollables in the above implementation, the handler would need a slight modification to allow for XY scrollable elements (if + if instead of if + else if essentially). I'll patch that up in the morning.

Looks like I was overthinking the problem. The code I posted above already selects y over x when an elment is scrollable on both axes so the edge case stands but the code doesn't need modification. I did look at the iNoBounce source and don't quite get why it's looking for the 'overscroll: touch' property or if there are any scenarios where it is indeed better to continuously search for scrollables during touchmove so there might be something I'm missing. Thoughts? @lazd

Theoton commented 5 years ago

My project also has this problem. Touchmove in horizontal is invalid. It can only scroll by touching with two fingers.

Theoton commented 5 years ago

I can only turn it off where I need to roll horizontally

fenixi0 commented 4 years ago

Hi guys, any solution for this? iNoBounce works great for my project as it fixes all the scrolling issues it had, however, it breaks the horizontal scrolling of one element.

How can it be fixed?

@monokee I tried your script, but couldn't make it work. do I just need to add the JS file to my footer? or what else? could you add some instructions? I'm just using plain html, jquery and js files.

thank you so much!

JosephShenton commented 4 years ago

It should have been fixed.

himynameisubik commented 3 years ago

Horizontal scrolling is still bugged on my end if iNoBounce 0.2.0 is used. It requires multiple "touches" to initiate the scrolling and most of the time it will be just "stuck". Any updates on this horizontal scroll behavior?

lazd commented 3 years ago

@himynameisubik this PR was supposed to fix it, but apparently it causes bounce https://github.com/lazd/iNoBounce/pull/65#issuecomment-653103524

himynameisubik commented 3 years ago

Hmm, if it fixes the stuck horizontal scrolling but re-introduces the bouncing there is no point to it? Or am I missing something?

lazd commented 3 years ago

@himynameisubik yeah that's what was reported, hence not merging... If you want to test other PRs and see if they get us closer, I'm down to investigate further.

himynameisubik commented 3 years ago

I tried the most of them but none of them worked for me unfortunately.

The current iNoBounce and most others that have the scroll locking on y=0 do work scrolling horizontally if y > 0.

lazd commented 3 years ago

@himynameisubik hmm, let me know if you can hack any of the code to make it work the way it's expected! PRs welcome.