refactor(ui): fold sidebar More section into a popup menu#105906
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54e5e08a81
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| private renderMoreMenuRoute(routeId: SidebarNavRoute) { | ||
| const active = this.isRouteActive(routeId); | ||
| return html` | ||
| <button |
There was a problem hiding this comment.
When an unpinned route lives in More, this button removes the href/shouldHandleNavigationClick path that top-level nav items still use, so Ctrl/Cmd-click and middle-click no longer open the route in a new tab; plugin tabs in renderMoreMenuPluginTab have the same regression. The sidebar CSS still documents nav entries as anchors for middle-click/new-tab, so these popup navigation items should render as links with role="menuitem" and only intercept plain clicks.
Useful? React with 👍 / 👎.
The expandable More section (uppercase header, chevron, persisted sidebarMoreExpanded pref, dedicated Edit pinned items row) collapses into a single More nav row that opens a transient menu with the unpinned routes, dynamic plugin tabs, and the pin editor entry. Sessions get the reclaimed vertical space.
More-menu route and plugin-tab rows render as real anchors again so Cmd/middle-click opens a new tab; ordinary clicks still route through onNavigate. Documents the popup More menu in docs/web/control-ui.md.
app-sidebar.ts grew past its ts-max-loc baseline with the More menu; extract the nav rows, More menu, and pin-editor menu into app-sidebar-nav-menus.ts (3116 -> 2949 lines) and ratchet the baseline entries for app-sidebar.ts, settings.ts, and app-host.ts down to their new sizes.
ba1b50d to
c7b08f5
Compare
…05906) The sidebar's expandable "MORE" section (uppercase header, chevron, persisted sidebarMoreExpanded pref, dedicated "Edit pinned items" row) collapses into a single More nav row that opens a transient popup menu with the unpinned routes, dynamic plugin tabs, and the pin editor entry. Menu rows are real anchors, so Cmd/middle-click keeps native link behavior; the More row carries the active highlight when the current route lives inside the menu. The sessions list gets the reclaimed vertical space, and one persisted setting is gone (stale keys are ignored on load). The nav rows and menus move to app-sidebar-nav-menus.ts to keep app-sidebar.ts under the TS LOC ratchet. Docs: docs/web/control-ui.md sidebar-navigation section updated. Fixes openclaw#105905
What Problem This Solves
The sidebar nav rendered a pinned set plus an expandable "MORE" section. The uppercase header + chevron cost a permanent row; expanded, the section (unpinned routes, plugin tabs, and a dedicated "Edit pinned items" row) pushed the sessions list — the sidebar's main content — half off-screen. The expansion state was a persisted pref (
sidebarMoreExpanded) that existed only to remember a chevron.Fixes #105905.
Why This Change Was Made
Collapse the whole section into a single trailing "More" nav row that opens a transient popup menu (the Claude.ai sidebar pattern):
aria-current="page".dismissTransientMenus).MOREheader/chevron CSS block, the dedicated edit row, and thesidebarMoreExpandedpref end-to-end (settings persistence, navigation snapshot, app-host wiring). Transient UI state gets no migration; a stale persisted key is simply ignored on load.Net prod diff is +18 LOC: the menu plus its lifecycle wiring replaces the section, one persisted setting, and ~90 lines of section CSS.
User Impact
sidebarMoreExpandedis dropped and ignored if present).Evidence
ui/src/app/settings.node.test.ts+ui/src/app-navigation-groups.test.ts— 37/37 pass.ui/src/e2e/sidebar-customization.e2e.test.ts, rewritten for the menu interaction): 5/6 pass locally, including the Workboard-in-More-menu test and both rewritten focus/keyboard tests. The remaining test ("pins routes, restores defaults…") fails locally at a pre-existing settings-search step (getByRole("link", { name: "Settings" })never visible) — verified to fail identically on unmodifiedmainin a fresh clone, so it is not introduced by this diff; flagged separately for triage.oxfmt --checkand scoped oxlint clean on changed files.app-sidebar.tsis split under the new TS LOC ratchet: the nav rows and menus moved toapp-sidebar-nav-menus.ts(3116 → 2949 lines) with baseline entries ratcheted down.check-guards/check-dependenciesfail on pre-existing main-side ratchet drift (~14 files grew, 10 unused exports landed while main CI runs were being cancelled) — unrelated to this diff; this PR's own files clear both guards. Landed with maintainer override; drift cleanup tracked separately.