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

fix(ComposedModal): wrap modal content in div to get VO in iOS working #11350

Conversation

annawen1
Copy link
Member

@annawen1 annawen1 commented May 5, 2022

Closes #11046

VO on iOS - focus was not getting trapped in the ComposedModal. When user swipes through modal content, expected behavior is the focus will loop back to start of modal instead of going out into the browser tool bar.

BEFORE:

RPReplay_Final1652120455.mov

AFTER: (I'm swiping back and forth to ensure both start and end sentinels redirect focus back within the modal)

RPReplay_Final1652120296.mov

Changelog

Changed

  • wrap modal content in another div - weird, but doing so gets VO to focus on the start/end sentinel buttons and redirect focus.
  • create a cds--modal-container-body class for styling

Testing / Reviewing

  • using VO, make sure focus in ComposedModal is trapped within modal until user closes the modal

@annawen1 annawen1 assigned annawen1 and unassigned annawen1 May 5, 2022
@annawen1 annawen1 requested a review from andy-blum May 5, 2022 14:26
@netlify
Copy link

netlify bot commented May 5, 2022

Deploy Preview for carbon-components-react ready!

Name Link
🔨 Latest commit dbc20df
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/6282722b58257b0009a5e47c
😎 Deploy Preview https://deploy-preview-11350--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented May 5, 2022

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit dbc20df
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/6282722b53eac50009c84dd7
😎 Deploy Preview https://deploy-preview-11350--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@joshblack
Copy link
Contributor

@annawen1 one thing that may be happening with VO on iOS is that ComposedModal needs to set inert and/or aria-hidden on content outside of the modal dialog, if I remember right. This is something we ran into when trying to build up Dialog for v11 where we set aria-hidden and inert on content that was not the modal dialog.

This could also totally be wrong haha, figured I'd share in case it helped out at all!

@annawen1
Copy link
Member Author

annawen1 commented May 9, 2022

@annawen1 one thing that may be happening with VO on iOS is that ComposedModal needs to set inert and/or aria-hidden on content outside of the modal dialog, if I remember right. This is something we ran into when trying to build up Dialog for v11 where we set aria-hidden and inert on content that was not the modal dialog.

This could also totally be wrong haha, figured I'd share in case it helped out at all!

@joshblack yeah that was the #1 solution that I've seen when searching online, but we do have similar focus wrap logic for our web components locale modal that's working on VO iOS so I'm trying to see why it doesn't do the same 🤔

UPDATE: weird, but it works when I use another div to wrap the start/end sentinel buttons within. Really not sure why VO decides to finally focus on the sentinel buttons with that extra div wrapping.

@annawen1 annawen1 changed the title fix(composed-modal): console logs for focus wrapping fix(ComposedModal): wrap modal content in div to get VO in iOS working May 9, 2022
@annawen1 annawen1 marked this pull request as ready for review May 9, 2022 18:25
@annawen1 annawen1 requested a review from a team as a code owner May 9, 2022 18:25
@annawen1 annawen1 requested review from joshblack and abbeyhrt May 9, 2022 18:25
@kodiakhq kodiakhq bot merged commit 9685228 into carbon-design-system:main May 16, 2022
@annawen1 annawen1 deleted the fix/vo-focus-wrapping-composed-modal branch May 16, 2022 17:11
kennylam pushed a commit to kennylam/carbon that referenced this pull request Jul 30, 2024
### Related Ticket(s)
carbon-design-system#11350 

### Description
Our current docs lack some of the events our components dispatch, this includes them in the JSDoc description using `@fires`.

### Changelog

**New**

- added missing events from component docs in CWC

<!-- React and Web Component deploy previews are enabled by default. -->
<!-- To enable additional available deploy previews, apply the following -->
<!-- labels for the corresponding package: -->
<!-- *** "test: e2e": Codesandbox examples and e2e integration tests -->
<!-- *** "package: services": Services -->
<!-- *** "package: utilities": Utilities -->
<!-- *** "RTL": React / Web Components (RTL) -->
<!-- *** "feature flag": React / Web Components (experimental) -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[a11y]: ComposedModal - VO on iPhone, focus isn't trapped when modal is open
4 participants