Add option to Dialog to permit/disable page scrolling

[camertron]

What are you trying to accomplish?

The SelectPanel component uses parts of the Dialog component under the hood, which comes with custom CSS that prevents the page from scrolling when the dialog is opened as a modal. This means that SelectPanels, despite behaving more like popovers, prevent page scrolling too. We'd like to enable scrolling for SelectPanels but keep the existing behavior for other dialogs.

Integration

No changes necessary in production.

List the issues that this change affects.

Closes https://github.com/github/primer/issues/4126

Risk Assessment

• Low risk the change is small, highly observable, and easily rolled back.
• Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
• High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

What approach did you choose and why?

I refactored our CSS rules and added a parameter to Dialog that conditionally adds the .Overlay--disableScroll CSS class to the <dialog> element. The parameter defaults to true to preserve existing behavior.

Since we can use it now, I also switched to using body:has(...) instead of the hackish body.has-modal selector for applying the scroll gutter and scroll disabling rules.

Anything you want to highlight for special attention from reviewers?

I really tried to write a test for this, but wasn't able to figure out how to simulate scrolling appropriately. If scrolling is disabled in CSS via overflow: hidden, JavaScript's scroll, scrollTo, and scrollBy functions scroll the page anyway. Firing manually-created wheel and touch events also didn't work - the page never scrolls, even when overflow is not set to hidden. If y'all have any ideas here, I'm all ears.

Accessibility

• No new axe scan violation - This change does not introduce any new axe scan violations.

Merge checklist

- [ ] Added/updated tests

• Added/updated documentation
• Added/updated previews (Lookbook)
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

[changeset-bot]

🦋 Changeset detected

Latest commit: f2980ea

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/view-components	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[jonrohan]

:has is technically supported now, think you should try and test this on a browser that doesn't support :has just to see what a fail case might look like

[camertron]

@jonrohan just verified that the failure case simply allows the page to scroll, which I don't think is a blocker. There's also a scroll gutter property that won't apply, but that also doesn't seem to affect functionality.