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

Fix: Prevent scrolling and interactions below a modal on mobile #267

Merged

Conversation

rbf
Copy link
Contributor

@rbf rbf commented Oct 22, 2022

pointer-events: none; does not have the expected effect on mobile. I added touch-action: none; which seems to work as expected (at least on the use cases I tested).

Please note that I only added touch-action: none; in scss/components/_modal.scss and the ran npm run build as suggested in the contribution guidelines. The diff looks a bit big to my expectations. Hopefully I didn't mess something up.

In any case, thanks for your work on pico! ❤️

Cheers!

@lucaslarroche
Copy link
Member

lucaslarroche commented Oct 23, 2022

Hi @rbf,
Thank you ❤️. That is definitively a bug, and I will merge your PR.

Weird. If I rebuild your branch, I have differences.
I guess it’s related to a difference in our caniuse database... but it should be locked in the package-lock.json
You could try to delete node_modules and reinstall npm i
If you still have a large diff after, I will simply rebuild on my side.

Also, I am working on v2 (PR, Project). If you wish, you can also open another PR against v2 branch.

Note, v2 is using yarn:

  • Install: yarn
  • Build: yarn build

@rbf
Copy link
Contributor Author

rbf commented Nov 7, 2022

Hi @lucaslarroche,

Thanks for you kind reply.

I guess it’s related to a difference in our caniuse database... but it should be locked in the package-lock.json. You could try to delete node_modules and reinstall npm i

Indeed, I must have had something weird going on in my node_modules folder. I deleted it and rebuilt the project as you suggested and the diff is now much smaller: it went from changes in 22 files down to 9 files. I force-pushed the changes to the same branch. I hope that is ok for you.

Also, I am working on v2 (#261, Project). If you wish, you can also open another PR against v2 branch.

Of course, here you are: #273.

Cheers.

@jacksongoode
Copy link

I was just about to post this issue as well, glad there is a fix!

@lucaslarroche lucaslarroche merged commit 1c218c5 into picocss:dev Dec 26, 2022
@lucaslarroche lucaslarroche mentioned this pull request Dec 26, 2022
@lucaslarroche
Copy link
Member

@rbf, sorry for the delay.
I just merged your PR into dev, I will add a few more fixes before merging into master.
Thank you again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants