Skip to content

Conversation

@atomiks
Copy link
Contributor

@atomiks atomiks commented Jan 1, 2026

Fixes a couple of issues with the focus guards:

  • Tabbing out of a Popover gets stuck on the focus guard in Safari (or moving virtual cursor out of a Popover on iOS)
  • Tabbing out of a Menu inside a Popover incorrectly restores focus to the popover container instead of the next tabbable
  • Tabbing out of a Combobox input whose popover is open should close the popup when focus lands on the Popover trigger (when it's the first tabbable element or the last one, when tabbing forward)

https://deploy-preview-3654--base-ui.netlify.app/experiments/popup-tabbing

@atomiks atomiks added component: menu Changes related to the menu component. type: bug It doesn't behave as expected. component: popover Changes related to the popover component. labels Jan 1, 2026
@pkg-pr-new
Copy link

pkg-pr-new bot commented Jan 1, 2026

  • vite-css-base-ui-example

    pnpm add https://pkg.pr.new/mui/base-ui/@base-ui/react@3654
    
    pnpm add https://pkg.pr.new/mui/base-ui/@base-ui/utils@3654
    

commit: 5b5d512

@mui-bot
Copy link

mui-bot commented Jan 1, 2026

Bundle size report

Bundle Parsed size Gzip size
@base-ui/react 🔺+217B(+0.05%) 🔺+33B(+0.03%)

Details of bundle changes


Check out the code infra dashboard for more information about this PR.

@netlify
Copy link

netlify bot commented Jan 1, 2026

Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit 5b5d512
🔍 Latest deploy log https://app.netlify.com/projects/base-ui/deploys/695654c1dcd1650008052fd1
😎 Deploy Preview https://deploy-preview-3654--base-ui.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 project configuration.

@atomiks atomiks force-pushed the fix/focus-guard-handling branch from 79b8d8c to 5ccff51 Compare January 1, 2026 09:03
Comment on lines +295 to +297
let nextTabbable = getTabbableAfterElement(
store.context.triggerFocusTargetRef.current || triggerElementRef.current,
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In Safari, aria-hidden={false} on the focus guard, so it should look for next tabbable starting from (after) the focus guard


const focus = useFocus(floatingRootContext, {
enabled: !disabled && ((!isInMenubar && isOpenedByThisTrigger) || parentMenubarHasSubmenuOpen),
enabled: !disabled && parentMenubarHasSubmenuOpen,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This otherwise causes Shift + Tab not to close the popup when focus lands back on the trigger in Menu; seems to still pass all Menubar tests

@atomiks atomiks marked this pull request as ready for review January 1, 2026 09:07
@atomiks atomiks force-pushed the fix/focus-guard-handling branch from fef44c4 to 5b5d512 Compare January 1, 2026 11:04
@atomiks atomiks changed the title [menu][popover] Fix focus guard handling [menu][popover][combobox] Fix focus guard handling Jan 2, 2026
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

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

Looks great. 👍
Very nice improvement when running on Safari.
However, have you checked Firefox? It is broken with tabbing to an after (Tab) element, only works with before (Shift+Tab). 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: menu Changes related to the menu component. component: popover Changes related to the popover component. type: bug It doesn't behave as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants