ActionMenu with overflow doesn’t contain scrollbars within its rounded border

[dylanatsmith]

Description

Scrollbar contained within rounded corners of ActionList

Steps to reproduce

1. Put a whole bunch of items in an ActionMenu too short to contain them
2. You may need to force enable scrollbars at OS level

Version

Whatever’s in dotcom

Browser

No response

[github-actions[bot]]

Uh oh! @dylanatsmith, the image you shared is missing helpful alt text. Check your issue body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

[siddharthkp]

Just passing by: overflow: scroll should be on the ActionList inside the Overlay. not on the overflow itself, that way we can ensure the border radius of the Overlay stays intact

[lesliecdubs]

Wondering if someone in Primer Design might be able to take a look and weigh in on the CSS change needed here?

[tallys]

@camertron can you follow up on this if it's React?

[lesliecdubs]

@tallys confirmed via Storybook that I believe this is happening in React.

[francinelucca]

Tested this suggestion: https://github.com/primer/react/issues/4938#issuecomment-2340216670. Setting the overflow inside the ActionList doesn't solve the problem by itself because the ActionList is unaware of the max-height set by the overlay due to the overlay being absolutely positioned. In order for the overflow to work in the ActionList, it would have to be given a max-height as well. Throwing this back in the backlog as a solution for this will require further discussion

[github-actions[bot]]

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

[sunnyi101]

Thanks for submitting this issue! We’ll review it during monthly planning preparation to consider picking up based on current priorities.

[francisfuzz]

Throwing this back in the backlog as a solution for this will require further discussion

@francinelucca - Hi! I was looking at your issue comment in https://github.com/primer/react/issues/4938#issuecomment-2492089164 and came across this last line as a part of our internal work efforts. I am wondering where the discussion for the solution would normally happen: in this issue, in github's Slack channels, or elsewhere?

[francinelucca]

@francisfuzz Usually in our internal triage meetings that happen on Mondays. That's where @sunnyi101's comment above came from. Next steps here would be to scope this work as part of a work stream which @sunnyi101/ @hellojanehere might have more context on. I see it has been included in the Paper Cuts board so there's a chance it gets worked on this week 🤞

[pksjce]

Picked this up as my next papercut

I tested this story https://primer.style/react/storybook/?path=/story/components-actionmenu-examples--setting-max-height And found that the scroll is not working at all. As in there are are a 100 items on that menu but I can see only 13. I can scroll with the down arrow on the keyboard but the vertical scrollbar and mouse scroll don't seem to work

Solution- It looks like Overlay component has styles for data-overflow attribute but doesn't actually set that attribute. Will throw up a PR to fix this. Once this fix is in, we can decide if the scrollbar looks right to mark this issue as resolved.\

[pksjce]

I have merged my fix. IMO we can close this bug. Waiting for confirmation by OP on this.

[crittermike]

Any info about when this fix will make it to dotcom? We're still seeing the issue on the security configurations page.

[francinelucca]

This fix was released as part of 37.28.0, which should already be in dotcom. Can you provide repro instructions for security configurations @crittermike ? we might need to reopen this bug

[crittermike]

Is there any way to see Storybook for that version? I'm looking at https://primer.style/react/storybook/?path=/story/components-actionmenu-examples--setting-max-height which is still broken but that's still 37.27.0.

I can provide repro instructions but I'd need to add folks to an org that has a ton of security configurations to test - are you the right person to add?

[francinelucca]

Tested in the deployment for v37.28.0 can confirm scroll is now present but the border radius still does not look correct, reopening issue

[pksjce]

Hey @francinelucca ! can you put in a screenshot/clip of what you are seeing in this deployment? Is this the buggy behaviour?

[github-actions[bot]]

Uh oh! @pksjce, at least one image you shared is missing helpful alt text. Check https://github.com/primer/react/issues/4938#issuecomment-3083654982 to fix the following violations:

• Images should have meaningful alternative text (alt text) at line 4

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

[francinelucca]

@pksjce need to set scrollbars to always

[github-actions[bot]]

Uh oh! @francinelucca, at least one image you shared is missing helpful alt text. Check https://github.com/primer/react/issues/4938#issuecomment-3084383383 to fix the following violations:

• Images should have meaningful alternative text (alt text) at line 1
• Images should have meaningful alternative text (alt text) at line 4

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

[hellojanehere]

@pksjce moved this back to in progress :)

[pksjce]

@crittermike - Do you have any example of what this should look like in the fixed case scenario? Or is this an issue with scrollbars with all primer components? Thanks for your patience with this!

[francinelucca]

I believe it should look like this (overflow inside ActionList, not Overlay)

[github-actions[bot]]

Uh oh! @francinelucca, at least one image you shared is missing helpful alt text. Check https://github.com/primer/react/issues/4938#issuecomment-3137260425 to fix the following violations:

• Images should have meaningful alternative text (alt text) at line 3

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

[pksjce]

This can now be marked as fixed. Here's what the change looks like. Its a tiny improvement to the scrollbar with border radius.

Before

https://github.com/user-attachments/assets/3d57b929-6554-4698-a130-e387a79b48d0

After

https://github.com/user-attachments/assets/e6535de7-2193-4199-943c-4ff2b288f9c1