Conversation
Core refactor of the `MainMenu` component that replaces BaseWeb's `StatefulMenu` with a custom data-driven implementation. This is the foundational PR for the `MainMenu` redesign, going into the feature branch for incremental review. - Remove `StatefulMenu` from BaseWeb, keep `StatefulPopover` for positioning/focus lock - Introduce section-based data model (`MenuSection`, `MenuItemConfig`) - Pure functions for menu construction (`buildMenuData`,`buildMenuSections`, `buildCommonItems`, etc.) - Memoized `MenuContent` component with stable callbacks
Adds full WAI-ARIA menu pattern support to the `MainMenu`, including keyboard navigation, proper ARIA semantics, and focus management. **Keyboard navigation** - **Open menu**: `Enter` / `Space` on the menu button (handles legacy `Spacebar` / `Space` key values) - **Navigate items**: `ArrowDown` / `ArrowUp` with wrapping at boundaries - **Jump to ends**: `Home` / `End` - **Activate item**: `Enter` / `Space` on focused menu item - **Close menu**: `Escape` returns focus to the trigger; `Tab` / `Shift+Tab` close the menu and advance focus to the next/previous tabbable element - **Disabled items**: All items are focusable and navigable, including disabled ones (per [WAI-ARIA menuitem role guidance](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/menuitem_role)). Disabled items use `aria-disabled` instead of the native `disabled` attribute; clicks are blocked via the `onClick` handler. **ARIA attributes** - Menu button: `aria-haspopup="menu"`, `aria-expanded` (tracks open/close state), `aria-label="Main menu"` - Menu container: `role="menu"`, `aria-label="Main menu"` - Menu items: `role="menuitem"`, `aria-disabled` for disabled items (native `disabled` removed to keep items focusable) - Dividers: `role="separator"`, `aria-hidden="true"` - Roving tabindex: focused item gets `tabIndex={0}`, all others get `tabIndex={-1}` **Focus management** - Focus moves to the first menu item when the menu opens - Focus returns to the menu button after `Escape` or item click, via react-focus-lock's `returnFocus` callback - `Tab` / `Shift+Tab` close the menu and programmatically advance focus to the next/previous tabbable element using `focusNextElement`/`focusPrevElement` from `focus-lock` - **WebKit limitation**: On Safari, focus restoration after menu close may not work because react-focus-lock invokes the `returnFocus` callback after BaseWeb's close animation timer, which puts the `.focus()` call outside the user-activation context **Clear cache dialog (WAI-ARIA best practice)** - Moves `autoFocus` from the destructive "Clear caches" button to the non-destructive "Cancel" button, per WAI-ARIA guidance for confirmation dialogs - Removes the now-unnecessary `defaultAction` prop from `ClearCacheProps` **Theme fix** - `focusRingMuted` shadow now uses `gray10` on dark backgrounds (was using `gray90`, which was invisible)
Adds a theme toggle (System / Light / Dark) directly into the main menu as `menuitemradio` elements, building on the accessibility refactor in #13878. **Changes:** - Three radio items (System, Light, Dark) appear at the top of the menu, inside a `role="group"` container with `aria-label="Theme"`. - Each item uses `role="menuitemradio"` with `aria-checked` to reflect the active theme. - Selecting a theme calls `setTheme` via `ThemeContext` without closing the menu, so users can preview different themes. - Keyboard navigation (ArrowUp/Down, Home/End, Enter/Space) flows seamlessly between radio items and action items in a single roving tabindex.
- Adds an **Auto-rerun toggle** (`menuitemcheckbox`) to the `MainMenu` developer section, surfacing the run-on-save setting directly in the menu. Wired to the same `saveSettings` path as the Settings dialog checkbox. - Extends the `MenuItem` discriminated union with a `MenuToggleItem` type and `ToggleItemRow` component, with full WAI-ARIA keyboard support (Enter/Space activation, roving tabindex). - Gates Settings, Rerun, Auto-rerun, and Clear cache behind `developmentMode` to hide them from app viewers.
- Adds a "Made with Streamlit v{version}" footer inside the main menu, with a hover-to-reveal copy button that copies the full version string to the clipboard.
- Merges the About menu item into the standard items section (removing its separate divider).
- Adds the missing `ESC` keyboard shortcut hint on the Record screen item during active recording (parity with `develop`).
- Nightly `.dev` versions are truncated for display (e.g., `v1.54.1.dev`) but the full version (e.g., `1.54.1.dev20260217`) is copied.
- Removes the **Settings** menu item from the main menu and deletes the `SettingsDialog` component entirely. All Settings dialog functionality has been absorbed by the new main menu:
- **Theme switching** → theme radio buttons (System / Light / Dark) in the menu
- **Auto-rerun (Run on save)** → toggle in the menu
- **Version footer** → footer in the menu
- **Wide mode** → removed from UI (set via `st.set_page_config(layout="wide")`)
- Migrates **5 theming E2E test files** from the Settings dialog selectbox flow to the main menu's `menuitemradio` buttons, using accessible selectors (`get_by_role("menuitemradio", name="Dark")`).
- Deletes `test_renders_settings_dialog_properly` and `test_settings_dialog_copies_version` from `main_menu_test.py` (coverage is now provided by the menu footer tests).
- Removes `SETTINGS` from the `DialogType` enum and all associated plumbing (`settingsCallback` in `App.tsx`, `Props`, `buildDevItems`, `StreamlitDialog` switch case).
- Cleans up orphaned styled components and updates stale `@see SettingsDialog` JSDoc references in `ThemeContext.tsx` and `types.ts`.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
✅ PR preview is ready!
|
📉 Significant bundle size change detected
Please verify that this change is expected. |
📈 Frontend coverage change detectedThe frontend unit test (vitest) coverage has increased by 0.2000%
🎉 Great job on improving test coverage! |
There was a problem hiding this comment.
Pull request overview
This PR consolidates six incremental MainMenu redesign PRs into develop, completing the migration of Settings dialog functionality into the main menu and removing the Settings dialog entirely. The refactor introduces a data-driven menu architecture with full WAI-ARIA support, theme switching radio buttons, auto-rerun toggle, and a version footer—all directly accessible from the main menu.
Changes:
- Backend: Removed Settings dialog plumbing (
settingsCallback,DialogType.SETTINGS), addedhandleRunOnSaveChangehandler, updatedclient.toolbarModeconfig description - Frontend Library: Enhanced
BaseButtonwith ARIA popup attributes, fixedfocusRingMutedshadow for dark backgrounds, updated theme-related JSDoc references - MainMenu Component: Complete refactor with theme radio group, toggle component, version footer with copy button, keyboard navigation, and accessible menu structure
- E2E Tests: Migrated 5 theming test files from Settings dialog selectbox to MainMenu
menuitemradiobuttons, updated snapshots, added comprehensive keyboard navigation tests
Reviewed changes
Copilot reviewed 32 out of 63 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
lib/streamlit/config.py |
Updated client.toolbarMode description to remove Settings dialog reference |
frontend/lib/src/theme/types.ts, getShadows.ts |
Updated JSDoc and fixed focusRingMuted shadow for dark themes (gray10 instead of gray90) |
frontend/lib/src/components/shared/BaseButton/* |
Added ARIA aria-haspopup and aria-expanded support with proper TypeScript typing |
frontend/app/src/components/MainMenu/* |
Complete refactor: added theme radio group, toggle component, version footer, keyboard navigation, and test utilities |
frontend/app/src/components/StreamlitDialog/* |
Removed entire SettingsDialog component and all styled-components; updated ClearCacheDialog to focus Cancel button (WAI-ARIA best practice) |
frontend/app/src/App.tsx |
Removed settingsCallback, added handleRunOnSaveChange, passed new props to MainMenu |
e2e_playwright/theming/*_test.py |
Migrated theme selection from Settings dialog to MainMenu radio buttons, using accessible get_by_role selectors |
e2e_playwright/main_menu_test.py |
Added keyboard navigation tests, version footer tests, removed Settings dialog tests, updated snapshot handling |
SummaryThis PR is a comprehensive refactor of the Streamlit
Code QualityThe refactor is well-structured and follows codebase conventions:
Minor observations (both reviewers agreed):
Test CoverageBoth reviewers agreed that test coverage is excellent:
Backwards CompatibilityThis is a user-facing breaking change in terms of UI surface (both reviewers aligned):
Security & RiskBoth reviewers agreed: No security concerns identified. No new external dependencies or network calls. URLs are opened via AccessibilityBoth reviewers agreed this PR is a significant accessibility improvement overall: proper WAI-ARIA menu button pattern, roving tabindex, full keyboard navigation, correct roles ( Point of disagreement — version footer CopyButton:
Consolidated assessment: The observation is technically correct — there is an ARIA semantics violation (a non-menuitem focusable control inside
Recommendations
VerdictAPPROVED — This is a well-executed, thoroughly tested refactor that significantly improves MainMenu accessibility, UX, and code quality. Both reviewers agreed on the overall high quality of the implementation. The CopyButton keyboard accessibility issue raised by gpt-5.3-codex-high is valid but non-blocking — it is a minor ARIA conformance gap on a secondary convenience feature that does not diminish the substantial accessibility gains of this PR, and can be addressed in a targeted follow-up. Consolidated review by 📋 Review by `gpt-5.3-codex-high`SummaryThis PR delivers a large Code QualityThe refactor is generally well-structured and significantly improves menu architecture (data-driven sections, clearer item types, stronger focus handling), but there is one blocking accessibility/interaction defect:
Test CoverageCoverage is strong overall:
Gap:
Backwards CompatibilityNo Python API/protocol break is visible from this diff, and Security & RiskNo new backend security issues were evident in the reviewed changes. AccessibilityAccessibility has improved in several areas (menu roles, focus management, keyboard nav), but the version copy control currently regresses keyboard accessibility:
Recommendations
VerdictCHANGES REQUESTED: The new menu contains a keyboard/a11y regression where the version copy action is not reachable/operable via keyboard within This is an automated AI review by 📋 Review by `opus-4.6-thinking`SummaryThis PR is a comprehensive refactor of the Streamlit
Code QualityThe code is well-structured and follows the codebase conventions:
Minor observations:
Test CoverageTest coverage is excellent:
The shared Backwards CompatibilityThis is a user-facing breaking change in terms of UI surface:
Since the PR is labeled Security & RiskNo security concerns identified:
Regression risks are well-mitigated by the comprehensive test suite. The main risk area is the WebKit focus-return limitation (documented in comments and addressed with AccessibilityThis PR is a significant accessibility improvement:
Recommendations
VerdictAPPROVED: This is a well-executed, thoroughly tested refactor that significantly improves the MainMenu's accessibility, UX, and code quality. The SettingsDialog removal and wide-mode toggle loss are intentional design decisions that are acknowledged via the PR labels. The comprehensive test coverage across unit and E2E tests provides strong confidence against regressions. This is an automated AI review by |
There was a problem hiding this comment.
nitpick: reset hover might help here to prevent the menu icon from being hovered (probably unintended)
Describe your changes
This PR is the culmination of the following PRs approved into this feature branch:
MainMenuRefactorMainMenuaccessibility improvementsMainMenuMainMenuMainMenuSettingsDialog