Skip to content

chore: Use inert in ariaHideOutside #8372

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

Merged
merged 7 commits into from
Jul 11, 2025

Conversation

devongovett
Copy link
Member

Reverts #8371

@LFDanLu
Copy link
Member

LFDanLu commented Jun 10, 2025

One bug found was that hovering a S2 SubmenuTrigger causes a focus ring to appear around the submenu that appears

@rspbot
Copy link

rspbot commented Jun 10, 2025

Build successful! 🎉

snowystinger
snowystinger previously approved these changes Jun 10, 2025
Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i know we found some things, but happy to get back in to work on more

@filipw01
Copy link
Contributor

@devongovett While you're deeper in the topic of ariaHideOutside, could you take a look at this issue? #8268 (comment)

@rspbot
Copy link

rspbot commented Jun 16, 2025

Build successful! 🎉

@devongovett
Copy link
Member Author

Fixed issues from testing:

  • focus ring no longer appears around submenus in S2
  • VoiceOver iOS cursor moves into dialogs properly

Investigated Firefox issue with focus not moving into Picker on mouse down. Turns out it is due to this 25 year old bug: https://bugzilla.mozilla.org/show_bug.cgi?id=53579 where calling element.focus() during a blur handler does not work. We do that in our preventFocus utility. However, looks like they finally fixed that bug recently, and it works in Firefox Developer Edition (141 beta). Given that, I decided not to work around the issue for now (which would involve some gross timeouts).

@rspbot
Copy link

rspbot commented Jul 11, 2025

@rspbot
Copy link

rspbot commented Jul 11, 2025

## API Changes

@react-aria/overlays

/@react-aria/overlays:ariaHideOutside

 ariaHideOutside {
   targets: Array<Element>
-  root: Element
+  options?: AriaHideOutsideOptions | Element
   returnVal: undefined
 }

@react-spectrum/overlays

/@react-spectrum/overlays:Overlay

 Overlay {
   children: ReactNode
   container?: Element
   disableFocusManagement?: boolean
   isKeyboardDismissDisabled?: boolean
   isOpen?: boolean
   nodeRef: MutableRefObject<HTMLElement | null>
   onEnter?: () => void
   onEntered?: () => void
   onEntering?: () => void
   onExit?: () => void
   onExited?: () => void
   onExiting?: () => void
+  shouldContainFocus?: boolean
 }

Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verified the S2 submenu hover fix and the VO iOS dialog fix locally, the rest of the behavior seems to work well in my quick sweep. Approving for testing

@devongovett devongovett added this pull request to the merge queue Jul 11, 2025
Merged via the queue into main with commit 7639e56 Jul 11, 2025
31 checks passed
@devongovett devongovett deleted the revert-8371-revert-8317-use-inert branch July 11, 2025 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants