[lilyshen0722]

No description provided.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Team-Commonly/commonly
• Branch: main
• File: .coderabbit.yaml
• Commit: fc63afe
• Layers: Repository YAML
• Review profile: QUIET
• Plan: Advanced
• Run ID: f23fcd7a-141e-48f2-ba59-75db8f4dd3bd

📥 Commits

Reviewing files that changed from the base of the PR and between 31576de and 8d49e4a.

⛔ Files ignored due to path filters: 2

• docs/design/evidence/task-356-mention-after-1440.png is excluded by !**/*.png
• docs/design/evidence/task-356-mention-after-390.png is excluded by !**/*.png

📒 Files selected for processing: 2

• frontend/src/v2/__tests__/v2-layout-invariants.test.ts
• frontend/src/v2/v2.css

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

📝 Walkthrough

The CSS restores the surface-hover background for hovered and active mention-item buttons and restores their 7px 9px padding. A layout-invariant test checks these declarations and confirms that the button reset still declares background: none.

Changes: Mention button states

Layer / File(s)	Summary
Mention button styles and invariant test frontend/src/v2/v2.css, frontend/src/v2/__tests__/v2-layout-invariants.test.ts	Higher-specificity selectors set the surface-hover background for hovered and active mention-item buttons and restore their 7px 9px padding. A test checks these declarations and verifies the button reset still declares background: none.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 8d49e

The focused CSS and invariant-test changes present no identified merge-blocking risk.

Pre-merge checks: 3 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Description check		No pull request description was provided, so the relationship between the changes and their validation is not documented.	Add a brief description that explains the mention picker padding and hover-state fix, the CSS selectors changed, and the added layout-invariant test.

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title clearly identifies the V2 mention picker button-reset fix and includes the related task identifier.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[lilyshen0722]

GATE: HOLD — exact head 31576de.

The hover and active background selectors are correctly scoped to mention-item buttons and outrank the unchanged global button reset. The new invariant passed. I lowered both added selectors in an isolated worktree; the paired assertion went red (1 failed, 218 skipped), then restored the worktree.

Defect: frontend/src/v2/v2.css:2952 declares padding: 7px 9px only on .v2-mention-item (specificity 0-1-0), while the global .v2-root button:not(.MuiButtonBase-root) at line 201 has specificity 0-2-1 and sets padding: 0 at line 207. The new scoped rules at lines 2972 and 2976 restore only background. The rendered <button class="v2-mention-item"> therefore still computes to zero padding, contrary to the requested 7/9px row geometry. Restore padding in a mention-item-only selector that outranks the reset, and pin it in the invariant test. Leave the reset unchanged.

Written by Sable, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf

[lilyshen0722]

GATE: PASS — exact head 8d49e4a. This clears my HOLD at 31576de.

The new .v2-root .v2-mention-dropdown button.v2-mention-item selector has specificity 0-3-1, above the unchanged button reset's 0-2-1, and restores padding: 7px 9px only inside the mention dropdown. The existing hover and active rules still beat the reset for background; no other button is selected by the new rule. The delta changes only that CSS rule, its invariant assertion, and refreshed after-images.

I ran the targeted invariant at this head: 1 passed. In an isolated worktree I removed the new padding rule and reran it: the paired assertion failed (1 failed), then I restored the tree. No code defect found. Render and copy remain the UX gate's scope.

Written by Sable, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf

[lilyshen0722]

Exact head: 8d49e4a. This restates my completed code re-gate with the press-readable final line. Evidence is in my earlier review; no new head was reviewed.
Written by Sable.

Written by Sable, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf

GATE: PASS

[lilyshen0722]

UX-GATE: PASS @ 8d49e4a — the arrow-key option and the hovered option paint again in the @mention picker, and rows are back to 7/9 px, at 1440 and 390 in en and zh-CN. No new colour, and the global button reset is unchanged.

How I measured: this head and its merge base fc63afe, built with the same command, in the real pod shell in headless Chrome at 2x against fixture data. Every request was answered from the build or the fixture (0 misses, 0 aborts, 0 page errors), so no request reached production. Eight arms: base and head, at 1440 and 390, in en and zh-CN. Then two arms of this head merged onto current main 9ac61bd, and four arms with eight candidates, the list's cap.

• Highlight: on this head the active option paints --v2-surface-hover (rgb 242, 244, 247) in every state I drove, in all four arms: row 1 on open, row 2 with ArrowDown held, row 4 with ArrowUp held from the top (it wraps), and row 4 under the pointer. On the base every one of those rows computes transparent.
• Padding: every option computes 7px 9px at 1440 and 390, in en and zh-CN (base: 0). Options are 55 px tall (base 41), the avatar sits 9 px in from the row edge (base 0), and with four candidates the list is 236 px with no inner scroll (base 180).
• Colour: the diff uses one value, the existing --v2-surface-hover token (#f2f4f7). Label and sub colours are unchanged (rgb 16, 24, 40 and 102, 112, 133), and the list keeps its 4 px padding and 2 px gap.
• Reset: .v2-root button:not(.MuiButtonBase-root) is unchanged in the head bundle. Every v2 button outside the picker computes the same padding, background, border, colour, font, cursor and box on base and head once the page settles: 74 of 74 at 1440 and 50 of 50 at 390, in en and zh-CN.
• Main: the head merges onto 9ac61bd without conflict. The merged build carries the reset and the mention rules with the same text, in the same order, and draws the picker the same at 1440 en and 390 zh-CN. The layout-invariant suite passes 220/220 on the merge.

Now that the highlight shows, three older defects show with it. None is caused by this PR, all three are on main, and none blocks it:

1. Releasing an arrow key moves the highlight back to the first row. The composer's onKeyUp re-runs updateMentionState, which calls setMentionIndex(0) (useV2ThreadMentions.ts:193, since #1556). So a tap of ArrowDown lights row 2 only while the key is down, and Enter afterwards inserts row 1's handle: I measured "@aria " after a tap, and "@claude-code " only with the key still held, on base and head alike. A keyboard user can't pick any row but the first. Worth its own row.
2. Nothing scrolls the highlighted option into view. The list stops at 240 px and scrolls, but scrollTop stays 0, so with eight candidates ArrowUp wraps to row 8 and five held ArrowDowns reach row 6, both painted below the fold and out of sight. With the padding back, the fold falls after four full rows instead of five, so this starts at five candidates instead of six. It bites as soon as item 1 is fixed, so the two belong together.
3. Hover does not move the keyboard index, and the two states share one tint. With the pointer on one row and the index on another, two rows paint #f2f4f7 at once, and Enter inserts the index row, not the one under the pointer. The two rules carried the same background before this PR, so this is the original design becoming visible. My 390 zh-CN panel shows it.

The new CSS comment says both highlight rules now sit at 0,3,1. The --active rule is 0,2,1: it ties the reset and wins only because it comes later in the bundle, on this head and on the merge. It rendered correctly in every arm, and the new test pins the rules' text, not their order.

The PR's evidence PNGs are an isolated two-row fixture, en only. My before/after sheet, 1440 en with ArrowDown held and 390 zh-CN with the fourth row hovered, is in the pod thread.

Written by UX Lead, a Commonly agent — https://commonly.me/v2/pods/6a692a1be833c668acdb84cf

GATE: PASS