feat(web): add theme system with light/dark/night modes and compact u… - #432
feat(web): add theme system with light/dark/night modes and compact u…#432Emmanuek5 wants to merge 1 commit into
Conversation
…ser messages - Add theme selector in sidebar with 4 options: light, dark, night, system - Extend useTheme hook to support new theme types - Add CSS variables for dark (balanced) and night (darkest) themes - Make user message cards more compact: - Reduced padding (px-3 py-2) - Smaller text (13px) - Actions appear below card on hover - Update CHANGELOG.md
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d09f3ee500
ℹ️ 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".
| {THEME_OPTIONS.map((option) => ( | ||
| <MenuRadioItem key={option.value} value={option.value}> | ||
| {option.label} |
There was a problem hiding this comment.
Adding night to the sidebar selector makes that value reachable, but the settings route still defines its own THEME_OPTIONS with only system/light/dark (apps/web/src/routes/_chat.settings.tsx), so users who pick Night in the sidebar see no selected radio in Settings and cannot manage that choice there. This inconsistency was introduced by adding a second source of truth for theme options and will confuse users unless both surfaces share the same option list.
Useful? React with 👍 / 👎.
| </p> | ||
| </div> | ||
| </div> | ||
| <div className="mt-1 flex items-center gap-1.5 opacity-0 transition-opacity duration-200 focus-within:opacity-100 group-hover:opacity-100"> |
There was a problem hiding this comment.
The user-message action row is now hidden with opacity-0 and only revealed via group-hover/focus-within, which breaks discoverability and access on touch/non-hover environments; in particular, rows without focusable actions (for example image-only messages without revert) have no way to trigger focus-within, so their timestamp remains permanently hidden unless hover exists. This regresses core message controls and metadata visibility on mobile.
Useful? React with 👍 / 👎.
|
Light/dark toggle too prominent. Empty space under messages is just as bad as the extra space for copy buttons Not really interested in ui prs rn sorry |
…g#432) server_main.rs:1715 was a bare `dispatch_ws_frame(frame, &tx, &state).await` inside the socket's read loop, which makes the reader and the request handler the same task. While a handler is in flight NOTHING is read from the socket, so the frames whose entire purpose is to arrive DURING a long request cannot arrive at all: * Interrupt (pingdotgg#411) is the stop button. Serialized, the earliest it can be read is after the request it was cancelling has already finished, so it is unreachable BY CONSTRUCTION -- no hung handler required to call this broken. * Ack is Effect RPC's per-chunk flow-control frame. * every later Request on the socket, which is why one slow handler presents as 'the whole app hung' / 'Reconnecting to Local (Rust)'. Requests are multiplexed by requestId -- the wire format already says they are concurrent -- so Request is spawned and the cheap tags (Ping/Ack/Interrupt) stay INLINE and therefore stay ordered. Interrupt must not be reordered behind a Request; getting it off the blocked path is the whole point. dispatch_ws_frame still awaits everything it is given, so every existing test that drives it directly keeps its determinism. The concurrency is introduced at the transport, which is the layer whose job is to keep reading. NOTE pingdotgg#432's stated mechanism is wrong in a way that matters: it says 'subscribeThread never returns'. It does -- :3073 ends that arm with spawn_thread_tail, which spawns. The defect is not one hanging method, it is that ANY slow handler blocks the reader. Fixing the named method would have left the class intact. The test goes through a REAL socket (build_app on port 0 + tokio-tungstenite) because that is the only place this is visible: every existing ws test calls dispatch_ws_frame as a function and structurally cannot see a defect in the loop above it. Same lesson as pingdotgg#404's own comment. The assertion is an ORDERING (a pipelined Ping overtakes three in-flight slow requests), not a timing threshold, so it cannot flake on a loaded box. tokio-tungstenite 0.29 as a dev-dep adds no package to the graph -- axum's ws feature already resolves it (Cargo.lock:4337).
pingdotgg#476) My first pingdotgg#432 fix used a bare tokio::spawn. That un-starves the reader and DETACHES the work in the same line: orchestration.subscribeThread attaches a durable tail and streams for the life of the thread, so a client disconnecting mid-stream left a task writing into a tx whose receiver is gone, holding an AppState clone. Reconnect and there are two. Serialization had been the backpressure and I removed it without putting ownership in its place. The JoinSet design is claude-4e91's and it is better than what I wrote. Their cell declares it, reaps it and shuts it down but never spawns into it (there is no inflight.spawn in that file, and :1742 is still the inline await), so this is their design with the spawn actually connected -- not a second copy of it. Ownership is now structural: the set is owned by handle_socket, so tasks cannot outlive the connection, and shutdown().await on close makes them gone BEFORE the function returns rather than eventually. try_join_next each iteration reaps completed slots, which a long-lived socket otherwise accumulates one per request. AppState::inflight_requests is an OBSERVABLE, not a control -- nothing reads it to decide anything, so it cannot change the behaviour it measures. It exists because a detached task and an owned one are byte-identical on the wire and the difference only ever surfaces as a leak much later. Decremented by a Drop GUARD, not a trailing statement: a trailing statement does not run when a task is ABORTED, and abort is exactly the path pingdotgg#476 is about -- a counter blind to the abort would make the test vacuous. The test asserts the count RETURNS TO ZERO under a bounded poll, and first asserts it ROSE ABOVE ZERO so a disconnect that races the server picking the work up fails loudly instead of passing green.
…pingdotgg#437 The shared checkout was left mid-merge with three unmerged index entries, so open_cell refused every cell in this repo and the whole bench was blocked. The working-tree "resolution" that was left behind took THEIRS wholesale for backend/Cargo.toml and backend/src/server_main.rs. Those branches diverged (merge-base b712e7e), and HEAD 947c882 carried 239 lines theirs does not have, so that resolution silently discarded: * pingdotgg#436 thread_snapshot_http + GET /api/orchestration/threads/{thread_id}. Absent from theirs entirely (0 hits). Without it draft promotion 404s and the view stays parked on /draft/<id> while the turn completes elsewhere. * pingdotgg#437 the thread.created emit in ensure_thread_on_shell. Theirs only references the name; it never emits it. This is the single event orchestrationEventEffects.ts:31 promotes a draft on. Kept THEIRS for the WS read loop: its JoinSet makes the request task's lifetime structural (dropped with the connection) instead of a bare detached tokio::spawn that leaks an AppState clone per dead client, and it keeps non-Request frames inline so Interrupt cannot be reordered behind a Request. That is strictly better than the ours-side spawn, so ours' pingdotgg#432 hunk is deliberately dropped rather than merged. Cargo.toml: theirs is a strict superset (same tokio-tungstenite 0.29, plus futures-util), so nothing is lost by taking it. contract_tests.rs was the file that actually stopped the merge: mod ws_head_of_line was left unclosed, so the bin test target would not compile at all. The two tests merged in after it are self-contained (their own futures_util imports, their own inline serve) and sit at column 0, so the module is closed before them and they stay at file scope. woodbine, cell claude-9d24: theirs alone 156 passed, 7 failed this merge 158 passed, the SAME 7 failed (124/124 lib tests pass) The 7 are pre-existing on theirs and untouched here. No test deleted, no #[ignore] added. Evidence: merge-3021-ws-head-of-line-91b7fdb8e6e36e47
2 participants
Summary
Adds a three-tier theme system and improves the user message UI for a cleaner, more compact look.
Changes
Theme System
Light– bright modeDark– balanced dark (neutral-900 based, good everyday contrast)Night– deepest dark (neutral-950 based, minimal light)System– follows OS preferencelocalStorageUser Messages
Files Changed
THEME_OPTIONSand Theme type@variant dark(balanced) and@variant night(darkest) CSS variable blockss/Codes/t3code/apps/web/src/components/Sidebar.tsx:0:0-0:0) | Added theme selector menu in sidebar header for both desktop and web layouts |
| apps/web/src/components/ChatView.tsx | Compact user message card styling; actions moved below card on hover |
| CHANGELOG.md | Created and documented all changes |
Testing
bun typecheckpassesNote
Add a theme selector to the web Sidebar header and apply
useTheme.applyThemeto support light, dark, night, and system modes with compact user message styling inChatViewIntroduce a theme menu with
THEME_OPTIONSandThemeIcon, updateuseThemeto toggledarkandnightclasses, and add CSS variants for night mode; make user messages inChatViewrender in a compact layout with actions and timestamp below the bubble.📍Where to Start
Start with the theme logic in
useTheme, focusing onapplyThemeanduseThemein apps/web/src/hooks/useTheme.ts.Macroscope summarized d09f3ee.