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

Modal allows background text selection & scroll #19

Open
marchellodev opened this issue Nov 2, 2022 · 6 comments
Open

Modal allows background text selection & scroll #19

marchellodev opened this issue Nov 2, 2022 · 6 comments

Comments

@marchellodev
Copy link

When the modal window is open, the user can still scroll the background content (via the mouse wheel) or select all text (via Ctrl+A).

To minimally reproduce this:

  1. open https://solid-headless.vercel.app/preview/dialog
  2. open dev tools and add a lot of text after the button html code
  3. open the modal window
  4. try scrolling or pressing Ctrl+A to select all the text

The issue might be related to tailwindlabs/headlessui#1056
Also maybe it has something to do with the fact that I am using astro js, but I do not think so.

@marchellodev
Copy link
Author

A temporary fix inspired by https://github.com/tailwindlabs/headlessui/blob/main/packages/%40headlessui-react/src/components/dialog/dialog.tsx (and a lot of googling):

  function closeModal() {
    setIsOpen(false);
    document.documentElement.style.overflow = "";
    document.documentElement.style.paddingRight = "";
  }

  function openModal() {
    const scrollbarWidth = window.innerWidth - document.body.offsetWidth
    setIsOpen(true);
    document.documentElement.style.overflow = "hidden";
    document.documentElement.style.paddingRight = scrollbarWidth+'px';
  }

@marchellodev
Copy link
Author

The text is still selectable with Ctrl+A though :(

@lxsmnsyc
Copy link
Owner

lxsmnsyc commented Nov 3, 2022

Hmmm interesting behavior, I'll take a look

Edit:

  • For background scrolling, it's noteworthy that the solution is to add pointer-events: none; to DialogOverlay. I do not want to add this to the component by default since scrolling isn't really described in the specification (besides mouse interaction, of course).
  • Ctrl + A theoretically can be "overridden", but like in the specification, there's no mention about what to do with text selection. I'm not sure if this is that of a huge issue (?), so I'm not that convinced yet.

@marchellodev
Copy link
Author

I have tried adding pointer-events: none;, but I can still scroll the background and now when I click on the background, the modal does not close :)

<DialogOverlay class="fixed inset-0 bg-white bg-opacity-40 pointer-events-none" />

@marchellodev
Copy link
Author

btw, when the modal is wrapped inside <dialog>, Ctrl+A works as it is supposed to:)

@lxsmnsyc
Copy link
Owner

lxsmnsyc commented Nov 3, 2022

the modal does not close :)

Right, that went over my head 🤣 so it's not the solution then. Let's see

Ctrl+A works as it is supposed to:)

Well yes but that's an HTML specification, not an ARIA specification actually. There's no mention for managing text selection

Edit:
Funny thing is that the HTML spec doesn't mention anything about this either which is why I don't have any motivation if this should be implemented or not.

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

No branches or pull requests

2 participants