Skip to content

fix(desktop): truncate long file paths in the changes panel - #4155

Merged
Astro-Han merged 1 commit into
apache:mainfrom
liuxiaocs7:fix/review-panel-long-path-truncation
Aug 29, 2026
Merged

fix(desktop): truncate long file paths in the changes panel#4155
Astro-Han merged 1 commit into
apache:mainfrom
liuxiaocs7:fix/review-panel-long-path-truncation

Conversation

@liuxiaocs7

@liuxiaocs7liuxiaocs7 commented Aug 29, 2026

Copy link
Copy Markdown
Member

Summary

In the workbar 变更 / Changes panel, a long, deeply-nested file path did not truncate: it overflowed the narrow panel and pushed the change counts (新增 N / 删除 N) and the collapse chevron off the right edge, where the list's overflow: hidden clipped them — with no ellipsis.

The panel already has a truncation chain (.maka-session-review-file-pathmin-width:0; flex:1 + Text maxLines={1}), but the width constraint never reaches it. Astryx Collapsible wraps each trigger row in an intermediate flex <span> that keeps the browser default min-width: auto, so it refuses to shrink below the full nowrap path width and overflows the button. This constrains that label span (min-width: 0; flex: 1 1 auto; overflow: hidden) so the path ellipsizes and the stats + chevron stay in view. flex: 1 1 auto keeps the stats right-aligned for short paths; min-width: 0 lets it shrink for long ones. The rule lives in unlayered review.css and triggerLabel sets no competing flex/min-width, so it overrides cleanly.

Fixes#4154

Verification

  • npx tsc -p apps/desktop/tsconfig.storybook.json --noEmit — no errors
  • npx biome check + npx biome format on the two touched files — pass
  • Added a deep-path fixture (.scratch/pyclient/gen/client/Comparator_pb2.py) to the Product/Session Workbar → Changes story and captured before/after from the built Storybook at a ~360px docked-panel width.

Not run (scope): full-repo npm run lint / typecheck / build / knip.

Screenshots (Changes panel, ~360px)

Before (clipped, no ellipsis)After (path truncates, stats + chevron visible)
beforeafter

Review focus

CSS-only layout fix plus a story fixture; no behavior change to data or IPC. The one thing to sanity-check visually is that short-path rows still right-align their change counts.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code (Opus 4.8) — diagnosed the root cause and wrote the CSS fix and the story fixture. Author reviewed. The fix commit carries a Generated-by trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

The Collapsible trigger wraps each review row in a flex <span> that keeps
the browser default min-width:auto, so a long file path cannot shrink and
overflows the narrow panel — clipping the change counts and chevron off the
right edge instead of ellipsizing the path. Constrain that label span so the
path truncates and the stats stay in view.
Adds a deep-path fixture to the Changes story to exercise the case.
Generated-by: Claude Code (Opus 4.8)

@jackwenerjackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I reviewed exact head 085682665bb12ee16fd224bcb6fc17667c47dd4a and found no P0–P2 issues.

The selector reaches the intermediate Collapsible label span that owns the missing shrink constraint without changing the path, statistics, or chevron components themselves. In a fresh production Storybook build, I loaded the real Changes story in headless Chrome: every statistics block and chevron remained inside its 422 px trigger; the three long paths had text-overflow: ellipsis, white-space: nowrap, and a smaller client width than scroll width, while the short path remained untruncated. Overriding only this new rule back to the previous min-width: auto / unconstrained label reproduced the defect: the first row's statistics and chevron moved beyond the trigger and the path stopped truncating.

The Storybook production build and story typecheck passed, as did Biome and git diff --check. Exact-head hosted CI is green, and the synthetic merge with current main is clean (9c95d5488861dd20ca79b15784a8287a31f034aa).


Automated review notice: This comment was posted by an automated review agent operated by WAWQAQ. It is not an independent human review and does not replace one.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the focused fix and the clear before/after evidence.

I reviewed exact head 085682665bb12ee16fd224bcb6fc17667c47dd4a and found no P0–P3 issues.

The reported problem is real: Astryx Collapsible introduces an intermediate flex label whose default min-width: auto prevents the existing file-path truncation chain from shrinking. Long paths therefore push the change statistics and chevron outside the narrow Changes panel.

The solution fixes the constraint at the closest existing layout seam. It scopes min-width: 0, flexible sizing, and clipping to the Collapsible label inside the Changes row without changing Astryx itself, adding layout state, or duplicating truncation logic. The long-path Storybook fixture exercises the real Workbar → Changes → Collapsible path and is proportionate to this visual regression.

The before/after result was manually confirmed: long paths ellipsize, statistics and chevrons remain visible, and short rows retain their intended alignment. Exact-head test and label checks pass, the PR is cleanly mergeable with current main, and there are no unresolved review threads.

Approved.

Review analysis was assisted by Codex and an independent @reviewer agent. Astro-Han verified the exact head, production layout owner, visual result, CI, mergeability, and severity judgment, and owns this review.

中文对照

谢谢你提交这个边界清晰的修复,并提供明确的前后对比证据。

我审查了精确 head 085682665bb12ee16fd224bcb6fc17667c47dd4a,没有发现 P0–P3 问题。

报告的问题是真实的:Astryx Collapsible 增加的中间 flex label 默认使用 min-width: auto,导致已有的文件路径截断链无法继续收缩。长路径因此会把变更统计和折叠箭头挤出狭窄的 Changes 面板。

当前方案在最近的既有布局边界上修复约束。它只为 Changes 行内的 Collapsible label 增加 min-width: 0、弹性尺寸和裁剪,没有修改 Astryx 本身、增加布局状态或复制截断逻辑。长路径 Storybook fixture 走真实的 Workbar → Changes → Collapsible 路径,规模与本次视觉回归相称。

前后视觉效果已经人工确认:长路径正确显示省略号,统计和折叠箭头保持可见,短路径也保留预期对齐。精确 head 的 testlabel 均已通过,与当前 main 合并干净,并且没有未解决的 Review Thread。

Approve。

本次审查分析由 Codex 和独立的 @reviewer 子代理协助;Astro-Han 核验了精确 head、生产布局 owner、视觉结果、CI、可合并性和问题分级,并对本次 Review 负责。

@Astro-Han
Astro-Han merged commit aa58623 into apache:mainAug 29, 2026
2 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/SUnder 100 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Long file paths in the Changes panel are clipped instead of truncated

3 participants

@liuxiaocs7@jackwener@Astro-Han