Skip to content

fix(sessions): board folder indent parity with list view (#3909) - #3922

Merged
bolichen97 merged 1 commit into
mainfrom
fix/board-folder-indent-3909
Aug 16, 2026
Merged

fix(sessions): board folder indent parity with list view (#3909)#3922
bolichen97 merged 1 commit into
mainfrom
fix/board-folder-indent-3909

Conversation

@chenmingwei23

Copy link
Copy Markdown
Contributor

Problem / Motivation

The sidebar's two views render nested folder bodies with different connector treatments even though both draw session rows through the same renderSessionRow. The list view's folder body uses ml-4 (16px margin + 1px connector border) with no left padding, so nested session rows sit flush against the connector line. The board/kanban column folder body in ChatSidebar.tsx still used border-l border-border ml-2 pl-1, so in board view the same rows sat visibly offset from the connector at a shallower indent. Follow-up from the First Principles advisory on PR #3903.

Why it matters

Users who switch between the list and board views see the same folders drawn with two different nesting geometries. The inconsistency reads as sloppiness in the dashboard's primary navigation surface, and every future visual pass on folder nesting has to reconcile two divergent class lists instead of one.

What changed (motivation -> approach -> change)

Symptom: nested session rows sit flush against the connector in list view but offset in board view. Root cause: the board-view folder body wrapper carries its own divergent class list (ml-2 pl-1) rather than the list view's flush-connector treatment (ml-4, no pl). Change: drop pl-1 and widen ml-2 to ml-4 on the board-view folder body so both views share the same connector geometry. Verified visually that board columns (min-w-[220px] flex-1) absorb the extra 8px without clipping -- session rows are flex children with truncate, so text compresses rather than overflows.

Scope note: the list view's folder body also carries mb-1 rounded-bl-md (bottom spacing + rounded connector tail). Those are not indentation and were left out deliberately to keep this change exactly what the issue names; they can ride with a future board-view visual pass.

Tests

No test changes: no existing component or snapshot test asserts on the board-view folder body class list (the existing geometry guard website/src/test/ChatSidebar.folderAlignment.test.tsx pins the list-view folder body to ml-4 present / pl-1 absent -- this change moves board view toward that same contract). Full ChatSidebar vitest set (43 files, 329 tests) passes.

Manual verification

Ran an isolated gateway + Vite dev server from this branch, seeded a folder ("Radar work", 3 sessions) with a nested subfolder ("Deep dives", 1 session) plus a tagged board column, and captured before/after screenshots of both views (scripted Playwright, headless Chromium). Board view now renders nested rows flush against the connector at the same 16px step as the list view; the list view is byte-identical before vs after (untouched, identical PNG hashes).

Screenshots / video

Board view -- before (ml-2 + pl-1, rows offset from connector):

board before

Board view -- after (ml-4, rows flush against connector, matching list view):

board after

List view (unchanged control -- before and after are byte-identical)

list before

list after

Related Issues

Closes #3909

Checklist

  • Single commit with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable) -- N/A, visual-parity class change only
  • No secrets, credentials, or internal references in the diff

The board/kanban column folder body used border-l ml-2 pl-1 while the
list view folder body uses border-l ml-4 with no pl, so nested session
rows sat offset from the connector line in board view only. Both views
render rows through the same renderSessionRow, so the container class
list is the only divergence. Drop pl-1 and widen ml-2 to ml-4 to match
the list view's flush-connector treatment.

Closes #3909
@chenmingwei23
chenmingwei23 requested a review from a team August 16, 2026 07:13
@chenmingwei23
chenmingwei23 requested a review from a team as a code owner August 16, 2026 07:13
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of c62add54ae80597272e36480d75e242f90001b85 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] c62add5

False positive or not applicable? A repository writer can comment:
/ai-review override gpt c62add54ae80597272e36480d75e242f90001b85: <one-sentence reason>

@github-actions

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — ✅ PASS

Advisory UX-level review of c62add54ae80597272e36480d75e242f90001b85 — updated in place on each push; does not block merge.

UX-Verdict: PASS

Three-line class tweak that removes a visible list/board nesting mismatch; after-board screenshot matches the list view's flush-connector geometry exactly, and truncation absorbs the deeper indent.

[UX-REVIEWED] c62add5

@github-actions

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed c62add54ae80597272e36480d75e242f90001b85 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] c62add5

Verdict parsed from the review's SHA-scoped output markers for commit c62add54ae80597272e36480d75e242f90001b85.

False positive or not applicable? A repository writer can comment:
/ai-review override fable c62add54ae80597272e36480d75e242f90001b85: <one-sentence reason>

@github-actions

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

Advisory design-level review of c62add54ae80597272e36480d75e242f90001b85 — updated in place on each push; does not block merge.

Design-Verdict: CONCERNS

One-line CSS fix is sound; the PR also commits four binary screenshots under temp-screenshots/ — verification leftovers that don't belong in the tree.

