ui: mobile navigation drawer & theme variant refinements#45107
ui: mobile navigation drawer & theme variant refinements#45107
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 637749f55d
ℹ️ 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".
Greptile SummaryThis PR delivers a mobile navigation drawer, a restructured topnav/sidebar shell, and a new in-header chat model picker with supporting test coverage across five areas. Key changes:
One logic issue was found: Confidence Score: 3/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: ui/src/styles/layout.mobile.css
Line: 223-236
Comment:
**Duplicate selector in same media query**
`.topnav-shell__actions` is declared twice inside the same `@media (max-width: 768px)` block — once for `width` / `justify-content` (line 223) and again for `gap` / `align-items` (line 233). Both sets of declarations apply (the cascade is additive), but having two separate rule sets for the same selector in the same scope makes it harder to trace which properties the element actually receives and is easy to leave in an inconsistent state on future edits. These should be consolidated into a single block:
```suggestion
.topnav-shell__actions {
width: 100%;
justify-content: space-between;
gap: 10px;
align-items: stretch;
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: ui/src/ui/app-render.helpers.ts
Line: 436-448
Comment:
**Optimistic override not rolled back on RPC failure**
`state.chatModelOverrides` is written optimistically before the `sessions.patch` call, but the `catch` block only records `state.lastError` — it never restores the previous value. If the RPC throws, the model picker permanently displays the user-selected model string while the server session still holds the old model. This silently shows stale UI until a full refresh or another successful patch.
The fix is to capture the previous override value before the optimistic write and restore it in the `catch` branch:
```ts
const prevOverride = state.chatModelOverrides[targetSessionKey];
state.chatModelOverrides = { ...state.chatModelOverrides, [targetSessionKey]: nextModel || null };
try {
// ... existing request + refresh ...
} catch (err) {
// Roll back so the picker reflects the actual server model.
state.chatModelOverrides = { ...state.chatModelOverrides, [targetSessionKey]: prevOverride };
state.lastError = `Failed to set model: ${String(err)}`;
}
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: fa65d9b |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa65d9b7e8
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8397128565
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eea1ab3c99
ℹ️ 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".
| const navDrawerOpen = Boolean(state.navDrawerOpen && !chatFocus && !state.onboarding); | ||
| const navCollapsed = Boolean(state.settings.navCollapsed && !navDrawerOpen); |
There was a problem hiding this comment.
Clear drawer-open state when not in drawer viewport
The derived nav state keeps state.navDrawerOpen active regardless of viewport width, so if a user opens the drawer on tablet/mobile and then resizes to desktop, navCollapsed is forced false (settings.navCollapsed && !navDrawerOpen) even though desktop has no visible drawer toggle/backdrop to close it. In that state, the sidebar collapse control appears ineffective until some unrelated action (like tab navigation) resets the flag, which is a persistent navigation regression for responsive resize flows.
Useful? React with 👍 / 👎.
…del override cache
…autofill pollution
…e CSS, propagate effective collapsed state to nav items
eea1ab3 to
9c34203
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c342036d1
ℹ️ 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".
| // Prefer the local cache — it reflects in-flight patches before sessionsResult refreshes. | ||
| const cached = state.chatModelOverrides[state.sessionKey]; | ||
| if (typeof cached === "string") { | ||
| return cached.trim(); |
There was a problem hiding this comment.
Invalidate stale model override cache after reset flows
resolveModelOverrideValue always trusts chatModelOverrides before reading sessionsResult, but this cache is only written (in picker/slash-command paths) and never cleared when the session model is recomputed elsewhere; after a sessions.reset path (for example the chat "Clear history" action), the gateway recomputes model defaults, yet the picker can stay pinned to the old cached override and show the wrong active model until a full reload.
Useful? React with 👍 / 👎.
…penclaw#45107) thanks @BunsDev ## Summary - Mobile navigation drawer with slide-over behavior at ≤1100px - Topnav & sidebar shell restructure with brand eyebrow - Chat model selection picker with optimistic caching + rollback - Nav breakpoint gap fix (769–1100px toggle visibility) - Skills page autofill pollution fix (autocomplete=off) - Delete confirm popover positioning (left/right by role) - Effective collapsed state propagation to nav items in drawer mode - Duplicate CSS selector consolidation - Session key race condition fixes in async model patching - 2 new test files + expanded test coverage (23 tests) Co-authored-by: Nova <nova@openclaw.ai>
…penclaw#45107) thanks @BunsDev ## Summary - Mobile navigation drawer with slide-over behavior at ≤1100px - Topnav & sidebar shell restructure with brand eyebrow - Chat model selection picker with optimistic caching + rollback - Nav breakpoint gap fix (769–1100px toggle visibility) - Skills page autofill pollution fix (autocomplete=off) - Delete confirm popover positioning (left/right by role) - Effective collapsed state propagation to nav items in drawer mode - Duplicate CSS selector consolidation - Session key race condition fixes in async model patching - 2 new test files + expanded test coverage (23 tests) Co-authored-by: Nova <nova@openclaw.ai>
…penclaw#45107) thanks @BunsDev ## Summary - Mobile navigation drawer with slide-over behavior at ≤1100px - Topnav & sidebar shell restructure with brand eyebrow - Chat model selection picker with optimistic caching + rollback - Nav breakpoint gap fix (769–1100px toggle visibility) - Skills page autofill pollution fix (autocomplete=off) - Delete confirm popover positioning (left/right by role) - Effective collapsed state propagation to nav items in drawer mode - Duplicate CSS selector consolidation - Session key race condition fixes in async model patching - 2 new test files + expanded test coverage (23 tests) Co-authored-by: Nova <nova@openclaw.ai>
…penclaw#45107) thanks @BunsDev ## Summary - Mobile navigation drawer with slide-over behavior at ≤1100px - Topnav & sidebar shell restructure with brand eyebrow - Chat model selection picker with optimistic caching + rollback - Nav breakpoint gap fix (769–1100px toggle visibility) - Skills page autofill pollution fix (autocomplete=off) - Delete confirm popover positioning (left/right by role) - Effective collapsed state propagation to nav items in drawer mode - Duplicate CSS selector consolidation - Session key race condition fixes in async model patching - 2 new test files + expanded test coverage (23 tests) Co-authored-by: Nova <nova@openclaw.ai>
…penclaw#45107) thanks @BunsDev ## Summary - Mobile navigation drawer with slide-over behavior at ≤1100px - Topnav & sidebar shell restructure with brand eyebrow - Chat model selection picker with optimistic caching + rollback - Nav breakpoint gap fix (769–1100px toggle visibility) - Skills page autofill pollution fix (autocomplete=off) - Delete confirm popover positioning (left/right by role) - Effective collapsed state propagation to nav items in drawer mode - Duplicate CSS selector consolidation - Session key race condition fixes in async model patching - 2 new test files + expanded test coverage (23 tests) Co-authored-by: Nova <nova@openclaw.ai>
…penclaw#45107) thanks @BunsDev ## Summary - Mobile navigation drawer with slide-over behavior at ≤1100px - Topnav & sidebar shell restructure with brand eyebrow - Chat model selection picker with optimistic caching + rollback - Nav breakpoint gap fix (769–1100px toggle visibility) - Skills page autofill pollution fix (autocomplete=off) - Delete confirm popover positioning (left/right by role) - Effective collapsed state propagation to nav items in drawer mode - Duplicate CSS selector consolidation - Session key race condition fixes in async model patching - 2 new test files + expanded test coverage (23 tests) Co-authored-by: Nova <nova@openclaw.ai>
Summary
Implements mobile navigation drawer, topnav/sidebar shell restructure, and chat model selection enhancements.
Changes
Files touched
15 files, +1,359 / -540 lines (5 commits)