Repository navigation
feat(shell): Refined UI step 2: top nav and icon cluster, Minimal/Full mode, Appearance settings, one brand mark (#18240) - #18373
Merged
Merged
Conversation
Contributor
|
Important Review skippedToo many files! This PR contains 109 files, which is 9 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
⛔ Files ignored due to path filters (5)
📒 Files selected for processing (109)
You can disable this status message by setting the
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
3 tasks
Contributor
✅ SSOT Configuration Compliance: Passing🎉 No new hardcoded values of either class — Known backlog in |
3 tasks
2 tasks
Owner
Author
|
Delta review (Sonnet) at 5df8dea: approve. The deleted test asserted module-instance identity, which holds with a single |
This was referenced Oct 12, 2026
Merged
This was referenced Oct 12, 2026
Merged
Owner
Author
|
Ejected from merge train #7 (#18401). This PR causes most of that train's red checks. Each is listed with its CI evidence:
Also on this PR from train-7: After the fixes, the delta needs a re-review, then the PR joins the next train. |
…nces, and PATCH /users/me/preferences writes only the fields it was sent (#18240)
… announcer in @autobot/ui, consumed by both apps (#18240)
…llows the account (#18240) useUiMode owns data-ui on <html> (minimal|full) and the start preference (last|minimal|full), writes through to localStorage, and resolves the opening mode at login (ui_start === 'last' ? ui_mode : ui_start). theme-variant-init sets data-ui before mount, the same CSP-safe pattern as the variant beside it. html[data-ui="minimal"] .adv hides advanced chrome and .only-min shows only in Minimal, both without !important (the selector is more specific than a display utility). usePreferences now sends and applies ui_mode, ui_start and theme_variant with the appearance prefs. The endpoint is a partial PATCH since 0b2892a360, and the only caller still sends every field it owns, so nothing is reset. An account theme_variant of 'default' does not override a device that never chose one, so a deployment pinning another build-time default keeps it.
…les (#18240) nav.homeAria, vaultTip, preferencesTip, platformAdmin(+Aria/Tip), profileMenu, nav.uiMode.*, settings.preferredMode.*, settings.additionalThemes.*, ui.minimalNote and ui.minimalNoteAction, each really translated (ar, de, es, fa, fr, he, lv, pl, pt, ur) and appended to its group; no existing key is touched. settings.appearance.* from the plan is settings.additionalThemes.* because settings.appearance is already a string key. The SLM locales are regenerated by scripts/lift_locale_translations.py: its two "Full" keys now lift a translation from the new settings.preferredMode.full. nav.slmAdmin and nav.slmAdminTitle have no reference left in the app but stay, append-only: the same lift derives the SLM's "SLM Admin" translation from them.
…18240) UiModeSettingsCard (Settings > Appearance): two aria-pressed mode cards and a Start-in radiogroup (Last used / Minimal / Full) over the same useUiMode state the header toggle drives. ThemePresetPicker is extended, not duplicated, with an Additional themes group (AdditionalThemesGroup) fed by the installed-theme registry, each theme badged "Deployed via SLM" with a link to the SLM console; EmberThemeToggle keeps the built-in variants only so no theme is offered twice. ThemePresetPicker's inline sr-only announcer moves onto the kit useAnnounce, which takes it from 611 to 587 lines, under the limit, so its ceiling is deleted. SettingsView does not grow (784).
…er, UiModeToggle, ProfileMenu, MobileNavPanel and a BaseModal status panel (#18240) Split the 290-line header and the hand-rolled status overlay out of App.vue (1144 -> 499 lines, now under the 600 limit, so its grandfathered ceiling is deleted). The brand is the kit BrandMark linking Home; the health dot is an aria-hidden overlay and System status moved into the profile menu. Primary links are Home, Chat, Knowledge, Company OS and Analytics (advanced); Automation is pinned to the existing NavOverflowMenu. The right-hand cluster is Secrets (advanced), Preferences, the Minimal/Full toggle and the Platform admin shield (advanced, gated by service.management, opens the SLM console in a new tab). Every icon has an i18n aria-label and a kit BaseTooltip. The active link is a 2px accent underline. config/navItems.ts stays the one registry: it gains the adv and overflow flags and navIconItems; the profile menu loses Plugins (it lives in the SLM console nav; INTENTIONALLY_HIDDEN records why), Secrets and Preferences; paletteNavItems still derives from it. The mobile drawer renders the same registry. The SLM text link, DarkModeToggle and LanguageSwitcher are no longer in the nav. One shared useSystemStatus instance is provided by App.vue (composables/ systemStatusContext.ts) so the dot and the panel show one snapshot. New strings are committed separately (feat(i18n)). nav.slmAdmin* stays (the SLM locale lift derives from it). Ratchets only move down: App.vue's scoped style, a LoginForm .logo-mark rule that had pushed distinct_class_names over its baseline, and the raw status colours the header and panel used.
…ps it that way (#18240) useUserStore.applyTheme/updateTheme and the profile modal's own theme buttons set data-theme from a second preference (default light) that could disagree with useTheme's persisted choice. They are removed, together with PreferencesPanel in the profile modal (appearance settings live in Settings only). The meta theme-color the store set from two hex values moves into useTheme and follows the active theme's surface token instead. repo_tests/single_attribute_writer_18240_test.py fails if anything but useTheme writes data-theme, or anything but useUiMode (and its pre-mount copy in theme-variant-init.ts) writes data-ui, with contrast pairs for the matcher and a control that the scan sees each canonical writer.
Refs #17856 row 7 routeConfig, getAllRoutes, findRouteByPath, getNavigationMenus and RouteConfig have zero importers: no static import, no dynamic import(), no require, no import.meta.glob, no string path in a build/test/storybook/CI config; the only mentions are two superseded plans under docs/superpowers. Router entries it described are live in router/index.ts, or tracked: /monitoring and /monitoring/system redirect to the SLM console, /monitoring/analytics is CodebaseAnalytics, /tools/files is FileBrowser (mockup screen in step 5), /tools/voice lives in chat voice, /monitoring/rum is #18277, /monitoring/validation is #18278.
…overable and touch-safe, drop the double read on icon links (#18240)
…y at a fresh sign-in, never on session restore (#18240)
…sh sign-in, never on session restore (#18240)
… first fresh-import test no longer times out under load (#18240)
mrveiss
force-pushed
the
issue-18240-shell
branch
from
October 12, 2026 10:10
b90e710 to
fbb50bf
Compare
3 tasks
Owner
Author
|
Delta review (Sonnet) at fbb50bf: approve. All three ejection causes are fixed:
The comment-stripper guard files are byte-identical to merge train 7's fix. The cold-import warm-up is scoped to 2 files, and per-test timeouts stay at the default. LOWs:
Queued for the next merge train. |
This was referenced Oct 12, 2026
This was referenced Oct 12, 2026
Merged
This was referenced Oct 12, 2026
ci: train CI keeps finding the same mechanical defects — check them before a train, not in it
#18499
Open
mrveiss
added a commit
that referenced
this pull request
Oct 12, 2026
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #18240
Refs #15469
Refs #17856
Refs #17959
Thinking Path
This is step 2 of the Refined UI redesign (umbrella #18237, step child #18240). It builds the shell from
docs/design/2026-10-11-refined-ui-redesign.md§4.1-4.3: the top nav and icon cluster, a Minimal/Full interface mode that follows the account, and Settings › Appearance (preferred mode, plus themes deployed via the SLM). It also covers the brand mark from #15469 and retiresconfig/routes.ts(#17856 row 7).Owner rule: reuse the canonical implementation and never duplicate it. Before writing anything, every planned new piece was audited against the codebase by concept.
ThemePresetPickeris extended instead.useAnnounce, which both apps now use.NavOverflowMenu,useSystemStatus, the kitBaseModal,usePreferencesanduseTheme.useUiMode, and the preferred-mode card.Duplicates found on the way were merged:
data-themewriters. Now there is one, and a guard enforces it.theme_variantwas saved only on the device. It now syncs with the account.Owner decisions (2026-10-11/12):
Single-issue rationale
This is one build step of the Refined UI redesign (#18240), with its backend change and the #17959 tooling fix that blocked its push. Each linked issue stays open because an acceptance criterion outside this diff remains (the visual review, #18266's social preview, other #17856 rows, #17959 AC2/AC3), so nothing here is closed and nothing else of the same scope is waiting to batch with it.
What Changed
Backend, the one backend change:
autobot-backend/api/users.pyUserPreferencesgains three fields, anchored with\z:ui_mode(minimal|full, defaultfull);ui_start(last|minimal|full, defaultlast);theme_variant, using the same id charset astheme_install._ID.PATCH /users/me/preferencesis now truly partial. It writes onlymodel_fields_set, following thechat_folders.pyprecedent. A client that omits a field can no longer reset it. The response shape is unchanged.tests/api/test_user_preferences_partial_patch.py. They include the contrast pair: a PATCH that omits a field keeps it, and a PATCH that sends it changes it.types/generated/api.tswas updated by hand, because the generator needs a builtopenapi.json.Kit (
libs/autobot-ui)BrandMarkandassets/brand/mark.svgreplace three hand-made tiles: the main app header, the SLM sidebar and login, and the main app login. Both favicons andassets/logo.svg(previously the Vue scaffold) use the mark.BaseTooltipis built on the existing--tooltip-*and--z-tooltiptokens. It opens on hover and focus and closes on Esc. You can move the pointer onto it (WCAG 1.4.13), and it is touch-safe.useAnnounceis promoted from the SLM, which now re-exports it.LanguageSettingsPanelandThemePresetPickeruse it.Shell (
autobot-frontend)App.vuegoes from 1144 to 500 lines, and its size ceiling entry is deleted. The header moves tocomponents/layout/:AppHeader,TopNavLinks,NavIconCluster,NavIconLink,UiModeToggle,ProfileMenu,MobileNavPanelandSystemStatusModal(on the kitBaseModal, replacing a hand-built overlay).config/navItems.tsstays the one registry. It gainsadv/overflowflags and icon-cluster entries, and the Platform admin link is gated byhasServiceManagementand opens in a new tab withrel="noopener noreferrer".useUiModeis a singleton. It setsdata-uibefore mount, syncs with the account, and announces changes. The.adv/.only-minrules use no!important.Settings › Appearance gains
UiModeSettingsCard.ThemePresetPickeris extended withAdditionalThemesGroup, which shows SLM-deployed themes with a "Deployed via SLM" badge.There is now one
data-themewriter,useTheme.useUserStore.applyTheme/updateThemeand the ProfileModal theme UI are removed. Guard:repo_tests/single_attribute_writer_18240_test.py, which also coversdata-ui.inventory(canonical): 19 concepts with more than one definition and no existing issue — led by two secret stores whose docstrings each claim they cannot disagree about the key #17856 row 7:
config/routes.tsis retired. It had 0 static or dynamic importers.routes.tsentryrouter/index.tsSystemMonitorCodebaseAnalytics.vue, liveFileBrowser.vueexists; its mockup screen is built in step 5VoiceConversationOverlay/Panel)i18n: every new string is in all 11 locales, append-only.
Tooling, #17959 (rides here because it blocked this push): a worktree's app tests resolved the main checkout's older
@autobot/ui. The 25 layout tests that use the new kit exports failed in pre-push, though they pass against the in-tree kit.libs/autobot-ui/vite-resolve.ts.autobotUiResolve()aliases@autobot/uito the in-treeindex.tsand dedupesvue/@vueuse/coreto the app's copy.autobot-frontend/vite.config.js,autobot-slm-frontend/vite.config.tsandautobot-slm-frontend/vitest.config.ts.tools/git-hooks/pre-pushnow claimslibs/autobot-uichanges through both apps: vue-tsc, plusvitest relatedon the changed kit files. It no longer reports them "COULD NOT RUN (unclaimed trees)", so pushes after this merges no longer needAUTOBOT_PREPUSH_ALLOW_UNRUNNABLE. This push itself still used the flag, with the owner's approval: the shared installed hook in.git/hookspredates the fix and changes only after merge and reinstall. Every check that can run passed in pre-push: vue-tsc in both apps, vitest (in-tree kit) and pytest.TestTheKitIsClaimedThroughTheApps, 5 tests that fail on the old hook.User-visible changes:
Ratchets, all down:
App.vueandThemePresetPickerceiling entries removed).css_rule_declarations9378 → 9374,components_declaring_styles379 → 378,hardcoded_zindex51 → 50.Follow-ups filed under #18241:
useSystemStatusare hardcoded English, and the file is over 600 lines.DarkModeToggle,LanguageSwitcherand theprofile.theme*keys are now unused. Each will be wired in or removed with evidence.Acceptance criteria
aria-pressed(UiModeToggle.vue), a tooltip (BaseTooltip) and an aria-live announcement (kituseAnnounce). Mode and theme persist per user server-side (users.py,usePreferences.ts; partial-PATCH tests).libs/autobot-ui/src/assets/brand/mark.svg, README). Both favicons use the mark.assets/logo.svgno longer holds the Vue scaffold.npx vitest run. Contrast:UiModeToggle.test.ts"pressing it enters Minimal…" fails against the main checkout's kit and passes in-tree. The pre-push hook now claims kit changes through the apps.node_modulesof its own, so only onevuecopy is reachable with or without dedupe. The candidate test passed either way, so it was removed rather than kept as false coverage. AC3 (production build unchanged) is argued, not measured: in CI thefile:link resolves to the sameindex.ts, and novite buildwas run.config/routes.tsretired with zero-importer evidence, and every entry is accounted for in the table above.Verification
At
1ee0bd4ebc, after the rebase ontoorigin/main:npm run type-checkpassed in autobot-frontend and autobot-slm-frontend: rc=0.useProviderFallbackChip,AdvancedControlView) pass when run alone and are outside this change (test(frontend): four vitest files fail in the full run but pass in isolation #18254 family).reach_scope_claim_17844_testrun alone: 10 passed.lift_locale_translations.py --check: rc=0.pre-commit run --from-ref origin/main --to-ref HEAD: 0 failed.After the review-fix commits (
7eef154e9b,d1fc226c53,27c59d0be9):useAiAnalysis, passes 4/4 when run alone.Notes:
@autobot/uito the in-tree kit, because a worktree'snode_modulesresolves the main checkout's older kit (tooling(frontend): in a worktree, vite/vitest still load the MAIN checkout's @autobot/ui — kit changes are not exercised by app tests until CI #17959).Model Used
Claude Opus 5.5 (coordinating). Sonnet subagents planned, audited duplication, implemented phases A, B and C, ran verification and the review, and fixed its findings.
🤖 Generated with Claude Code