Off-Canvas contentScroll inner scrollbox

[SassNinja]

I've been facing an issue with the off-canvas option contentScroll: false when using an inner box in the off-canvas with a specific height that is supposed to be scrollable.
Due to the touchmove and touchstart event listeners you can't touchmove on mobile devices within that box.

You can find an example here:
https://codepen.io/KaiTheGreat/pen/mMBZjp
Simply open the codepen on a mobile device (or use e.g. the chrome dev tools with touch emulation), click the red alert button and try to touchmove in the gray box.

This PR fixes the issue be adding additional listeners to [data-off-canvas-scrollbox] which is supposed to be assigned to the inner box.
I've extended the visual test offcanvas/nested.html to show the fix.

@kball @IamManchanda @brettsmason what do you think about this?

[IamManchanda]

This looks good to me! Thanks for the pen!

[kball]

I think we'd need to document this approach... that said, part of me wonders if we could fix the original handler to not need this... perhaps by checking the target in the event handler and not preventing default/propagation for inner elements?

[SassNinja]

@kball I 100% agree: we should definitely find a way without extensive usage of data-off-canvas-scrollbox!

In a current project I've added dozen of these attributes but it's still not working in all cases. Precisely I've great problems with a slider that is dynamically build and uses touch events (which are getting prevented by the off-canvas).
So a solution without that attribute is probably the best way.

As far as I see the whole touch event listeners are primarily created for iOS.
I'm still trying to understand what's exactly happing in _stopScrollPropagation and _recordScrollable so any help on this is appreciated.

Any idea how to stop propagation (so the body's _stopScrolling doesn't take effect) for the off-canvas and it's scrollable children without an additional class/selector?

[SassNinja]

I'm coming back to this issue since I'm more and more reaching the limits of the current touch handling of the off-canvas (if contentScroll === false).

Firstly I've come to the conclusion there's probably no way around the additional data-off-canvas-scrollbox attribute. Although I'd prefer not using it I don't see any alternative because the inner scrollbox has to be treated the same way as the outer off-canvas. Otherwise there will always be the problem the page content (body) gets scrolled when the end of the inner scrollbox is reached (iOS issue).

So imo it's unavoidable to have such a markup

<div class="off-canvas" id="test" data-off-canvas data-content-scroll="false">
    <p>Lorem ipsum</p>
    <div id="innerTest" data-off-canvas-scrollbox>
        <p>dolor sit amet</p>
    </div>
</div>

Secondly I've got an UI issue with the scrollbox feature.
Although my scrollbox attribute works the user may get confused if the inner scrollbox is not completely visible (e.g. if not having scrolled down enough) because after having reached the end of the scrollbox, further scroll down does not continue to scroll down the outer off-canvas.
So the user has to put his finger outside the scrollbox to be able to reach the rest of the off-canvas.

It would be more intuitive to cause a scroll down/up in the off-canvas if the user has reached the end of the inner scrollbox and is still touch moving down/up.

I'm currently working on this additional scroll continue feature.
But regarding the markup I will stick to the data-off-canvas-scrollbox attribute.

@kball hope this is ok for you?
To get away from preventing default / stopping propagation doesn't seem to be possible in my eyes so far.

[SassNinja]

Done!

I've now extended the present _stopScrollPropagation function to affect the outer off-canvas as well (if called from the scrollbox).
Seems to work fine for me. I'm not 100% happy with looking for the outer off-canvas (elem.closest('[data-off-canvas]')) on every touch move. But since it's limited to the boundary area of the scrollbox and since this.$element is not available in this scope it's good to go with in my eyes.

@kball @IamManchanda would be great if can review this PR!
(just comment in the further dummy text in the visual test nested.html line 209-213 to see what my latest commit does)

p.s.

if you wonder why I've also added [data-off-canvas-scrollbox-outer] to the parent selector:
This provides the possibility to explicitly specify the outer parent container in cases the default [data-off-canvas] doesn't fit.

