Uh oh!
There was an error while loading. Please reload this page.
fix(ui): separate session row actions from navigation - #2566
Conversation
7ce5220 to
b717680Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Approved — separating the navigation button and MoreMenu at the existing SideNavItem boundary is the smallest correct fix for the invalid nested-button structure. The menu state, rename focus handoff, and pending-action guard remain on the same path, while the action trigger keeps the original trailing-column geometry. I did not find a reproducible click, keyboard, focus, RTL, narrow-width, or zoom regression.
I left one P2 inline for a current-main integration issue. It is a one-line import correction rather than a product-design concern, so it does not need to block this approval, but the branch should be rebased and CI rerun before merge.
One optional visual detail to confirm: stale rows previously dimmed the nested menu together with the navigation button. As a sibling, the action trigger now remains at full opacity. The position and dimensions are unchanged; this is only a small emphasis difference, so I am not treating it as a finding.
中文对照
已批准。在现有 SideNavItem 边界将导航按钮和 MoreMenu 分离,是修复无效嵌套按钮结构的最小正确方案。菜单状态、重命名焦点交接和异步操作防重入仍沿用原路径,action trigger 也保持原有尾部列几何位置。没有发现可复现的点击、键盘、焦点、RTL、窄宽或缩放回归。
我留下了一条 P2 inline,指出与当前 main 组合时的一处问题。它只需修正一行 import,并非产品设计问题,因此不需要阻塞本次批准;但合并前应 rebase 并重新运行 CI。
另有一个可选的视觉细节请作者确认:旧实现中 stale 行会连同嵌套菜单一起变淡;菜单成为 sibling 后会保持完整透明度。位置和尺寸没有变化,只是很小的强调层级差异,因此不作为 finding。
AI-assisted review disclosure: Codex assisted with diff inspection, adversarial interaction and layout review, current-main compatibility verification, and draft wording. Astro-Han verified the surviving issue, chose its severity and scope, and made the final approval decision.
Uh oh!
There was an error while loading. Please reload this page.
b717680 to
13571c6Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for fixing the current-main import during the rebase. I reviewed the final one-line increment on 13571c6; the test now uses the supported @maka/core/session entry point, and the fresh CI run is fully green.
The underlying UI change remains the smallest correct fix: navigation and the row action are sibling controls, while the existing menu, rename, focus, and pending-action behavior stays on the same path. I did not find any remaining reproducible P0–P3 issues.
The final head is cleanly mergeable. Approved and ready to merge.
AI-assisted review disclosure
Codex performed the incremental source and CI inspection. I, Astro-Han, reviewed the evidence and made the final review decision.
Uh oh!
There was an error while loading. Please reload this page.
Summary
Session rows rendered their trailing action menu inside the row navigation button, producing invalid nested-button markup and a React hydration warning.
This keeps the row navigation and
MoreMenuas sibling controls while preserving the existing trailing column, status indicators, rename focus handoff, and pending-action guard. The Storybook smoke contract now checks both that every populated row has its sibling action control and that no row contains nested buttons.Fixes#2510
中文说明
会话侧栏此前把操作菜单按钮放在整行导航按钮内部,形成无效的嵌套
button,并在 Desktop 启动时触发 React hydration 警告。本 PR 将行导航与
MoreMenu调整为同级控件,同时保留原有的尾部列、状态提示、重命名焦点交接和异步操作防重入逻辑。Storybook smoke 也会验证每个会话行都使用同级操作控件,且不再出现嵌套按钮。Verification
npm --workspace @maka/ui test— 263/263 passnpm --workspace @maka/desktop run typecheck:storiesnode --test scripts/storybook-visual-smoke.test.mjsnpx biome checkon the four changed filesChecklist
Does this PR entail a change in behavior?