spec: toolbelt buttons on collapsed conversation blocklist items (#9810)#10227
spec: toolbelt buttons on collapsed conversation blocklist items (#9810)#10227lonexreb wants to merge 3 commits intowarpdotdev:masterfrom
Conversation
There was a problem hiding this comment.
Overview
This spec proposes a collapsed-row toolbelt for conversation blocklist items, including Fork and overflow link-copy actions.
Concerns
- The new spec file does not follow the repo's
PRODUCT.md/TECH.mdspec layout, which can make the approved behavior invisible to the implementation/spec-context workflow. - The keyboard behavior is underspecified and internally unclear for the new Fork and kebab controls.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| @@ -0,0 +1,74 @@ | |||
| # Spec: Toolbelt buttons on collapsed conversation blocklist items (GH-9810) | |||
There was a problem hiding this comment.
PRODUCT.md and, when needed, TECH.md under the ticket directory; adding SPEC.md is likely to be missed by implementation/spec-context tooling. Rename this to the appropriate conventional spec file(s) or state why this exception is intentional.
| - B6. Keyboard: Tab/Shift-Tab moves focus between rows; pressing | ||
| Enter on the kebab opens it; arrow keys navigate within. |
There was a problem hiding this comment.
Co-Authored-By: Warp <agent@warp.dev>
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec adds an inline toolbelt for collapsed conversation rows with Fork, overflow link-copy actions, right-click menu access, modal hiding, and keyboard behavior.
Concerns
- The link-copy menu creates a new direct surface for conversation/share/debug URLs, but the behavior contract does not explicitly require modal-equivalent privacy, authorization, and redaction semantics.
- Acceptance criteria and tests cover the conversation-link action but do not verify share/debug link activation or their copied values.
- The roving-focus model keeps the inline controls out of the Tab order but does not define the accessibility contract needed for implementation and testing.
Security
- The debug/share link copy surface needs explicit reuse of existing modal access and disclosure controls before implementation.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| - B3. Kebab overflow menu (⋯) contains: | ||
| - "Copy conversation link" (always present). | ||
| - "Copy share link" (present only when the conversation has a | ||
| share URL — same condition the modal uses). | ||
| - "Copy debug link" (present only in dev / debug-mode builds OR | ||
| when the user has the existing developer-tools setting on). |
There was a problem hiding this comment.
| - A3. Click kebab → "Copy conversation link" copies the same URL | ||
| the modal-side action would. |
There was a problem hiding this comment.
| - B6. Keyboard focus uses row-roving navigation. Tab/Shift-Tab | ||
| moves focus between collapsed conversation rows as it does today; | ||
| the inline Fork button and kebab are not separate Tab stops. | ||
| When a collapsed row is focused, Left/Right arrow moves the | ||
| active in-row control between the row body, Fork button, and | ||
| kebab. Enter or Space activates the active in-row control: |
There was a problem hiding this comment.
|
Round-1 fixes for oz-for-oss review: (1) restructured to canonical PRODUCT.md/TECH.md layout (added TECH.md for technical detail); (2) defined precise keyboard model (row-level Tab stop, arrow roving between Fork/kebab, Enter to activate, Esc to escape); (3) added Link Copy Privacy Contract requiring share/debug link copy actions to reuse the modal's auth/redaction/audit pathways; (4) added acceptance items for share/debug link parity with modal and revoked-permission behavior; (5) added Accessibility Contract with explicit ARIA roles and labels; (6) added tests for keyboard roving, share/debug parity, and a11y audit. |
|
/oz-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This spec PR defines the product and technical behavior for adding a collapsed-row conversation toolbelt with Fork, overflow link-copy actions, right-click access, keyboard navigation, accessibility requirements, and parity with the modal-side authorization/redaction/audit pathways.
Concerns
- Clarify the focus implementation so the single Tab-stop model remains unambiguous across
aria-activedescendantand roving-tabindex approaches. - Add a right-click regression expectation that opening the overflow menu does not also expand/open the row.
Verdict
Found: 0 critical, 0 important, 2 suggestions
Approve with nits
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| - Roving focus is implemented via `tabindex="-1"` on the inline | ||
| buttons + `tabindex="0"` on the row container, with a JS-managed | ||
| `aria-activedescendant` (or roving `tabindex` swap) per existing | ||
| patterns in the codebase. |
There was a problem hiding this comment.
💡 [SUGGESTION] Clarify which focus pattern is required here: aria-activedescendant keeps the row as the focused element, while a roving tabindex swap may move DOM focus onto Fork/kebab. The spec should say how either option preserves the single row Tab stop.
| - T3. Right-click outside the toolbelt opens the kebab menu at the | ||
| cursor. |
There was a problem hiding this comment.
💡 [SUGGESTION] Add an assertion that the contextmenu event does not also trigger the row's open/expand action; otherwise a right-click implementation could open the menu and modal together.
Spec for #9810. Inline toolbelt on collapsed conversation rows: primary Fork button + kebab overflow (copy conversation/share/debug link). Right-click on the row also opens the kebab menu (per @david). Modal-open hides the inline toolbelt to avoid duplication.
Closes (spec-only) #9810.