✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

✅ PR preview is ready!

Summary

This PR refactors the MainMenu component by replacing BaseWeb's StatefulMenu with a custom data-driven implementation. Key changes include:

• Removing StatefulMenu dependency while keeping StatefulPopover for positioning/focus lock
• Introducing a section-based data model (MenuSection, MenuItemConfig)
• Creating pure functions for menu construction (buildMenuData, buildPrimaryItems, buildDevItems, etc.)
• Simplifying styled components by removing unused BaseWeb-specific overrides
• Updating all unit tests and e2e tests to match the new implementation

This is a foundational PR for a MainMenu redesign, with accessibility improvements noted for a follow-up PR.

Code Quality

The refactoring follows React best practices well:

Strengths:

• Pure functions for menu construction make the logic easier to test and reason about
• Data-driven approach with MenuSection[] cleanly separates data from rendering
• Good use of useMemo for menu data construction (lines 470-502 in MainMenu.tsx)
• Proper TypeScript interfaces (MenuItemConfig, MenuSection) provide type safety
• Well-documented code with JSDoc comments explaining the menu structure and precedence rules
• Semantic HTML with <button> elements for menu items improves over the previous implementation

Minor observations:

• MainMenu.tsx:509 - Returning <></> for empty case works, but return null is more conventional for conditional rendering
• The LOG constant at line 47 could use the module pattern getLogger("MainMenu") which it does - good practice

The styled components cleanup removes ~120 lines of unused code, which improves maintainability.

Test Coverage

Unit Tests (MainMenu.test.tsx):

• Comprehensive coverage with 30+ test cases covering:
  • Menu item visibility and hiding based on configuration
  • Host menu items integration and conflict resolution
  • Developer mode features (Clear cache)
  • Disabled states when server disconnected
  • Keyboard shortcuts display
  • Menu item ordering
  • Recording state indicators
  • Metrics tracking
• Good use of negative assertions (e.g., verifying items are NOT rendered when hidden)
• Proper async handling with vi.useFakeTimers() for BaseWeb popover

E2E Tests:

• Updated main_menu_test.py with new escape key close test (lines 30-39)
• Updated st_set_page_config_test.py to use more robust text-based locators instead of index-based (lines 271-275, 287-309)
• The e2e tests follow best practices:
  • Uses get_by_test_id and get_by_text as recommended
  • Uses expect assertions with auto-wait
  • Avoids index-based selectors

Gap: No unit test for the openInNewTab error handling when popup blocker prevents window.open (line 60-64). Consider adding a test that mocks window.open returning null.

Backwards Compatibility

User-visible change:

• The label "Record a screencast" was changed to "Record screen" (line 247 in MainMenu.tsx)
• This is reflected in e2e test updates (main_menu_test.py lines 118, 129)

This is a deliberate design change and is backwards compatible in terms of functionality - no API changes or breaking changes to st.set_page_config() menu items.

The menu item ordering remains consistent with the existing behavior, and all configurable menu items (getHelpUrl, reportABugUrl, aboutSectionMd, hideGetHelp, hideReportABug) continue to work as expected.

Security & Risk

No security concerns identified:

• URL handling in openInNewTab uses window.open(url, "_blank") which is standard and safe
• Host menu items are properly sanitized through the existing sendMessageToHost callback pattern
• No new external dependencies introduced

Low risk items:

• The refactoring is significant but well-tested
• BaseWeb's StatefulPopover continues to provide focus lock and escape key handling

Accessibility

Current state:

• Uses semantic <button> elements for menu items (good improvement)
• Menu container has aria-label="Main menu" (line 440)
• Dividers have role="separator" and aria-hidden="true" (lines 418-419)
• Disabled state properly communicated via HTML disabled attribute

Concerns requiring follow-up (as noted in PR description):