Watch

  • Four PNGs land in temp-screenshots/board-folder-indent-3909/ (~80KB) with no mention in the description. Cause: verification artifacts committed alongside the fix → they enter git history permanently and seed a precedent for dumping evidence dirs at repo root → repo bloat with no reader. Move them to the PR description and drop the directory from the commit. (before-list.png and after-list.png are byte-identical in size, reinforcing they're process artifacts, not assets anything references.)

Suggestions

  • The list-view wrapper this claims parity with is border-l border-border mb-1 ml-4 rounded-bl-md (ChatSidebar.tsx:3271); the board wrapper omits mb-1 rounded-bl-md. If full flush-connector parity is the intent the comment states, match the whole class list or note why the two are intentionally excluded.

[DESIGN-REVIEWED] c62add5

@github-actions

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — ✅ PASS

Advisory premise-level review of c62add54ae80597272e36480d75e242f90001b85 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push; does not block merge.

Both files read. The change is a one-line Tailwind class edit in website/src/pages/ChatSidebar.tsx plus four committed screenshots. I verified the surrounding code: the list-view wrapper at ChatSidebar.tsx:3271/3278 is border-l border-border mb-1 ml-4 rounded-bl-md, the board wrapper is now border-l border-border ml-4, the geometry guard test pins list view only, and committing screenshots under temp-screenshots/ is explicit repo convention (.gitignore:89 calls them "committed deliverables"; 1300+ files already exist there). The two list-view PNGs in the patch share one blob hash (e4ce0755), confirming the "byte-identical control" claim.

First-Principles-Verdict: PASS

A named defect (#3909, prior advisory follow-up) fixed at the divergent class list itself; every rider is declared and conventional.

What this change ships

Intent: make nested folder rows in board view indent and hug the connector exactly as in list view — a FIX.

  1. Board-view nested rows indent 16px flush against the connector, matching list view — justified (issue Bring board-view folder body indentation to parity with the list view #3909).
  2. Board empty-folder "new chat" affordance shifts with the same wrapper — rides along, same geometry, declared by implication.
  3. Four before/after screenshots under temp-screenshots/board-folder-indent-3909/ — declared; repo-conventional deliverables (.gitignore:89).

Watch

The description names the cause as "two divergent class lists instead of one", but the lists stay divergent by mb-1 rounded-bl-md (board ChatSidebar.tsx:2376 vs list 3271/3278 — counted, pattern border-l border-border). The scope note declares the deferral, so the future visual-pass cost the description cites is reduced, not removed.

Subtractions

  • Drop one of before-list.png / after-list.png — they are the same blob (e4ce0755 twice in the patch); one PNG plus the description's "byte-identical" note carries the control claim.

[FIRST-PRINCIPLES-REVIEWED] c62add5

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Aug 16, 2026
@chenmingwei23

Copy link
Copy Markdown
Contributor Author

Disposition for Design Review (CONCERNS on c62add5) -- one item per finding:

Watch: four PNGs committed under temp-screenshots/ -- REBUTTED. Committing review evidence there is this repository's documented convention, not a leftover: temp-screenshots/README.md states the directory holds PR review evidence, explains why the files are committed rather than attached (automated PR workflows cannot produce a user-attachments upload, and only a committed file puts the PR in scope for the UX review path gate in ux-review.yml, which greps changed paths for temp-screenshots/), and the directory is pruned periodically while SHA-pinned URLs keep resolving. Moving the images "to the PR description" would break both properties. Two factual corrections: the description does reference the screenshots (the Screenshots section embeds all four with SHA-pinned URLs), and the byte-identical before-list.png/after-list.png pair is deliberate and called out in the body -- it is the unchanged-control evidence that the list view was not touched.

Suggestion: mb-1 rounded-bl-md omitted from the board wrapper -- REBUTTED (already noted as intentional). The suggestion offers two acceptable outcomes: match the whole class list, or note why the exclusion is intentional. The PR body's scope note does the latter explicitly: mb-1 (bottom spacing) and rounded-bl-md (connector tail radius) are not indentation, and issue #3909 names exactly pl-1 and ml-2 -> ml-4; the two classes are left for a board-view visual pass so this diff stays exactly what the issue asks. The in-code comment scopes its parity claim to the connector indentation the change makes.

No code change; both dispositions are prose-only on the reviewed head.

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tier 1 auto-approve: cosmetic-UI (5 files). Criteria: no conflict, no requested changes, security path denylist clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: cosmetic-UI — board folder indent parity with list view, CSS alignment only (4 screenshots + 1 component).

@bolichen97
bolichen97 enabled auto-merge (squash) August 16, 2026 08:03
@bolichen97
bolichen97 merged commit d2d2acd into main Aug 16, 2026
58 checks passed
@bolichen97
bolichen97 deleted the fix/board-folder-indent-3909 branch August 16, 2026 08:03
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 16, 2026

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tier 1 auto-approve: cosmetic-UI (5 files). Criteria: no conflict, no requested changes, security path denylist clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: cosmetic-UI — board folder indent parity with list view (ml-2 pl-1 → ml-4 class change).

encomjp pushed a commit to encomjp/kirocrew-customapi that referenced this pull request Aug 22, 2026
…3909) (kirodotdev#3922)

The board/kanban column folder body used border-l ml-2 pl-1 while the
list view folder body uses border-l ml-4 with no pl, so nested session
rows sat offset from the connector line in board view only. Both views
render rows through the same renderSessionRow, so the container class
list is the only divergence. Drop pl-1 and widen ml-2 to ml-4 to match
the list view's flush-connector treatment.

Closes kirodotdev#3909
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.

Bring board-view folder body indentation to parity with the list view

2 participants