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: Y Scrollbar region always present when using a mouse #826

Open
michael-iden opened this issue Oct 28, 2024 · 15 comments · May be fixed by #827
Open

Modal: Y Scrollbar region always present when using a mouse #826

michael-iden opened this issue Oct 28, 2024 · 15 comments · May be fixed by #827

Comments

@michael-iden
Copy link
Contributor

Expected behavior
Scrollbar region only is shown only when necessary based off the contents of the modal overflowing

Actual behavior
Y Scrollbar region always present when a mouse input device is present

Steps to reproduce the issue

  1. Plug in a mouse
  2. Open a modal
  3. See that the modal doesn't scroll but the scroll region is still visible

Screenshots or code
Image

Pharos version
14.4.0

Your environment

  • OS: macOS
  • Browser: Chrome, Firefox, Safari
  • Version: Latest for each as of 2024-10-28

Additional information
https://github.com/ithaka/pharos/blob/develop/packages/pharos/src/components/modal/pharos-modal.scss#L98

@brentswisher
Copy link
Contributor

brentswisher commented Oct 28, 2024

🤔 This doesn't happen in the storybook examples that I can see. I wonder if something on the css of that page it making it think that the content is overflowing the modal even though it doesn't look like it.

@brentswisher
Copy link
Contributor

🤔 The plot thickens, I don't see it even on that page you shared in the screenshot:
Image

@michael-iden
Copy link
Contributor Author

@brentswisher do you have a mouse plugged in? There are some mac level settings that can also affect this

Image

@michael-iden
Copy link
Contributor Author

Here is storybook for me with what I believe to be default mac settings and have a mouse connected
Image

@brentswisher
Copy link
Contributor

brentswisher commented Oct 28, 2024

Hmm, interesting. I do have a Magic Mouse connected via Bluetooth. My settings match what you have selected.

@brentswisher
Copy link
Contributor

I vaguely remembered a similar conversation and it looks like this maybe came up once before in #531 but was deemed out of scope - not that it shouldn't be looked into, but just as additional info.

Are you using a wired mouse?

@daneah
Copy link
Member

daneah commented Oct 28, 2024

@michael-iden @brentswisher as mentioned this has been raised in the past (offline as well) and we have closed citing the macOS-level settings as the fix. I believe that if a user has a mouse plugged in such that the Pharos modal shows this scroll bar, they would have a similar experience regularly on other sites across the web as well.

@michael-iden
Copy link
Contributor Author

michael-iden commented Oct 28, 2024

I am using a wired, generic Dell mouse. And possibly due to the magic mouse having touchpad-like capabilities, that prevents the scroll regions from appearing, although I don't know exactly why that is deemed different than a physical scroll wheel?

Whether the scroll region appears also depends what overflow behavior is. If we have overflow-y: auto instead of overflow-y: scroll (or remove overflow-y) from the line linked in the description, the scrollbars aren't shown.

Furthermore, it looks like if the modal would possibly overflow, the whole modal including the overlay scrolls, not just the contents of the modal.

Screen.Recording.2024-10-28.at.9.22.22.AM.mov

@daneah
Copy link
Member

daneah commented Oct 28, 2024

Whether the scroll region appears also depends what overflow behavior is.

Right—this is kind of what we'd established in past looks at this.

Furthermore, it looks like if the modal would possibly overflow, the whole modal including the overlay scrolls, not just the contents of the modal.

This one might be something I noticed recently too and am surprised by; I would maybe expect the covered content not to scroll, but I might also remember some discussion around this from an a11y standpoint that I'd need to dig to find.

@michael-iden
Copy link
Contributor Author

michael-iden commented Oct 28, 2024

Talked more with @daneah and we will remove overflow-y from the line linked in the description to not show the scrollbar on the modal content since it never seems to have any effect (div will never actually overflow and become scrollable) given the other overflow/scrolling rules we have as part of the greater modal including the overlay

@brentswisher
Copy link
Contributor

Could you restate a little of that conversation here? It seems like we want the modal to overflow, not the content behind the modal? It feels like that is a bug to me (and potentially an a11y issue), it that the desired effect?

@brentswisher
Copy link
Contributor

Right now #827 is feeling a little like updating our UI to match a bug, instead of resolving the underlying issue, so just trying to understand the thought process there, its very possible I am missing some piece of it though.

@daneah
Copy link
Member

daneah commented Oct 28, 2024

@brentswisher essentially the modal currently expands to meet the height of its content, and the content does not scroll at all—rather, the modal acts a little like a sheet, and scrolling pulls the part of the sheet that's off-screen further up. So removing the overflow-y property would not impact this behavior at all; it would only have the effect of removing the scroll bar. I think the interaction seems sound, but if others thing the modal should only ever be as tall as the viewport and have the content scrolling within, I am open to that. We would need to consider things like making sure the close button and maybe the title stays on screen?

@michael-iden
Copy link
Contributor Author

I can add this to the next pharos ops discussion so we can look at the various considerations with the designers and accessibility experts. Even if we did change the scrolling/overflow behavior more to what you describe, I posit that overflow-y: scroll should be removed and let the default overflow-y: auto behavior take over here so we only get those scroll regions when necessary

@brentswisher
Copy link
Contributor

Ah, that makes more sense, thank you. I would be fine with moving forward with removing it.

I do still worry that we have a bug still here somewhere, as I don't think this should be possible, where the content scrolls behind the modal:
https://github.com/user-attachments/assets/010d45bd-cf2b-4ace-95e9-f571676b0d31

But, that's maybe a separate thing.

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

Successfully merging a pull request may close this issue.

3 participants