[SassNinja]

@ncoden I've just run into the same issue for another project.
Would you review this PR and merge if ok?

I've created a new, simple codepen that shows the issue pretty well.
Just open the dev tools with touch emulation (or any real mobile device) and try to touchmove down the drilldown in the offcanvas.
https://codepen.io/KaiTheGreat/pen/oELvLB

Without my fix (checkbox unchecked) this doesn't work.

[ncoden]

@SassNinja I am not able to make it works with the codepen you given. I tested with both Firefox dev tools and BrowserStack (safari on iOS 11). Should the drilldrow scroll on mobile ?

[SassNinja]

@ncoden I think FF doesn't emulate the touch behavior appropriately.
Try the Chrome dev tools, click the 'Toggle device toolbar' icon, choose iPad landscape and try to touch move (not scroll!) the drilldown in the offcanvas.

The drilldown is supposed to be moved.
Without my PR (checkbox unchecked) this doesn't work.

[SassNinja]

Maybe it helps if you take a look at this screen capture to understand the issue

[ncoden]

Hmm.. I am unable to reproduce the "patch" in https://codepen.io/KaiTheGreat/pen/oELvLB

See: https://giphy.com/gifs/l3nSOjb8rbfQIfYBi (in Giphy because this is a huge gif)

[DanielRuf]

Which Chrome version was used for testing and what can we do here? Is some more testing needed?

[SassNinja]

I think we need an example which always reproduces the issue in talk (not only on my side).
In general it's seems to be really tricky error-prone if you've got more than just 'simple content' in the off-canvas on mobile / touch devices.

I'll try to create another, more complex codepen that hopefully always shows the issue.

[SassNinja]

The following codepen shows an off-canvas with advanced/complex content, that causes issues on mobile if having set contentScroll:false
https://codepen.io/KaiTheGreat/pen/EERrBV

@ncoden would you please check again? Do you have problems to touchmove on mobile this time (if checkbox unchecked)?

In the course of creating this codepen I also reviewed my code for this feature to see if there's a chance to make it more simple. But I didn't find any way to keep all the content within the off-canvas working without offering [data-off-canvas-scrollbox] and [data-off-canvas-scrollbox-outer].

The scrollbox is required because the touch event listeners must also be added to every scrollable container within the off-canvas (in particular needed for iOS – not reproducible with chrome dev tools). The outer scrollbox is required in cases where the off-canvas element is not the scrollable container of the content (e.g. when using a 'pseudo fixed body' as shown in my codepen).

If this PR is good so far (and the issue in talk reproducible) I'll adjust the docs and explain the scrollbox feature there in detail.

A note regarding the noUiSlider (in the pseudo accordion):
this doesn't work even if the feature checkbox is checked – but it's a problem of the slider because it's too dependent on the body touchmove.
I've just added to slider in my codepen to show a more detailed use case 😉
(there's already a PR to fix that problem: leongersen/noUiSlider#821)

[SassNinja]

Poke @ncoden

Are you able to reproduce the issue (and the fix) with my latest codepen?

[ncoden]

I missed your reply. I'll check that again when I have some time.

[SassNinja]

Poke @ncoden

I'm curious if you are able to reproduce the issue now with the latest codepen.

Would like to bring this PR across the line so I may update some projects ;)

[SassNinja]

I'd like to finish this PR...

My latest codepen is still relevant:
https://codepen.io/KaiTheGreat/pen/EERrBV

Since the codepen is probably a very very specific use case (and bcz it should be done anyway) I've added a section in the docs to describe this incl. a simple example.

@ncoden @DanielRuf would be awesome if one of you could manage to take a look!

[joeworkman]

This pull request has been mentioned on Foundation Open Source Community. There might be relevant details there:

https://foundation.discourse.group/t/foundation-for-sites-v6-6-0-is-here-farout/30/1