Fix SidePanel and Modal issues on Safari

[hris27]

Purpose

• The opening animation of the SidePanel and Modal was broken on Safari and was fixed in react-modal@3.14.1.
• Due to focus management specifics in Safari (and Firefox on macOS) for mouse users after closing the SidePanel or Modal the focus is returned to the body (or the focus-root created by @reach/router in the case of the Dashboard) which may cause an unwanted page scroll.

Approach and changes

• Updated react-modal to the latest release - v3.14.4.
• Started using the preventScroll prop which would prevent the page from scrolling when the trigger element is not visible. This can happen if the user scrolls the main content while the side panel is visible. This should not cause any accessibility issues as we don't expect the main content to be scrolled when using keyboard navigation because of the focus trap. For mouse users it would actually improve the UX by not scrolling back to the trigger element after closing the side panel.

Definition of done

• Development completed
• Reviewers assigned
• Unit and integration tests
• Meets minimum browser support
• Meets accessibility requirements

[changeset-bot]

🦋 Changeset detected

Latest commit: 4701742

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

This PR includes changesets to release 1 package

Name	Type
@sumup/circuit-ui	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

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/sumup/oss-circuit-ui/9HQ9syp8v44wZm7vFUgQtpedkrU8
✅ Preview: https://oss-circuit-ui-git-bugfix-pmntss-1530-side-panel-safari-issues.sumup-vercel.app

[sumup-clark]

Hey @hris27,
we are super excited that you are contributing! 🎉There's one more thing you need to do. Please accept our Contributor License Agreement. It helps you and us to collaborate on clear terms and focus on what we love most: code.

Thanks!

[codecov]

Codecov Report

Merging #1498 (4701742) into main (f4cee56) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1498   +/-   ##
=======================================
  Coverage   92.33%   92.33%           
=======================================
  Files         201      201           
  Lines        4213     4213           
  Branches     1321     1321           
=======================================
  Hits         3890     3890           
  Misses        304      304           
  Partials       19       19           

Impacted Files	Coverage Δ
packages/circuit-ui/components/Modal/Modal.tsx	82.05% <ø> (ø)
...ages/circuit-ui/components/SidePanel/SidePanel.tsx	86.48% <ø> (ø)

[connor-baer]

Looks good to me, though I would love @robinmetral's opinion on the a11y-related changes once he's back from vacay.

[robinmetral]

Thanks a lot for addressing this and for the thorough context @hris27, that's a really interesting issue! reactjs/react-modal#389 lays it out really well and even links to the related docs on MDN 💯

I don't think preventing scroll will cause any accessibility problems. We could manually return focus to the trigger element in Safari and FF on macOS for consistency (as suggested in reactjs/react-modal#858 (comment)), but I'd rather keep our implementation leaner and let react-modal address this (it's been mentioned in the plans for v5 🤞)

[hris27]

Great, thanks @robinmetral. I'm going to merge this in main. Could you help me syncing it with canary?