Skip to content

Make the selected sidebar thread stand out clearly and never go stale - #123

Merged
marmeladema merged 1 commit into
mainfrom
claude/approval-request-persistence-c47tky
Jul 24, 2026
Merged

Make the selected sidebar thread stand out clearly and never go stale#123
marmeladema merged 1 commit into
mainfrom
claude/approval-request-persistence-c47tky

Conversation

@marmeladema

@marmeladema marmeladema commented Jul 24, 2026

Copy link
Copy Markdown
Owner

What & why

Two problems with the sidebar's selected-thread state:

  1. The selected thread was barely distinguishable. .thread.active used the same --panel2 background as :hover, only brightening the text — in a long list it was hard to tell which thread was open.
  2. Selection could go stale. It was applied imperatively (querySelectorAll + classList.toggle in openThread/openDraftThread). Thread rows are rebuilt whenever the list reloads, so a hand-set highlight could survive a rebuild and leave a previously selected row still looking selected.

Changes

Styling — app.css

  • Restyle .thread.active using the code-editor convention: a neutral raised fill with a faint 9% accent whisper (color-mix, with a plain --panel2 fallback) plus a thin 2px accent left-rail carrying the signal — no colored wash, no bold. Reads as clearly selected in a long list while staying restrained, and derives from the active theme accent (blue in IDE/default/bubbles, green in terminal).
  • .thread.active:hover sits a step above at 13% so the selected row still reacts to hover without jumping.
  • The 9% tint keeps selection clearly above a plain hover (bare --panel2) and coexists cleanly with the leading status glyph (! approval / x error / o running / * unread, each keeping its own semantic color) and the trailing ... actions button.

State — app.js

  • Derive the selection from state.threadId as the single source of truth. markThreadRowActive toggles the class and mirrors it to aria-current; syncActiveThreadHighlight recomputes every row; threadRow applies it at creation so re-renders preserve exactly one active row. openThread and openDraftThread now call the shared helper.

Tests

  • Adds a Playwright test (tests/e2e/tests/thread-selection.spec.ts): create two threads, switch, reload — asserting exactly one active row, that switching moves it, that the accent rail and aria-current are present, and that the highlight survives a full list rebuild on reload.

Screenshots

  • Regenerated the README desktop and mobile screenshots for the new selected-thread styling.

🤖 Generated with Claude Code


Generated by Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced “active” sidebar thread styling with accent tinting and an inset accent rail.
    • Improved thread selection highlighting to be consistent across switching threads and opening drafts.
    • Ensured the correct active thread is reflected for accessibility and remains accurate after reloads.
  • Tests

    • Added end-to-end coverage to verify single active highlight, visual styling via non-none box shadow, correct aria-current state, and persistence after page reload.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fa41b485-6bf3-46c9-adf2-551f9e94ff97

📥 Commits

Reviewing files that changed from the base of the PR and between 408e9d3 and 5ccdd22.

⛔ Files ignored due to path filters (2)
  • docs/screenshots/ide-desktop.png is excluded by !**/*.png, !docs/screenshots/**
  • docs/screenshots/ide-mobile.png is excluded by !**/*.png, !docs/screenshots/**
📒 Files selected for processing (3)
  • crates/giskard-server/static/app.css
  • crates/giskard-server/static/app.js
  • tests/e2e/tests/thread-selection.spec.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • crates/giskard-server/static/app.css
  • tests/e2e/tests/thread-selection.spec.ts
  • crates/giskard-server/static/app.js

📝 Walkthrough

Walkthrough

Changes

The sidebar thread selection now derives active styling and aria-current from state.threadId, synchronizes selection when opening or drafting threads, adds accent-tinted active styling, and introduces Playwright coverage for switching and reload restoration.

Sidebar thread selection

Layer / File(s) Summary
State-driven highlighting
crates/giskard-server/static/app.js, crates/giskard-server/static/app.css
Thread rows derive active state from state.threadId; draft and opened-thread transitions synchronize the sidebar, while active rows receive accent-tinted styling and an accent rail.
Selection behavior validation
tests/e2e/tests/thread-selection.spec.ts
End-to-end tests verify thread switching, visual highlighting, aria-current, and restoration after reload.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main change: improved selected-thread styling and more reliable sidebar selection state.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/approval-request-persistence-c47tky

Comment @coderabbitai help to get the list of available commands.

Two problems with the sidebar's selected-thread state:

- The selected thread was barely distinguishable: .thread.active used the
  same --panel2 background as :hover, only brightening the text, so in a
  long list it was hard to tell which thread was open.
- Selection was applied imperatively (querySelectorAll + classList.toggle
  in openThread/openDraftThread). Thread rows are rebuilt whenever the list
  reloads, so a hand-set highlight could survive a rebuild and leave a
  previously selected row still looking selected.

Styling (app.css): restyle .thread.active using the code-editor
convention -- a neutral raised fill with a faint 9% accent whisper
(color-mix, with a plain --panel2 fallback) plus a thin 2px accent
left-rail carrying the signal, no colored wash and no bold. Reads as
clearly selected in a long list while staying restrained, and derives
from the active theme accent (blue in IDE/default/bubbles, green in
terminal). .thread.active:hover sits a step above at 13%. The tint stays
clearly above a plain hover and coexists with the leading status glyph
and trailing actions button.

State (app.js): derive the selection from state.threadId as the single
source of truth. markThreadRowActive toggles the class and mirrors it to
aria-current; syncActiveThreadHighlight recomputes every row; threadRow
applies it at creation so re-renders preserve exactly one active row.
openThread and openDraftThread call the shared helper.

Tests: add a Playwright test (create two threads, switch, reload)
asserting exactly one active row, that switching moves it, that the
accent rail and aria-current are present, and that the highlight survives
a full list rebuild on reload. Regenerate the README desktop and mobile
screenshots for the new styling.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012P8N8b5SuaxDKEMcAAN73U
@marmeladema
marmeladema force-pushed the claude/approval-request-persistence-c47tky branch from 408e9d3 to 5ccdd22 Compare July 24, 2026 21:07
@marmeladema
marmeladema merged commit 8cd00e3 into main Jul 24, 2026
5 checks passed
@marmeladema
marmeladema deleted the claude/approval-request-persistence-c47tky branch July 24, 2026 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants