Quasar: QDialog doesn't clean up properly when closed and destroyed at the same time, which breaks scrolling throughout the app.

Created on 4 Dec 2019  路  1Comment  路  Source: quasarframework/quasar

Describe the bug
When you hide (by using $refs.dialog.hide()) and destroy (by using v-if on QDialog or some of its parents) dialog at the same time, it doesn't call __preventScroll(false).

It results in class q-body--prevent-scroll persisting on body forever, and all scroll throughout the app stops working.

It's very minor issue, and there are many ways around it, but still this case is confusing and can happen in real life.

Tested on MacOS chrome and iOS safari

Codepen/jsFiddle/Codesandbox (required)
https://codepen.io/leo-buneev/pen/YzPXKLp

To Reproduce
Steps to reproduce the behavior:

  1. Go to 'https://codepen.io/leo-buneev/pen/YzPXKLp'
  2. Click on 'Open dialog'
  3. Click on 'Destroy and hide dialog'
  4. See that body still has class q-body--prevent-scroll

Expected behavior
QDialog cleans up after itself calling __preventScroll(false) among other things. Body doesn't have class q-body--prevent-scroll after dialog is closed.

Platform (please complete the following information):
OS: MacOS
Node: any
NPM: any
Yarn: any
Browsers: Chrome
iOS: 13
Android: didn't test
Electron: didn't test

Additional context
Looking at QDialog, I think I know why it happens.

computed: {
  // equals false right after .hide() was called
  useBackdrop() { ... }
},
watch: {
    useBackdrop (v) {
      // Never called with v = false
      this.__preventScroll(v)
      this.__preventFocusout(v)
    }
},
methods: {
  __cleanup (hiding) {
      clearTimeout(this.shakeTimeout)
      if (hiding === true || this.showing === true) {
        EscapeKey.pop(this)
        this.__updateState(false, this.maximized)
        // Here `useBackdrop` is FALSE (because we called .hide()), but watcher wasn't called yet, 
        // and will never be called because during next tick component will already 
        // be destroyed
        if (this.useBackdrop === true) {
          this.__preventScroll(false)
          this.__preventFocusout(false)
        }
      }
    },
}

Solution could be using different variable (instead of this.useBackdrop) for checking if cleanup methods were called already. I can create PR, but I think the team will probably write better code than I will :)

Thanks!

bug

Most helpful comment

Hi,

Thank you for filing this ticket!
Fix will be available in "quasar" v1.5.8.

>All comments

Hi,

Thank you for filing this ticket!
Fix will be available in "quasar" v1.5.8.

Was this page helpful?
0 / 5 - 0 ratings