Add an in-place screenshot markup tool to the browser pane#6335
Open
1shiharat wants to merge 8 commits into
Open
Add an in-place screenshot markup tool to the browser pane#63351shiharat wants to merge 8 commits into
1shiharat wants to merge 8 commits into
Conversation
Adds a Draw button to the browser toolbar (local and remote/SSH panes) that freezes the visible viewport into a still image and overlays a drawing canvas. Users can mark it up with pen, highlighter, arrow, rectangle, ellipse, and text (color, thickness, undo/redo), then copy the composited PNG to the clipboard to paste into their agent — reusing the existing clipboard screenshot path, so it works for local and remote agents alike. The drawing layer is a renderer-side canvas, so the base image is the only environment-specific piece: local panes use webview.capturePage(), remote panes snapshot the already-displayed screencast frame. Drawing model, compositing, and shape rendering are isolated, unit-tested modules.
- Copy now always produces a PNG (the clipboard handler accepts PNG only; a JPEG fallback silently produced an empty clipboard) and targets the clipboard size limit so realistic viewports stay full-resolution. - Flatten the captured base image onto opaque white so transparent page backgrounds no longer ghost the live view through the frozen backdrop. - Keep markup text-input keystrokes local (focus the field explicitly and stop propagation) so the browser pane's global handlers can't swallow them. - Make the Draw toolbar button toggle markup mode off on a second click.
- Add a font-size control (text tool) to the markup toolbar. - Add a Select tool to re-edit committed shapes: click to select, drag to move, change color/width/font-size of the selection, Delete to remove, and double-click text to re-edit its content. Undo/redo now snapshots the whole shape list so every edit (not just adding) is reversible. - Fix the text input: transparent ink-colored field (no dark theme box over the screenshot) and ignore Enter during IME composition so Japanese conversion no longer commits the annotation early. - Split the grown overlay logic into focused modules (editor hook, pointer + keyboard hooks, canvas renderer, document edits, hit-testing) to stay within the file-size budget.
- Clicking elsewhere while a text box is open now commits the text and switches to the Select tool, instead of discarding it (the input unmounted before its blur fired) and opening a new box at the click. - Move the drawing toolbar to the bottom (stacked above the actions bar) and open the color/size popover upward.
- Hide the text shape being re-edited from the canvas so the live input isn't doubled by its committed render. - Drop the input's padding/border and use the same font as the canvas so the typing preview sits exactly where the text renders (no shift); field-sizing keeps the box hugging the text. - Estimate text width with full-width (CJK) awareness so the selection box and hit bounds track Japanese text instead of clipping it. - Move the font-size control out of the color popover into its own toolbar button showing the current size.
Draw the selection box on the canvas even while a text shape is being re-edited (the shape stays hidden so it isn't doubled), and strip the box off the text input so it renders only the editable glyphs. The frame is now identical whether selecting or editing, and the text overlays the same position with the same font, so there's no shift between the two.
Force the text input's line-height to 1 and zero its height/padding so its top edge matches the canvas textBaseline:'top' render — the editing text no longer sits slightly lower than the committed/selected text.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughBrowserPane now supports markup drawing in both remote and local panes. The change adds shared markup components, a markup editor hook, screenshot capture and composition utilities, hit-testing and rendering helpers, and new locale strings for the markup UI. It also wires the new flow into the browser pane toolbar and pane content for both remote and local views. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/renderer/src/components/browser-pane/markup/markup-hit-test.test.ts (1)
16-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a highlight hit-test case matching rendered width.
Please add a test asserting highlight selection uses the widened highlight stroke semantics, so this path is regression-protected.
src/renderer/src/components/browser-pane/markup/markup-canvas-render.ts (1)
27-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a shared UI token for the selection outline.
Line 30 hard-codes
#3b82f6, which makes the canvas overlay bypass the renderer theme/token system. Please resolve this from an existing token/CSS variable instead of introducing a new color literal here.As per coding guidelines, “Never invent new color values...” and “All UI work ... must follow
docs/STYLEGUIDE.mdand use tokens defined insrc/renderer/src/assets/main.css.”Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0f19723d-9ec7-4b89-8a0f-8881681c24aa
📒 Files selected for processing (25)
src/renderer/src/components/browser-pane/BrowserPane.tsxsrc/renderer/src/components/browser-pane/markup/MarkupDrawButton.tsxsrc/renderer/src/components/browser-pane/markup/MarkupOverlay.tsxsrc/renderer/src/components/browser-pane/markup/MarkupToolbar.tsxsrc/renderer/src/components/browser-pane/markup/markup-base-image.tssrc/renderer/src/components/browser-pane/markup/markup-canvas-render.tssrc/renderer/src/components/browser-pane/markup/markup-clipboard-delivery.tssrc/renderer/src/components/browser-pane/markup/markup-drawing-model.test.tssrc/renderer/src/components/browser-pane/markup/markup-drawing-model.tssrc/renderer/src/components/browser-pane/markup/markup-editor-document.tssrc/renderer/src/components/browser-pane/markup/markup-hit-test.test.tssrc/renderer/src/components/browser-pane/markup/markup-hit-test.tssrc/renderer/src/components/browser-pane/markup/markup-screenshot-compose.test.tssrc/renderer/src/components/browser-pane/markup/markup-screenshot-compose.tssrc/renderer/src/components/browser-pane/markup/markup-shape-render.test.tssrc/renderer/src/components/browser-pane/markup/markup-shape-render.tssrc/renderer/src/components/browser-pane/markup/useMarkupEditor.tssrc/renderer/src/components/browser-pane/markup/useMarkupKeyboardShortcuts.tssrc/renderer/src/components/browser-pane/markup/useMarkupMode.tssrc/renderer/src/components/browser-pane/markup/useMarkupPointerHandlers.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.json
- Hit-test highlights against their rendered (4x) thickness so the select tool doesn't miss large highlight strokes. - Reset transient editor state (pending text, selection, in-progress drag) on Clear all so a pending input blur can't re-add text and no stale selection lingers over the emptied canvas. - Guard markup completion with the capture token so a stale onDeliver can't reset or error a session the user already cancelled or restarted. - Skip the grab-element keyboard shortcut while the markup overlay is open, matching the already-disabled grab toolbar buttons. - Localize the markup error messages and surface them via a toast (the controller's error was otherwise never shown).
Contributor
✅ Action performedReview finished.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this adds
A screenshot-markup tool for the in-app browser pane. A new Draw button freezes the visible page into a still image and overlays a drawing surface for pen, highlighter, arrow, rectangle, ellipse, and text (color, thickness, font size, undo/redo, plus a select tool to move/restyle/re-edit shapes). Copy markup then puts the annotated PNG on the clipboard to paste into your agent.
It's fully opt-in and additive — nothing changes unless you click the new button, and no existing flow is touched.
Why
When steering an agent against a live page, "make this look like X" is far faster to express by drawing on a screenshot than by typing it out. This turns the browser pane's existing "grab element → feedback" idea into a freehand markup pass, delivered through the same clipboard path the terminal already uses for pasted screenshots — so it works for local and remote/SSH agents alike.
A note on scope (sorry for the surprise)
This is a sizable, unsolicited contribution and I didn't open an issue first — apologies for dropping it without a heads-up. It's deliberately self-contained (all renderer-side, new modules under
browser-pane/markup/, one small wiring change inBrowserPane.tsx) and opt-in, so it should be low-risk to evaluate or set aside. I'm very happy to split it into smaller PRs, rework the UX, or close it if it doesn't fit the roadmap — just say the word.How it works
<canvas>overlay — it never touches the page. Only the base image is environment-specific: local panes usewebview.capturePage(), remote panes snapshot the displayed screencast<img>. Compose + delivery are shared.clipboard:writeImage; pasting into the agent reuses Orca's clipboard-screenshot flow (which writes the temp file on the correct host for local or remote agents).Testing
pnpm lint,pnpm typecheck, andpnpm testpass locally.Compatibility / review notes
Cmd/Ctrl+Z) is platform-guarded vianavigator.userAgent.clipboard:writeImagehandler.Screenshots
X: @1shiharat