quasarframework / quasar

Quasar Framework - Build high-performance VueJS user interfaces in record time
https://quasar.dev
MIT License
25.81k stars 3.51k forks source link

QPullToRefresh preventing scroll on touch screens #3644

Closed jeancaffou closed 5 years ago

jeancaffou commented 5 years ago

A possible resurface of #166

Using Quasar v1.0.0.beta-11

QPullToRefresh prevents touch scrolling on mobile devices in certain conditions: When QPullToRefresh is wrapped in a container with fixed height and overflow-y: auto, and child element overflows the parent, it is impossible to touch scroll the child, possibly because QPullToRefresh doesn't propagate touch events to the child.

It seems that somebody else has a simmilar issue: https://forum.quasar-framework.org/topic/3164/q-pull-to-refresh-prevents-scrolling/2

pdanpdan commented 5 years ago

Just add scroll-y class on container. Like this:

<div style="height: 400px;" class="scroll-y">
  <q-pull-to-refresh ...
jeancaffou commented 5 years ago

What overflows is actually the child of QPullToRefresh (so container of QPullToRefresh and QPullToRefresh have height of for example 90vh with a sticky footer / header), and scroll-y class just adds overflow-y: auto, which is already added on the child.

jeancaffou commented 5 years ago

Another thing to note is that I'm using vue-virtual-scroll-list component inside of QPullToRefresh, which needs its own scrolling logic.

Unfortunately quasar doesn't have a virtual list component.

pdanpdan commented 5 years ago

Please provide example in codepen showing the problem. By the way, did you add the class and test?

jeancaffou commented 5 years ago

I did add the class and test, no luck.

Here is a SFC sample that reproduces the problem.

<template>
  <q-pull-to-refresh>
    <q-list style="width: 100%; height: 300px; border: 1px solid red;" class="scroll-y">
      <q-item v-for="i in test" clickable v-ripple :key="i">{{ i }}</q-item>
    </q-list>
  </q-pull-to-refresh>
</template>

<script>
export default {
  name: 'Test.vue',
  computed: {
    test () {
      let r = []
      for (let i = 0; i < 1000; i++) {
        r.push(i)
      }
      return r
    }
  }
}
</script>
pdanpdan commented 5 years ago

You cannot use it like this.

Take a look here for different supported use cases: https://github.com/quasarframework/quasar/blob/dev/quasar/dev/components/components/pull-to-refresh-part1.vue

In your case you should do something like this:

<template>
  <q-pull-to-refresh @refresh="done => done()">
    <q-list style="width: 100%; height: 300px; border: 1px solid red;" class="scroll-y" @touchstart="preventPull">
      <q-item v-for="i in test" clickable v-ripple :key="i">
        {{i}}
      </q-item>
    </q-list>
  </q-pull-to-refresh>
</template>

<script>
export default {
  name: 'Test',
  computed: {
    test () {
      let r = []
      for (let i = 0; i < 1000; i++) {
        r.push(i)
      }
      return r
    }
  },
  methods: {
    preventPull (e) {
      let parent = e.target

      while (parent !== void 0 && !parent.classList.contains('scroll-y')) {
        parent = parent.parentNode
      }

      if (parent !== void 0 && parent.scrollTop > 0) {
        e.stopPropagation()
      }
    }
  }
}
</script>
jeancaffou commented 5 years ago

Your solution doesn't fully fix the problem, when the scroll position is 0, I am unable to touch scroll down. Scrolling up correctly triggers refresh. Only if the scroll is > 0, then touch scrolling works for up and down..

Adding this logic (detecting drag direction) into the component seems like a huge overhead. It would be better if the QPullRefresh component would be able to handle this.

pdanpdan commented 5 years ago

How would you scroll higher than 0? And the component takes care of this for the parent. It cannot do what you should do in userland.

jeancaffou commented 5 years ago

I mean if the scroll position is exactly 0, then touch still doesn't work - event gets propagated to QPullRefresh.

Only if I scroll down with the mouse wheel or by dragging the scrollbar, then touch works.

Try testing your solution in chrome devtools by enabling the device toolbar.

pdanpdan commented 5 years ago

I really don't understand what you mean. I just gave you an working, tested example. If you don't want QPullToRefresh to be active at all on the list just use @touchstart.stop. You should provide a codepen showing your problem.

jeancaffou commented 5 years ago

Take a look at this screen record: https://youtu.be/FMyAioQ2Vlg

Here I'm using your unmodified example.

As you can see at the beginning, touch scrolling doesn't work. Then after a few seconds I use the scroll wheel on the mouse (let's call this cheating) and touch scrolling works in both directions. When I scroll back to top and release (right at the end of the video), then I am again unable to scroll down with touch.

If I scroll up when I'm at top position I want QPullRefresh to work, which it does. This is OK. (I also think that it is correct that it works only at top scroll position and not otherwise, which by default it does) But I also want to be able to scroll down, which I can't.

I can't see how I can make it clearer than this, Maybe @nothingismagick can also explain it since he got it right away on the forum.

pdanpdan commented 5 years ago

As I asked you already a few times, please provide a codepen.

BTW, what mobile?

jeancaffou commented 5 years ago

Codepen: https://codepen.io/anon/pen/RdvWMg

I am using Quasar V1. The bug is not present on v0, and does need this preventPull fix.

Any mobile, but on the screen cast I am using chrome developer tools with device toolbar enabled.

pdanpdan commented 5 years ago

Ok, I got it - my build and the official build have different implementations of VTouchPan.

Please make a short test for me - add "quasar": "https://github.com/pdanpdan/quasar#quasar-v1.0.0-beta.11.1-gitpkg", in your application's package.json under "dependencies".

Do a yarn && quasar dev and tell me if you can still reproduce the problems.

jeancaffou commented 5 years ago

Using your version of Quasar fixes the problem.

I do still need the preventPull method in order to scroll up correctly.

On Quasar v0 like I tested on codepen this method was not needed.

jeancaffou commented 5 years ago

When is this fix expected to be merged with the main quasar branch? Testing this on Quasar v1--24 still has the same problem. @pdanpdan @rstoenescu

rstoenescu commented 5 years ago

Fix will be available in v1-rc.1

jeancaffou commented 5 years ago

In Quasar v1.0.5 QPullToRefresh is not showing up in this use case

pdanpdan commented 5 years ago

Please follow up in unified #5051