Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closing Nested Modals Disables Page Scrolling #10925

Open
nkeena opened this issue Jan 19, 2024 · 16 comments · Fixed by #10969
Open

Closing Nested Modals Disables Page Scrolling #10925

nkeena opened this issue Jan 19, 2024 · 16 comments · Fixed by #10969
Assignees
Milestone

Comments

@nkeena
Copy link
Contributor

nkeena commented Jan 19, 2024

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

  • You can donate funding to this issue. We receive the money once the issue is completed & confirmed by you.
  • 100% of the funding will be distributed between the Filament core team to run all aspects of the project.
  • Thank you in advance for helping us make maintenance sustainable!
Fund with Polar
@nkeena nkeena added bug Something isn't working low priority unconfirmed labels Jan 19, 2024
@mohammadkamil
Copy link

i also have this issue.

@danharrin
Copy link
Member

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

@zepfietje
Copy link
Member

I've had to temporarily revert the fix (02d4fad) since it caused issues with modals on iOS (they were no longer scrollable).

@zepfietje
Copy link
Member

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

@zepfietje zepfietje self-assigned this Jan 29, 2024
@david-lobo
Copy link

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

@zepfietje
Copy link
Member

@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
Copy link

nabildz commented Mar 28, 2024

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
Copy link
Member

danharrin commented Mar 28, 2024

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

@nkeena
Copy link
Contributor Author

nkeena commented Apr 24, 2024

Any solution here yet?

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

@KhairulAzmi21
Copy link
Contributor

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
Copy link
Member

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.

@polar-sh polar-sh bot added the Fund label Jun 3, 2024
@danharrin danharrin removed the Fund label Jun 3, 2024
@polar-sh polar-sh bot added the Fund label Jun 3, 2024
@danharrin danharrin removed the fund label Jun 4, 2024
@flolanger
Copy link

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
Copy link
Member

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

@flolanger
Copy link

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
Copy link
Member

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

@flolanger
Copy link

flolanger commented Jun 12, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

8 participants