Skip to content

Commit 0ae40e7

Browse files
backnotpropbacknotprop
committed
feat(annotate): restore a restricted thumbs-up on comment-only HTML surfaces
The v0.27.5 comment-only ruling removed every label affordance from HTML and live-app annotate surfaces, leaving no one-click positive feedback: the only path was opening the composer and typing prose. Restore exactly ONE affordance, the hardcoded 'Looks good' thumbs-up, on both input routes: - selection toolbar: commentOnly + a provided onQuickLabel now renders only the thumbs-up (no Delete, no Zap picker, Alt+digit suppressed); HtmlViewer passes a handler that filters by label id as defense in depth - pinpoint: the composer gains an optional one-click 'Looks good' footer action (disabled once anything is typed, so it can never discard a draft), emitting the same isQuickLabel comment shape with the draft's multi-select targets The trust-boundary clamp is untouched: redline/quickLabel modes stay collapsed to selection, so a hostile page still cannot force a DELETION or an arbitrary label. THUMBS_UP_LABEL moves to utils/quickLabels as the canonical definition.
1 parent 1080436 commit 0ae40e7

9 files changed

Lines changed: 264 additions & 32 deletions

File tree

‎.github/workflows/test.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ jobs:
148148
packages/editor/App.archiveReadOnly.test.tsx
149149
packages/editor/App.htmlChrome.test.tsx
150150
packages/ui/components/AnnotationToolbar.commentOnly.test.tsx
151+
packages/ui/components/CommentPopover.quickLookGood.test.tsx
151152
packages/editor/actionsLabelMode.test.ts
152153
153154
opencode-v2:

‎packages/ui/components/AnnotationToolbar.commentOnly.test.tsx‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,25 @@ async function mount(props: { commentOnly?: boolean; withQuickLabels?: boolean }
5252
}
5353

