filamentphp / filament

A collection of beautiful full-stack components for Laravel. The perfect starting point for your next app. Using Livewire, Alpine.js and Tailwind CSS.
https://filamentphp.com
MIT License
19.43k stars 2.98k forks source link

Closing Nested Modals Disables Page Scrolling #10925

Closed nkeena closed 3 months ago

nkeena commented 10 months ago

Package

filament/filament

Package Version

v3.2.6

Laravel Version

v10.41.0

Livewire Version

v3.0.0.0

PHP Version

PHP 8.2.11

Problem description

When opening a nested modal, scroll lock is lost on the backdrop, allowing you to scroll the backdrop. After closing the nested modal (using the cancel button, x icon, or clicking off the modal) scroll lock is resumed on the backdrop. After closing the parent (or first) modal, page scrolling becomes disabled until refreshing the page or navigating away.

There are no console errors and this problem seems to occur on all browsers when using npm run dev or npm run build with vite.

Expected behavior

When opening a nested modal (or slide-over), scroll lock on the backdrop should not be lost. After closing all modals, you should be able to resume scrolling the page without having to refresh or navigate away.

Steps to reproduce

  1. Install the reproduction repository locally
  2. Run composer install
  3. Run the migrations and seeders php artisan migrate --seed
  4. Create a filament user php artisan make:filament-user
  5. Run npm run build
  6. Login using your credentials /admin/login
  7. Navigate to the orders table and set the number of visible order to "all" so that the page scrolls
  8. Click "New order"
  9. Click "+" suffix action on either the product or customer input
  10. Observe the backdrop scrolls
  11. Close all modals
  12. Observe the page won't scroll
  13. Observer no console errors
  14. Refresh the page and observe the page is scrollable again

Reproduction repository

https://github.com/nkeena/filament-modal-bug

Relevant log output

No response

Donate 💰 to fund this issue

Fund with Polar

mohammadkamil commented 10 months ago

i also have this issue.

danharrin commented 10 months ago

This is an Alpine.js bug, so I've replaced their scroll handling implementation with a separate library

zepfietje commented 10 months ago

I've had to temporarily revert the fix (https://github.com/filamentphp/filament/commit/02d4fad9db8cdcc261c6ab5f2a09b3599a6504c3) since it caused issues with modals on iOS (they were no longer scrollable).

zepfietje commented 10 months ago

Will look into using this fork: https://github.com/rick-liruixin/body-scroll-lock-upgrade.

david-lobo commented 9 months ago

I'm having this problem too. is there any news on a fix for it please?

zepfietje commented 9 months ago

@david-lobo I haven't found the time to work on this issue yet. If you want to help fixing it, try using the scroll lock package fork I posted above and test if any of the iOS issues still occur.

Let me know if you want to work on this and what your findings are. 🙂

nabildz commented 8 months ago

i think this workaround is best if you don't want to preserve the scroll state, just dispatch a page reload event after closing the last modal

FilamentView::registerRenderHook( 'panels::body.end', static fn () => '<script>window.addEventListener("reload", () => window.location.reload())</script>', );

$this->dispatch('reload');

Dispatch a page reload after create action for example

Tables\Actions\CreateAction::make()->after(function ($data,$record) { $this->dispatch('reload'); })

danharrin commented 8 months ago

That's the same as just doing a redirect really from the action

nkeena commented 7 months ago

Any solution here yet?

In the meantime, is there a way to trigger a page refresh when the parent modal closes?

KhairulAzmi21 commented 6 months ago

Any solution here yet?

In the meantime, is there a way to trigger a page refresh when the parent modal closes?

Do you find any temporary solution ?

Having the same problem right now :)

danharrin commented 6 months ago

If you want to help fixing it, try using the scroll lock package fork I posted above and test if any of the iOS issues still occur.

flolanger commented 5 months ago

Alright, I used the suggested fork body-scroll-lock-upgrade instead of body-scroll-lock and uncommented all changes in your initial PR for the modal js and blade view. Sadly, this fork still has the iOS Issue, where you can't scroll the modal. It fixes the alpine bug though. :/

Tested on an iPhone 15 Pro.

danharrin commented 5 months ago

Thank you for playing a part there in testing, it is appreciated. Not sure where to go from here then

flolanger commented 5 months ago

Thank you for playing a part there in testing, it is appreciated. Not sure where to go from here then

Would it be possible to contribute to alpine, if that is a known issue there?

danharrin commented 5 months ago

Potentially, I don't know if I have the knowledge to do so but we can look into it

flolanger commented 5 months ago

Today I asked a frontend dev, and he said that (with the alpine version) this could be a scoping issue. Because both modals use "isOpen", this could lead to scoping issues when dealing with multiple instances. Each modal should have its own variable.

Maybe this would be a soultion.

VincentLahaye commented 4 months ago

Just spent 3 hours figuring this out.

It's far from ideal but "working" on Windows and iPhone.

I implemented this commit : https://github.com/filamentphp/filament/pull/10969/commits/671891382c2b8fb39c1f67e59cd375c37c24cd3c

Overriden this view : /resources/views/vendor/filament/components/modal/index.blade.php To add a basic check to disable the JS Scroll Body Locks if IOS.

x-data="{
        isOpen: false,

        livewire: null,

        isIOS: /iPad|iPhone|iPod/.test(navigator.userAgent) && !window.MSStream,

        close: function () {
            this.isOpen = false

            this.$refs.modalContainer.dispatchEvent(
                new CustomEvent('modal-closed', { id: '{{ $id }}' }),
            )

            this.$nextTick(() => {
                if (document.getElementsByClassName('fi-modal-open').length || this.isIOS) {
                    return
                }

                window.clearAllBodyScrollLocks()
            })
        },

        open: function () {
            this.isOpen = true

            if(!this.isIOS) {
                window.clearAllBodyScrollLocks()
                window.disableBodyScroll(this.$root)
            }

            this.$refs.modalContainer.dispatchEvent(
                new CustomEvent('modal-opened', { id: '{{ $id }}' }),
            )
        },
    }
danharrin commented 4 months ago

That sounds like a good workaround, I would welcome a PR

danharrin commented 3 months ago

Addressed by @wychoong in #13740