gulp-handlebars icon indicating copy to clipboard operation
gulp-handlebars copied to clipboard

Tests should be version independent

Open lazd opened this issue 10 years ago • 0 comments

Right now, tests have to be updated whenever the Handlebars compiler changes its output, compatible version, etc.

lazd avatar Sep 16 '14 15:09 lazd

Duplicate of #2?

lazd avatar May 05 '18 03:05 lazd

@JosephShenton ?

lazd avatar May 15 '18 00:05 lazd

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.

monokee avatar Jun 12 '18 20:06 monokee

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

lazd avatar Jun 12 '18 20:06 lazd

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 avatar Jun 12 '18 21:06 monokee

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!

monokee avatar Jun 16 '18 00:06 monokee

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

lazd avatar Jun 16 '18 00:06 lazd

@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 avatar Jun 16 '18 00:06 monokee

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 avatar Jun 16 '18 00:06 monokee

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

monokee avatar Jun 16 '18 12:06 monokee

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

Theoton avatar Jan 25 '19 03:01 Theoton

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

Theoton avatar Jan 25 '19 03:01 Theoton

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!

fenixi0 avatar Mar 26 '20 06:03 fenixi0

It should have been fixed.

JosephShenton avatar Mar 29 '20 01:03 JosephShenton

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?

himynameisubik avatar Jan 12 '21 11:01 himynameisubik

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

lazd avatar Jan 12 '21 17:01 lazd

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

himynameisubik avatar Jan 12 '21 22:01 himynameisubik

@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.

lazd avatar Jan 12 '21 23:01 lazd

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

  • They either not prevent the bouncing which makes me unsure on what it even does/is needed for.
  • They have the horizontal scroll-locking issue if the scrollable container is on y=0

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

  • Update: Nevermind, seems like y > 0 doesn't change much most of the time. Still requires multiple touches to be able to scroll.

himynameisubik avatar Jan 13 '21 09:01 himynameisubik

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

lazd avatar Jan 13 '21 19:01 lazd