Skip to content

fix(ui): drop the ⌘↵ hint from the comment popover and the composer's Looks good on HTML - #1693

Merged
backnotprop merged 1 commit into
mainfrom
fix/comment-popover-trim
Oct 5, 2026
Merged

backnotprop merged 1 commit into
mainfrom
fix/comment-popover-trim

Conversation

@backnotprop

Copy link
Copy Markdown
Owner

Why

Owner feedback (verbatim):

the cmd+enter on comments popover seems unnecessary. Similarly, the looks good button being added on the HTML comment popover doesn't make sense. It doesn't exist elsewhere. Why are we showing up for HTML? If you want to add a looks good comment, you can just select text and click looks good.

Provenance:

  • The composer's one-click "Looks good" came from 0ae40e7 (2026-08-24, "feat(annotate): restore a restricted thumbs-up on comment-only HTML surfaces"), published in @plannotator/ui 0.32.0.
  • The {submitHint} text (renders ⌘↵ / Ctrl+Enter) in the comment popover footer dates from feat(ai): AI backbone + inline chat for code review #363.

What changes

  1. CommentPopover: removes the submit hint text from both footers (popover and dialog). The Mod+Enter keyboard behavior is unchanged. Other submitHint users (AITab, AskAIInput, DocumentAIChatPanel, DecisionControl, ReviewSidebar) are untouched.
  2. HTML/live composer "Looks good" removed: the onQuickLookGood prop and its footer button on CommentPopover, handleCommentLooksGood on useHtmlAnnotation, and the HtmlViewer wiring.
    Kept: the selection toolbar's 👍 "Looks good" on HTML surfaces (the AnnotationToolbar commentOnly + onQuickLabel path from the same commit, filtered to THUMBS_UP_LABEL). That is the path the owner points to. The trust-boundary clamp is untouched.
  3. Tests: deleted CommentPopover.quickLookGood.test.tsx and removed it from the DOM_TESTS list in .github/workflows/test.yml; dropped the composer half of the maxAdditionalTargets cap test (the typed-comment half still pins the cap); the minted-ids test now mints through handleCommentSubmit.
  4. @plannotator/ui consumers: packages/ui/HANDOFF.md has a new "Comment composer trim" section recording the removed public prop (onQuickLookGood) and hook member (handleCommentLooksGood). No version bump.
  5. Docs: the AGENTS.md comment-only paragraph described the toolbar as "passed no quick-label handler", which had been stale since 0ae40e7. It now describes the toolbar's 👍 and states that the composer has none.

Verification

  • bun run typecheck: clean.
  • DOM_TESTS=1 bun test packages/ui packages/editor: 2389 pass, 4 fail. The 4 failures are the DocBadges open-in placement tests, which also fail on main.
  • bun test scripts/dom-test-allowlist.test.ts: pass.
  • bun run --cwd apps/review build && bun run build:hook: OK.
  • Headless Chromium (--use-mock-keychain --password-store=basic, temp PLANNOTATOR_DATA_DIR) on the built app:
    • Markdown annotate: the comment popover footer has no ⌘↵ / Ctrl+Enter text. With text typed, Meta+Enter closed the popover and saved the annotation, which appears in the panel.
    • HTML annotate: a pinpoint click on a button opens the composer with buttons Expand, Close, Remove this target, Save, Ask AI, Attachments. There is no Looks good button and no hint text.
    • HTML annotate: drag-selecting text in the page still shows the selection toolbar Copy, Comment, Looks good, Cancel (👍 present).

… Looks good on HTML

Owner feedback: the visible Cmd+Enter hint in the comment popover footer is
unnecessary, and the one-click "Looks good" button on the HTML/live-app
pinpoint composer exists nowhere else. To leave a thumbs-up on those
surfaces, select text and use the selection toolbar's 👍.

- CommentPopover: remove the submitHint text from both footers (Mod+Enter
  still saves) and the onQuickLookGood prop + footer button.
- useHtmlAnnotation / HtmlViewer: remove handleCommentLooksGood and its
  wiring. The selection toolbar's commentOnly 👍 and the trust-boundary
  clamp are unchanged.
- Tests: delete CommentPopover.quickLookGood.test.tsx (and its DOM_TESTS
  entry); drop the composer half of the host-cap test; mint through
  handleCommentSubmit in the minted-ids test.
- Docs: HANDOFF.md records the removed public prop; AGENTS.md describes the
  toolbar 👍 and the composer without one.
@backnotprop
backnotprop merged commit fa50842 into main Oct 5, 2026
28 checks passed
backnotprop added a commit that referenced this pull request Oct 5, 2026
…poser (#1712)

* feat(ui): bring the one-click thumbs-up back to the HTML pinpoint composer

A pinpoint click on an HTML / live-app element opens the comment composer
directly and never shows the selection toolbar, so after #1693 removed the
composer's "Looks good" an element could not get a one-click thumbs-up.

- CommentPopover: onQuickLookGood is back with its old contract (optional,
  absent renders nothing, disabled once anything is typed or attached), now
  as an emoji-only 👍 button beside Save: aria-label and tooltip "Looks good",
  the toolbar 👍's green hover tint, a focus-visible ring. No key binding;
  Mod+Enter still saves the typed comment.
- useHtmlAnnotation: handleCommentLooksGood is back, sharing one commit path
  with handleCommentSubmit (commitComposerDraft), so the 👍 is the toolbar's
  THUMBS_UP_LABEL quick-label COMMENT on the pinned element with its anchor,
  element context and shift-click extra targets. HtmlViewer passes it again.
- The removal never shipped in a ui release, so hosts see no API change;
  HANDOFF.md is rewritten to say so. AGENTS.md describes the composer 👍.
- Tests: the composer 👍 labels the pinned element (anchor, context, closes);
  Mod+Enter still saves a typed comment and an empty Mod+Enter creates
  nothing; the 👍 respects the host target cap; an absent prop renders none.

* fix(ui): keep the guides.show viewer bundle unchanged; restore focus after the 👍

- The 👍 button's disabled grey-out is now an inline filter instead of a
  Tailwind grayscale utility. That utility was the only class new to the
  pinned guides.show viewer CSS (CommentPopover.tsx is scanned into it), so
  the viewer bundle and guide-viewer-manifest stay byte-identical to main:
  build:viewer + check:manifest report in sync, no guides.show deploy.
- A 👍 click now goes through the same tail as Save: it clears the draftKey
  draft and calls restoreOpeningFocus. New DOM test: focus returns to the
  opener (fails without the restore).
- HtmlViewer comment notes the button also shows when the composer opens
  from a text selection (a harmless duplicate of the toolbar 👍).
- HANDOFF.md notes the focus restore and the viewer-CSS constraint.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant