Skip to content

spec: toolbelt buttons on collapsed conversation blocklist items (#9810)#10227

Open
lonexreb wants to merge 3 commits intowarpdotdev:masterfrom
lonexreb:spec/9810-collapsed-conversation-toolbelt
Open

spec: toolbelt buttons on collapsed conversation blocklist items (#9810)#10227
lonexreb wants to merge 3 commits intowarpdotdev:masterfrom
lonexreb:spec/9810-collapsed-conversation-toolbelt

Conversation

@lonexreb
Copy link
Copy Markdown
Contributor

@lonexreb lonexreb commented May 6, 2026

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.

@cla-bot cla-bot Bot added the cla-signed label May 6, 2026
@github-actions github-actions Bot added the external-contributor Indicates that a PR has been opened by someone outside the Warp team. label May 6, 2026
@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 6, 2026

@lonexreb

I'm starting a first review of this spec-only pull request.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

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.md spec 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

Comment thread specs/GH9810/PRODUCT.md Outdated
@@ -0,0 +1,74 @@
# Spec: Toolbelt buttons on collapsed conversation blocklist items (GH-9810)
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.

⚠️ [IMPORTANT] This repo's spec workflow expects approved specs as 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.

Comment thread specs/GH9810/SPEC.md Outdated
Comment on lines +37 to +38
- B6. Keyboard: Tab/Shift-Tab moves focus between rows; pressing
Enter on the kebab opens it; arrow keys navigate within.
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.

⚠️ [IMPORTANT] The keyboard contract says Tab/Shift-Tab moves focus between rows but does not state how the new Fork button and kebab control receive focus/activation; specify whether controls are tabbable or row-roved, plus Enter/Space behavior, so the collapsed toolbelt is keyboard-accessible.

Co-Authored-By: Warp <agent@warp.dev>
@lonexreb
Copy link
Copy Markdown
Contributor Author

lonexreb commented May 7, 2026

/oz-review

@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 7, 2026

@lonexreb

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 /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

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

Comment thread specs/GH9810/PRODUCT.md Outdated
Comment on lines +25 to +30
- 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).
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.

⚠️ [IMPORTANT] [SECURITY] This adds a new surface for copying conversation/share/debug URLs, but the behavior contract does not require these inline actions to reuse the modal's privacy, authorization, and redaction gates. Add that requirement so restricted conversations or diagnostic links cannot be exposed through this menu.

Comment thread specs/GH9810/PRODUCT.md
Comment on lines +58 to +59
- A3. Click kebab → "Copy conversation link" copies the same URL
the modal-side action would.
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.

⚠️ [IMPORTANT] A3 only verifies Copy conversation link, but B3 also introduces Copy share link and Copy debug link. Add acceptance criteria for their conditional availability and that activation copies the same modal-equivalent URLs.

Comment thread specs/GH9810/PRODUCT.md Outdated
Comment on lines +37 to +42
- 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:
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.

⚠️ [IMPORTANT] These controls are intentionally not Tab stops, but the spec does not define accessible names/roles, focus-visible state, or screen-reader announcement for the active in-row control. Add those requirements so the roving-focus behavior is implementable and testable.

@lonexreb
Copy link
Copy Markdown
Contributor Author

lonexreb commented May 8, 2026

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.

@lonexreb
Copy link
Copy Markdown
Contributor Author

lonexreb commented May 8, 2026

/oz-review

@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 8, 2026

@lonexreb

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 /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

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-activedescendant and 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

Comment thread specs/GH9810/TECH.md
Comment on lines +53 to +56
- 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.
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.

💡 [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.

Comment thread specs/GH9810/TECH.md
Comment on lines +132 to +133
- T3. Right-click outside the toolbelt opens the kebab menu at the
cursor.
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.

💡 [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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed external-contributor Indicates that a PR has been opened by someone outside the Warp team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant