Skip to content
This repository has been archived by the owner on Jun 20, 2022. It is now read-only.

Modal Dialog's "scroll lock" is incorrect #25

Closed
theKashey opened this issue Sep 4, 2018 · 6 comments · Fixed by #103
Closed

Modal Dialog's "scroll lock" is incorrect #25

theKashey opened this issue Sep 4, 2018 · 6 comments · Fixed by #103
Labels
A11Y ♿️ Acessibility enhancement ✨

Comments

@theKashey
Copy link
Contributor

Modal dialog on mounting is setting overflow:hidden directly on body, and remove it on unmount.
Don't do that :)

Problems:

  1. What if you have two modals? It will "unlock" scroll after the first unmount.
  2. It will hide scroll bar on windows machines, casing content to "jump"
  3. It does not work for mobile safari at all 😺

https://github.com/theKashey/react-scroll-locky will do the job
@gregberge
Copy link
Member

Thanks, same I will take a look!

The problem with two modals is easy too solve, others are trickier!

@theKashey
Copy link
Contributor Author

theKashey commented Sep 5, 2018 •
edited
Loading

Just fill the issues, and I'll help.

@gregberge gregberge mentioned this issue Sep 8, 2018
Closed
gregberge added a commit that referenced this issue Oct 18, 2018
@gregberge
feat: make modals accessible
adcb3ea 
Closes #31
Closes #26
Closes #25

BREAKING CHANGE:

"persistent" prop has been removed from Modal, Modal are now
non-persistent.

"sui-modal-backdrop" has been removed, it is now "sui-modal".
@theKashey
Copy link
Contributor Author

  • Add <input type="text" /> into the modal
  • Open it using Safari Mobile
  • Select input
  • ....
  • Safari would use position:absolute instead of position:fixed, and your customer will lose the modal.

@gregberge
Copy link
Member

Hello @theKashey, yeah I know the scroll lock is still incorrect 😣. Can you help on that point?

@theKashey
Copy link
Contributor Author

Sure. Let me just fix one theoretical issue with portals first.

theKashey added a commit to theKashey/smooth-ui that referenced this issue Jan 22, 2019
@theKashey
fix: add scroll-lock to modals, fixes smooth-code#25
83b5901
@theKashey theKashey mentioned this issue Jan 22, 2019
Merged
@theKashey
Copy link
Contributor Author

Solved by creating another library :)

gregberge pushed a commit that referenced this issue Jan 22, 2019
@theKashey @gregberge
feat: add scroll-lock to modals, fixes #25 (#103)
23f35dd
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
A11Y ♿️ Acessibility enhancement ✨
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add scroll-lock to modals theKashey/smooth-ui
2 participants
@gregberge @theKashey