Fixing modal document padding on mobile

[jdrush89]

Authors: Please fill out this form carefully and completely.

Reviewers: By approving this Pull Request you are approving the code change, as well as its deployment and mitigation plans.
Please read this description carefully. If you feel there is anything unclear or missing, please ask for updates.

What are you trying to accomplish?

Fixing an issue where modals could render offscreen due to adding too much document padding on mobile screens. Not 100% sure why showing the backdrop on dotcom causes window.innerWidth to increase far beyond document.body.clientWidth on mobile screens. It makes more sense to get the page's scrollbar width before showing the backdrop though and it fixes the issue in this case.

Screenshots

Before

After

Integration

List the issues that this change affects.

Closes #2150

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?

Show the backdrop after the scrollbar width calculation since it seems to throw it off.

Anything you want to highlight for special attention from reviewers?

Accessibility

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

Merge checklist

• Added/updated tests - Not sure how to test this in capybara. I don't see the body style getting modified in the dialog tests.
• Added/updated documentation - N/A
• Added/updated previews (Lookbook) - There's an existing preview for this.
• 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: cb222f2

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	Patch

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

[camertron]

Hey @jdrush89, thanks for this 👍 Can you verify it doesn't break other dialogs in dotcom?

Also, please add a changeset by running npx changeset 🙇

[jdrush89]

@camertron checked in the changeset. What's the best way to verify? I can put a breakpoint in that file and change the order of these lines and it seems to be fine. I was trying to see if I could build a new version of the gem and install that in my dotcom codespace but it doesn't seem to be working.

[camertron]

Hey @jdrush89, good question! I would just find a few other examples of modal dialogs in dotcom and make sure they work/look as expected. You can make the change by modifying the copies of the files in node_modules (which is I think the approach you've already tried?), or go partway though the release process we have documented here.

[jdrush89]

@camertron I modified the local files and verified that dialogs still behave correctly. I don't see the publish job running on my PR, could it be like primer/react where it doesn't publish a package on PRs from forks? Do you know why the triage checks are failing, could that be another fork issue?

[camertron]

Ahh shoot you're right, some of these CI jobs can't run from forks. Since all the actual tests are passing, I'm going to go ahead and merge this with my admin privileges.