1. styled-components.ts:98-100 - The &:focus-visible style removes outline without providing an alternative focus indicator. The TODO comment acknowledges this, but it should be addressed promptly:

    "&:hover, &:focus-visible": {
      backgroundColor: theme.colors.darkenedBgMix15,
    },

    // TODO(accessibility): Add visible focus ring for keyboard navigation.
    "&:focus-visible": {
      outline: "none",
    },

2. Missing ARIA menu pattern attributes (as noted in test comments, lines 619-620):
   • aria-haspopup, aria-expanded on trigger
   • role="menu", role="menuitem" on menu/items

These are acknowledged for a follow-up PR but should be tracked.

Recommendations

1. Add focus ring styling - The TODO at line 98-99 in styled-components.ts should be addressed before or shortly after merge. Consider using theme.shadows.focusRing or similar for consistent focus indication.

2. Consider adding unit test for popup blocker handling - The openInNewTab function logs a warning when popup is blocked (line 61-64), but this isn't tested. A simple test mocking window.open returning null would improve coverage.

3. Track accessibility improvements - Ensure the mentioned ARIA attributes (aria-haspopup, aria-expanded, role="menu", role="menuitem") are addressed in the follow-up PR as planned.

4. Minor cleanup - Line 509 could use return null instead of return <></> for slightly more idiomatic React code.

Verdict

APPROVED: This is a well-executed refactoring that improves code maintainability through a data-driven approach while maintaining backwards compatibility. The test coverage is comprehensive, and the acknowledged accessibility gaps are planned for a follow-up PR. The code follows React best practices and the Streamlit codebase conventions.

This is an automated AI review using opus-4.5-thinking. Please verify the feedback and use your judgment.

📉 Python coverage change detected

The Python unit test coverage has decreased by 0.0024%

• Current PR: 93.1664% (23443 statements, 1602 missed)
• Latest develop: 93.1688% (23466 statements, 1603 missed)

✅ Coverage change is within normal range.

📊 View detailed coverage comparison

📉 Frontend coverage change detected

The frontend unit test (vitest) coverage has decreased by 0.1500%

• Current PR: 86.5400% (13853 lines, 1864 missed)
• Latest develop: 86.6900% (13909 lines, 1850 missed)

💡 Consider adding more unit tests to maintain or improve coverage.

📊 View detailed coverage comparison

Summary

This PR refactors the main menu into a data-driven, sectioned structure with new styled button rows, updates the "Record screen" label, adds a menu width theme primitive, and refreshes e2e snapshots. It also updates frontend and e2e tests, including a new Escape-to-close test.

Code Quality

Overall structure is cleaner: menu data generation is separated from rendering, and the menu layout is easier to reason about and extend. The new helper utilities and tests read well and align with existing patterns.

Test Coverage

Frontend unit tests were updated for the refactored menu (including minimal mode and host items). E2E coverage includes updated snapshots and a new Escape-to-close test for the menu. No Python unit tests were required for these changes.

Backwards Compatibility

No public API or protobuf changes detected. This is primarily a UI refactor with updated menu labels and layout, which should be backwards compatible for users.

Security & Risk

No new security-sensitive flows identified in this change set.

Accessibility

The menu item buttons remove visible focus outlines without replacing them, which breaks keyboard focus visibility and violates our a11y guidelines. StyledMenuItemRow currently sets outline: none for :focus-visible without adding a replacement ring. See frontend/app/src/components/MainMenu/styled-components.ts:94-101.

Recommendations

1. Add a visible focus ring for StyledMenuItemRow when :focus-visible (use theme.shadows or similar) and remove/replace the outline: none rule. File: frontend/app/src/components/MainMenu/styled-components.ts:94-101.
2. If host-provided separators are intended to create grouping, consider mapping host separator items into sections instead of dropping them. File: frontend/app/src/components/MainMenu/MainMenu.tsx:301-306.

Verdict

CHANGES_REQUESTED: Add a visible focus style for menu item buttons to meet accessibility requirements.

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