5454
describe.if(hasDom)('AnnotationToolbar commentOnly seam', () => {
55-
test('commentOnly hides Delete and offers commenting only', async () => {
55+
test('commentOnly without a label handler hides Delete and every label affordance', async () => {
5656
const titles = await mount({ commentOnly: true });
5757
expect(titles).toContain('Comment');
5858
expect(titles).not.toContain('Delete');
5959
expect(titles).not.toContain('Quick label');
6060
expect(titles).not.toContain('Looks good');
6161
});
6262

63+
test('commentOnly with a label handler restores ONLY the thumbs-up (no Delete, no picker)', async () => {
64+
// The restricted restore: HTML/live surfaces get exactly one label
65+
// affordance back. A regression that also renders the Zap picker (or
66+
// Delete) re-opens the comment-only ruling this seam enforces.
67+
const titles = await mount({ commentOnly: true, withQuickLabels: true });
68+
expect(titles).toContain('Comment');
69+
expect(titles).toContain('Looks good');
70+
expect(titles).not.toContain('Delete');
71+
expect(titles).not.toContain('Quick label');
72+
});
73+
6374
test('the default (markdown surface) toolbar keeps Delete and quick labels', async () => {
6475
const titles = await mount({ withQuickLabels: true });
6576
expect(titles).toContain('Delete');

‎packages/ui/components/AnnotationToolbar.tsx‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,13 @@ import React, { useState, useEffect, useRef, useMemo } from "react";
22
import { AnnotationType } from "../types";
33
import { createPortal } from "react-dom";
44
import { useDismissOnOutsideAndEscape } from "../hooks/useDismissOnOutsideAndEscape";
5-
import { type QuickLabel, getQuickLabels } from "../utils/quickLabels";
5+
import { type QuickLabel, getQuickLabels, THUMBS_UP_LABEL } from "../utils/quickLabels";
66
import { copyTextToClipboard } from "../utils/clipboard";
77
import { acquireTypeToCommentCapture } from "../shortcuts/plan-review/annotationMode.shortcuts";
88
import { FloatingQuickLabelPicker } from "./FloatingQuickLabelPicker";
99

1010
type PositionMode = 'center-above' | 'top-right';
1111

12-
const THUMBS_UP_LABEL: QuickLabel = {
13-
id: 'thumbs-up',
14-
emoji: '👍',
15-
text: 'Looks good',
16-
color: 'green',
17-
};
18-
1912
const isEditableElement = (node: EventTarget | Element | null): boolean => {
2013
if (!(node instanceof Element)) return false;
2114
if (node.matches('input, textarea, select, [role="textbox"]')) return true;
@@ -34,9 +27,11 @@ interface AnnotationToolbarProps {
3427
onQuickLabel?: (label: QuickLabel) => void;
3528
/** Text to copy when the button is clicked */
3629
copyText?: string;
37-
/** Comment-only surfaces (HTML / live-app viewer): hide the Delete action.
38-
* Markdown surfaces keep the full toolbar. Quick labels are already gated
39-
* by the presence of onQuickLabel. */
30+
/** Comment-only surfaces (HTML / live-app viewer): hide the Delete action,
31+
* the quick-label picker, and the Alt+digit label shortcuts. A provided
32+
* onQuickLabel then renders ONLY the hardcoded 👍 "Looks good" button —
33+
* the one label affordance restored to these surfaces. Markdown surfaces
34+
* keep the full toolbar. */
4035
commentOnly?: boolean;
4136
/** Hide the copy button (set when a keyboard copy handler exists) */
4237
hideCopyButton?: boolean;
@@ -132,14 +127,17 @@ export const AnnotationToolbar: React.FC<AnnotationToolbarProps> = ({
132127
return;
133128
}
134129

135-
// Alt+N applies quick label (picker closed)
130+
// Alt+N applies quick label (picker closed). Comment-only surfaces
131+
// suppress this path: their only label affordance is the 👍 button.
136132
const isDigit = (e.code >= 'Digit1' && e.code <= 'Digit9') || e.code === 'Digit0';
137133
if (isDigit && !e.ctrlKey && !e.metaKey && e.altKey) {
138134
e.preventDefault();
139-
const digit = parseInt(e.code.slice(5), 10);
140-
const index = digit === 0 ? 9 : digit - 1;
141-
if (index < quickLabels.length) {
142-
onQuickLabel?.(quickLabels[index]);
135+
if (!commentOnly) {
136+
const digit = parseInt(e.code.slice(5), 10);
137+
const index = digit === 0 ? 9 : digit - 1;
138+
if (index < quickLabels.length) {
139+
onQuickLabel?.(quickLabels[index]);
140+
}
143141
}
144142
return;
145143
}
@@ -160,7 +158,7 @@ export const AnnotationToolbar: React.FC<AnnotationToolbarProps> = ({
160158
window.removeEventListener("keydown", handleKeyDown);
161159
releaseCapture();
162160
};
163-
}, [onClose, onRequestComment, onQuickLabel, quickLabels, showQuickLabels]);
161+
}, [onClose, onRequestComment, onQuickLabel, quickLabels, showQuickLabels, commentOnly]);
164162

165163
useDismissOnOutsideAndEscape({
166164
enabled: !showQuickLabels,
@@ -238,20 +236,22 @@ export const AnnotationToolbar: React.FC<AnnotationToolbarProps> = ({
238236
/>
239237
{onQuickLabel && (
240238
<>
241-
<ToolbarButton
242-
ref={zapButtonRef}
243-
onClick={() => setShowQuickLabels(prev => !prev)}
244-
icon={<ZapIcon />}
245-
label="Quick label"
246-
className={showQuickLabels ? "text-amber-500 bg-amber-500/10" : "text-amber-500 hover:bg-amber-500/10"}
247-
/>
239+
{!commentOnly && (
240+
<ToolbarButton
241+
ref={zapButtonRef}
242+
onClick={() => setShowQuickLabels(prev => !prev)}
243+
icon={<ZapIcon />}
244+
label="Quick label"
245+
className={showQuickLabels ? "text-amber-500 bg-amber-500/10" : "text-amber-500 hover:bg-amber-500/10"}
246+
/>
247+
)}
248248
<ToolbarButton
249249
onClick={() => onQuickLabel(THUMBS_UP_LABEL)}
250250
icon={<span className="block w-4 h-4 text-sm leading-4 text-center">👍</span>}
251251
label="Looks good"
252252
className="hover:bg-green-500/10"
253253
/>
254-
{showQuickLabels && zapButtonRef.current && (
254+
{!commentOnly && showQuickLabels && zapButtonRef.current && (
255255
<FloatingQuickLabelPicker
256256
anchorEl={zapButtonRef.current}
257257
onSelect={(label) => {

‎packages/ui/components/CommentPopover.quickLookGood.test.tsx‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/**
2+
* Composer one-click "Looks good" (DOM_TESTS=1)
3+
*
4+
* The restored thumbs-up for comment-only surfaces: pinpoint clicks open the
5+
* composer directly (never the selection toolbar), so the composer carries a
6+
* footer "Looks good" action when the host passes onQuickLookGood. The button
7+
* must never discard a draft: once the user has typed, it disables and Save
8+
* becomes the path. Hosts that pass nothing render no button at all.
9+
*/
10+
11+
import { afterEach, describe, expect, test } from 'bun:test';
12+
import React, { act } from 'react';
13+
import { createRoot, type Root } from 'react-dom/client';
14+
import { CommentPopover } from './CommentPopover';
15+
16+
const hasDom = typeof document !== 'undefined';
17+
18+
let root: Root | null = null;
19+
let host: HTMLElement | null = null;
20+
21+
async function mount(props: { onQuickLookGood?: () => void }): Promise<void> {
22+
const anchor = document.createElement('p');
23+
anchor.textContent = 'pinpointed element text';
24+
document.body.appendChild(anchor);
25+
host = document.createElement('div');
26+
document.body.appendChild(host);
27+
root = createRoot(host);
28+
await act(async () =>
29+
root?.render(
30+
<CommentPopover
31+
anchorEl={anchor}
32+
contextText="pinpointed element text"
33+
isGlobal={false}
34+
onSubmit={() => {}}
35+
onClose={() => {}}
36+
onQuickLookGood={props.onQuickLookGood}
37+
/>,
38+
),
39+
);
40+
await act(async () => new Promise((resolve) => setTimeout(resolve, 0)));
41+
}
42+
43+
function looksGoodButton(): HTMLButtonElement | null {
44+
return (
45+
Array.from(document.querySelectorAll<HTMLButtonElement>('button')).find((b) =>
46+
b.textContent?.includes('Looks good'),
47+
) ?? null
48+
);
49+
}
50+
51+
function composerTextarea(): HTMLTextAreaElement {
52+
const el = document.querySelector<HTMLTextAreaElement>('textarea');
53+
if (!el) throw new Error('composer textarea did not render');
54+
return el;
55+
}
56+
57+
afterEach(async () => {
58+
if (root) await act(async () => root?.unmount());
59+
root = null;
60+
host?.remove();
61+
host = null;
62+
document.body.textContent = '';
63+
});
64+
65+
describe.if(hasDom)('CommentPopover onQuickLookGood', () => {
66+
test('renders the footer button and a click fires the host callback', async () => {
67+
let fired = 0;
68+
await mount({ onQuickLookGood: () => fired++ });
69+
const btn = looksGoodButton();
70+
expect(btn).not.toBeNull();
71+
expect(btn!.disabled).toBe(false);
72+
await act(async () => btn!.click());
73+
expect(fired).toBe(1);
74+
});
75+
76+
test('disables once the user has typed, so a click can never discard a draft', async () => {
77+
let fired = 0;
78+
await mount({ onQuickLookGood: () => fired++ });
79+
const textarea = composerTextarea();
80+
await act(async () => {
81+
const setter = Object.getOwnPropertyDescriptor(
82+
HTMLTextAreaElement.prototype,
83+
'value',
84+
)!.set!;
85+
setter.call(textarea, 'actually, one concern');
86+
textarea.dispatchEvent(new Event('input', { bubbles: true }));
87+
});
88+
const btn = looksGoodButton();
89+
expect(btn).not.toBeNull();
90+
expect(btn!.disabled).toBe(true);
91+
await act(async () => btn!.click());
92+
expect(fired).toBe(0);
93+
});
94+
95+
test('absent callback renders no button (markdown/global composers unchanged)', async () => {
96+
await mount({});
97+
expect(looksGoodButton()).toBeNull();
98+
});
99+
});

‎packages/ui/components/CommentPopover.tsx‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,14 @@ interface CommentPopoverProps {
5353
initialText?: string;
5454
/** Called on submit with comment text and optional images */
5555
onSubmit: (text: string, images?: ImageAttachment[]) => void;
56+
/**
57+
* One-click "Looks good" action (comment-only HTML/live surfaces, where
58+
* pinpoint clicks open this composer directly and never see the selection
59+
* toolbar's 👍). Renders a thumbs-up button in the footer; disabled once
60+
* the user has typed or attached anything, so a click can never discard a
61+
* draft. The parent owns annotation creation and closing.
62+
*/
63+
onQuickLookGood?: () => void;
5664
/** Optional live draft observer for submit paths outside the popover. */
5765
onDraftChange?: (text: string, images?: ImageAttachment[]) => void;
5866
/** Called when popover is closed/cancelled */
@@ -148,6 +156,7 @@ export const CommentPopover: React.FC<CommentPopoverProps> = ({
148156
isGlobal,
149157
initialText = '',
150158
onSubmit,
159+
onQuickLookGood,
151160
onDraftChange,
152161
onClose,
153162
draftKey,
@@ -521,6 +530,21 @@ export const CommentPopover: React.FC<CommentPopoverProps> = ({
521530
(allowEmptySubmit && initialText.trim().length > 0);
522531
const canAskAI = !!onAskAI && !askAIDisabled && text.trim().length > 0;
523532

533+
// Shared by both footers. Disabled once anything is typed or attached so a
534+
// click can never discard a draft; with content present, Save is the path.
535+
const quickLookGoodButton = onQuickLookGood ? (
536+
<button
537+
type="button"
538+
onClick={onQuickLookGood}
539+
disabled={hasUnsavedContent}
540+
className="inline-flex items-center gap-1 px-2 py-1.5 text-xs font-medium rounded-md text-muted-foreground hover:text-foreground hover:bg-green-500/10 disabled:opacity-50 disabled:cursor-not-allowed transition-colors"
541+
title={hasUnsavedContent ? 'Clear the comment to use Looks good' : 'Add "Looks good" without typing'}
542+
>
543+
<span aria-hidden="true">👍</span>
544+
Looks good
545+
</button>
546+
) : null;
547+
524548
if (mode === 'dialog') {
525549
return createPortal(
526550
<div
@@ -632,6 +656,7 @@ export const CommentPopover: React.FC<CommentPopoverProps> = ({
632656
{!coarsePointer && (
633657
<span className="text-[10px] text-muted-foreground">{submitHint}</span>
634658
)}
659+
{quickLookGoodButton}
635660
{onAskAI && (
636661
<button
637662
onClick={handleAskAI}
@@ -781,6 +806,7 @@ export const CommentPopover: React.FC<CommentPopoverProps> = ({
781806
{!coarsePointer && (
782807
<span className="text-[10px] text-muted-foreground">{submitHint}</span>
783808
)}
809+
{quickLookGoodButton}
784810
{onAskAI && (
785811
<button
786812
onClick={handleAskAI}

‎packages/ui/components/html-viewer/HtmlViewer.tsx‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type { Annotation, EditorMode, ImageAttachment, InputMethod } from "../..
1717
import { AnnotationType } from "../../types";
1818
import { copyTextPreservingFocus } from "../../utils/clipboard";
1919
import { getIdentity } from "../../utils/identity";
20+
import { THUMBS_UP_LABEL } from "../../utils/quickLabels";
2021
import {
2122
createVimHudCommand,
2223
getVimHudPhase,
@@ -877,10 +878,15 @@ export const HtmlViewer = forwardRef<ViewerHandle, HtmlViewerProps>(
877878
positionMode="center-above"
878879
element={hook.toolbarState.element}
879880
copyText={hook.toolbarState.selectionText}
880-
// HTML/live surfaces are comment-only: no Delete, no quick
881-
// labels (onQuickLabel deliberately not passed). The markdown
882-
// surface keeps the full toolbar.
881+
// HTML/live surfaces are comment-only: no Delete, no label
882+
// picker, no Alt+digit labels (commentOnly). Exactly ONE label
883+
// affordance is restored: the hardcoded 👍 "Looks good". The
884+
// wrapper filters by id as defense in depth, so no present or
885+
// future toolbar path can emit an arbitrary label here.
883886
commentOnly
887+
onQuickLabel={(label) => {
888+
if (label.id === THUMBS_UP_LABEL.id) hook.handleQuickLabel(label);
889+
}}
884890
onAnnotate={hook.handleAnnotate}
885891
onRequestComment={hook.handleRequestComment}
886892
onClose={hook.handleToolbarClose}
@@ -898,6 +904,10 @@ export const HtmlViewer = forwardRef<ViewerHandle, HtmlViewerProps>(
898904
isGlobal={false}
899905
draftKey={`html:${hook.commentPopover.draftKey}`}
900906
onSubmit={hook.handleCommentSubmit}
907+
// Pinpoint clicks open this composer directly, so it carries
908+
// the surface's one-click "Looks good" (the global composer
909+
// does not: a document-wide thumbs-up is not a thing).
910+
onQuickLookGood={hook.handleCommentLooksGood}
901911
onClose={hook.handleCommentClose}
902912
skillReferences
903913
onAskAI={onAskAI}

‎packages/ui/components/html-viewer/htmlPinpointProtocol.test.tsx‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import React from 'react';
1313
import { act } from 'react';
1414
import { createRoot } from 'react-dom/client';
1515
import type { Annotation } from '../../types';
16+
import { AnnotationType } from '../../types';
1617

1718
const hasDom = typeof document !== 'undefined';
1819
const hookModule = hasDom ? await import('./useHtmlAnnotation') : null;
@@ -274,16 +275,39 @@ describe.if(hasDom)('pinpoint click-to-pin flow', () => {
274275
expect(document.querySelector('[data-comment-popover]')).toBeNull();
275276
});
276277

277-
test('the HTML selection toolbar is comment-only: no Delete, no quick labels', async () => {
278+
test('the HTML selection toolbar is comment-only plus the restored thumbs-up: no Delete, no label picker', async () => {
279+
// The restricted restore: exactly ONE label affordance (the hardcoded
280+
// 👍 "Looks good") came back to HTML surfaces. Delete and the Zap
281+
// picker staying gone is what preserves the comment-only ruling.
278282
const { postSelection } = await mountViewer({ mode: 'selection', onAdd: () => {} });
279283
await postSelection({ ...selectionMessage, anchor: undefined });
280284
const toolbar = document.querySelector('.annotation-toolbar');
281285
if (!toolbar) throw new Error('toolbar missing');
282286
const titles = Array.from(toolbar.querySelectorAll('button')).map((b) => b.title);
283287
expect(titles).toContain('Comment');
288+
expect(titles).toContain('Looks good');
284289
expect(titles).not.toContain('Delete');
285290
expect(titles).not.toContain('Quick label');
286-
expect(titles).not.toContain('Looks good');
291+
});
292+
293+
test('the toolbar thumbs-up creates an isQuickLabel comment annotation', async () => {
294+
// Guards the restricted handler wiring in HtmlViewer: the 👍 must
295+
// produce the same labeled-comment shape the markdown surface produces,
296+
// never a DELETION and never a bare comment.
297+
const added: Annotation[] = [];
298+
const { postSelection } = await mountViewer({ mode: 'selection', onAdd: (ann) => added.push(ann) });
299+
await postSelection({ ...selectionMessage, anchor: undefined });
300+
const toolbar = document.querySelector('.annotation-toolbar');
301+
if (!toolbar) throw new Error('toolbar missing');
302+
const thumbs = Array.from(toolbar.querySelectorAll<HTMLButtonElement>('button')).find(
303+
(b) => b.title === 'Looks good',
304+
);
305+
if (!thumbs) throw new Error('thumbs-up missing');
306+
await act(async () => thumbs.click());
307+
expect(added).toHaveLength(1);
308+
expect(added[0]?.type).toBe(AnnotationType.COMMENT);
309+
expect(added[0]?.isQuickLabel).toBe(true);
310+
expect(added[0]?.text).toBe('Looks good');
287311
});
288312

289313
test('redline mode is CLAMPED on HTML surfaces: a pinpoint selection opens the composer instead of auto-deleting', async () => {

0 commit comments

Comments
 (0)