Summary

This PR improves the main menu's accessibility by moving the version footer (and its CopyButton) outside role="menu", updates keyboard handling for the footer, and aligns unit/e2e tests and snapshots with the new structure. The overall direction is good and addresses a real ARIA issue.

Code Quality

The refactor in frontend/app/src/components/MainMenu/MainMenu.tsx and frontend/app/src/components/MainMenu/styled-components.ts is clean and readable, and the semantic separation of menu items vs footer is a strong improvement.

One behavioral issue remains:

• Reverse-tab regression / focus trap risk in frontend/app/src/components/MainMenu/MainMenu.tsx:706-716 and frontend/app/src/components/MainMenu/MainMenu.tsx:723-733:
  • When streamlitVersion is present, Tab handling in the menu always falls through (break), including Shift+Tab.
  • Shift+Tab on the footer intentionally returns focus to the menu, but Shift+Tab from the menu does not close the popover anymore, so reverse keyboard traversal can cycle inside the popover instead of moving to the previous page control.
  • This is a behavior change from prior menu behavior and from the no-footer path, which still closes on Shift+Tab.

Test Coverage

Coverage is generally good:

• Frontend unit tests add checks for:
  • Footer being outside role="menu".
  • Forward Tab close behavior from footer.
  • Shift+Tab and Escape behavior from the footer.
• E2E updates correctly retarget selectors from role="menu" to the popover container where needed.

Gap identified:

• No test currently validates reverse escape from the popover when footer exists (e.g. Shift+Tab from menu should close and move backward, if that is intended), so the regression above is not caught.

Backwards Compatibility

No API-level breaking changes were found. However, keyboard behavior changed for users navigating backwards with Shift+Tab when the version footer is present, which is a backward-compatibility concern for keyboard interaction patterns.

Security & Risk

No direct security concerns identified.
Primary risk is interaction regression in keyboard navigation/focus management.

Accessibility

Positive:

• Moving CopyButton outside role="menu" resolves an ARIA semantic violation and is a clear accessibility win.

Concern:

• Current Shift+Tab behavior with the footer can trap reverse traversal inside the popover flow, which is an accessibility/usability regression risk for keyboard users.

Recommendations

1. Update frontend/app/src/components/MainMenu/MainMenu.tsx keyboard logic so that with footer enabled:
   • Tab from menu continues to move to the footer copy button.
   • Shift+Tab from menu closes with "shift-tab" and returns focus backward (consistent with existing no-footer behavior).
2. Add a targeted test in frontend/app/src/components/MainMenu/MainMenu.test.tsx for reverse-tab exit behavior when footer exists (and optionally an e2e assertion if desired).

Verdict

CHANGES REQUESTED: The accessibility semantics fix is strong, but reverse keyboard navigation has a regression risk that should be resolved before merge.

This is an automated AI review by gpt-5.3-codex-high. Please verify the feedback and use your judgment.

Summary

This PR improves accessibility and layout of the version copy button in the MainMenu footer by:

1. Moving the version footer outside role="menu": The StyledMenuVersionFooter (with its CopyButton) is now a sibling of StyledMenuContainer rather than a child. A new StyledMenuPopoverContent wrapper owns the outer padding and minWidth for both the menu and footer. This fixes a WAI-ARIA violation where an interactive <button> was nested inside role="menu" without being a menuitem.
2. Adding keyboard navigation for the footer CopyButton: Tab from the last menu item moves focus to the CopyButton (via focus-lock), then a second Tab closes the popover. Escape from the CopyButton also closes the popover. Shift+Tab returns focus to the menu.
3. Removing hover jitter on copy button: The transform: scale(0.9) → scale(1) transition is replaced with a clean opacity-only transition.

Files changed: 2 frontend source files, 1 frontend test file, 2 e2e test files.

Code Quality

The code is well-structured, clean, and follows established patterns in the codebase:

• StyledMenuPopoverContent (styled-components.ts:51-64): The new wrapper component is well-documented with a JSDoc comment explaining its purpose and how padding is distributed between it and the footer. The padding and minWidth properties were correctly migrated from the old StyledMenuContainer.

• handleFooterKeyDown (MainMenu.tsx:723-743): The new keyboard handler is clean and follows the same pattern as handleKeyDown. It correctly handles forward Tab (close), Shift+Tab (let focus-lock cycle back), and Escape (close and return focus to trigger).

• Tab behavior branching in handleKeyDown (MainMenu.tsx:707-716): The conditional if (streamlitVersion) check to decide whether to let focus-lock handle Tab or close immediately is a pragmatic solution. One thing to note: this condition also applies to Shift+Tab from menu items when the footer is present — Shift+Tab from the first menu item will cycle focus backward to the CopyButton via focus-lock instead of closing the menu. This is a deliberate tradeoff (users can always use Escape), but adding a brief inline comment noting that both Tab and Shift+Tab are affected would improve readability.

• Spacing consistency: The marginTop: theme.spacing.xs added to StyledMenuVersionRow correctly matches the flex gap: theme.spacing.xs that previously existed on StyledMenuContainer, maintaining visual consistency. The footer's paddingLeft/Right: theme.spacing.sm aligns with menu item padding structure (StyledMenuItemRow also uses theme.spacing.sm horizontal padding).

Test Coverage

Frontend unit tests — Good coverage of the new keyboard navigation scenarios:

• renders the CopyButton outside role=menu — directly validates the ARIA fix.
• does not close the menu on Tab from a menu item when footer exists — validates the two-step Tab behavior.
• closes the menu on forward Tab from the CopyButton — validates final Tab closes the popover.
• does not close the menu on Shift+Tab from the CopyButton — validates backward Tab returns to menu.
• closes the menu on Escape from the CopyButton — validates Escape closes from footer.

Tests correctly use positive + negative assertions (e.g., toBeInTheDocument vs not.toBeInTheDocument), consistent with the testing guidelines.

Minor gap: There is no unit test for Shift+Tab from a menu item when the footer is present (i.e., verifying the menu stays open and focus moves to the CopyButton). The existing Shift+Tab close test at line 304 only covers the no-footer case (getProps() without streamlitVersion). Adding one test case would complete the coverage matrix.

E2E tests — Updated appropriately:

• test_tab_closes_menu now correctly tests the two-step Tab sequence (Tab to CopyButton, Tab to close).
• test_main_menu_version_footer_visible and test_main_menu_version_footer_copies_version updated to use popover locator instead of menu locator, reflecting the new DOM structure.
• Snapshot tests in main_menu_test.py and custom_theme_fonts_test.py updated to find version text via the popover wrapper instead of role="menu".

Backwards Compatibility

This change is backwards compatible for end users:

• The visual appearance is preserved (padding, spacing, hover behavior all match).
• The only behavioral change is that Tab from the menu now requires two presses to close (one to reach the CopyButton, one to close). This is a minor UX change but improves accessibility. Escape still closes the menu in one press.
• No API changes, no protobuf changes, no Python changes beyond E2E test updates.

Security & Risk

No security concerns identified. The changes are purely in the frontend presentation layer and keyboard navigation logic. No new data flows, network calls, or state management changes.

Regression risk is low: The changes are well-scoped to the MainMenu component and its styled components. The conditional branching on streamlitVersion ensures the no-footer case continues to work exactly as before.

Accessibility

This PR is a net positive for accessibility:

• Fixes a WAI-ARIA violation: The CopyButton is no longer an invalid child of role="menu". It now lives outside the menu container as a sibling in the popover, which is correct per the ARIA menu pattern (only menuitem, menuitemradio, menuitemcheckbox, and group/separator are valid children of role="menu").
• Keyboard navigation is well-designed: Tab→CopyButton→Tab to close, Escape to close from anywhere, Shift+Tab to go back — all follow logical focus flow within the popover's focus-lock.
• Focus-lock integration: The focusLock prop on StatefulPopover correctly keeps focus within the popover boundary, and the custom returnFocus callback properly handles focus restoration for all close reasons.
• Minor note: Shift+Tab from menu items when the footer exists will cycle to the CopyButton rather than closing the menu. Per strict WAI-ARIA menu-button pattern, Shift+Tab should close the menu. However, since the CopyButton is an intentional interactive element in the popover, this tradeoff is reasonable — users can always dismiss with Escape.

Recommendations

1. Consider adding a unit test for Shift+Tab from menu items when the footer is present. This would verify the menu stays open and focus moves to the CopyButton. Currently, Shift+Tab is only tested from the CopyButton (line 1617) and from the no-footer case (line 304).

2. Consider adding a brief comment in the handleKeyDown Tab case (line 707) noting that the streamlitVersion check intentionally affects both Tab and Shift+Tab:

   case "Tab": {
     if (streamlitVersion) {
       // A CopyButton exists in the version footer outside role="menu"
       // but inside the popover's focus-lock.  Let focus-lock move
       // focus there (forward Tab) or cycle back (Shift+Tab) instead
       // of closing the menu immediately.
       break
     }

Both suggestions are minor improvements and are not blocking.

Verdict

APPROVED: A well-implemented accessibility improvement that fixes a real WAI-ARIA violation, adds clean keyboard navigation for the footer CopyButton, and includes comprehensive test coverage. The code is clean, follows existing patterns, and introduces no regressions.

This is an automated AI review by opus-4.6-thinking.