mirror of
https://github.com/stablyai/orca.git
synced 2026-10-29 08:07:18 +00:00
Fix markup draw hint to use dismissable popover and respect surface visi
- Replace the force-open Tooltip discovery hint with a Popover (title, body, Try it/Got it actions) that can be dismissed via outside click, Escape, or window blur — Radix's tooltip couldn't handle Electron webview clicks, which never reach the renderer document. - Gate the hint on a new `surfaceActive` prop so it only opens for the visible pane, preventing a portaled layer from anchoring against a hidden/zero-size trigger at the viewport origin. - Add locale strings for the new badge/try/dismiss copy across all supported languages and cover the hook's eligibility/dismiss behavior with unit tests.
This commit is contained in:
@@ -2632,6 +2632,7 @@ function RemoteBrowserPagePane({
|
||||
onClick={() => (markup.isActive ? markup.cancel() : void markup.start())}
|
||||
disabled={!frameUrl}
|
||||
active={markup.isActive}
|
||||
surfaceActive={isActive}
|
||||
className="h-7 w-7"
|
||||
/>
|
||||
</div>
|
||||
@@ -5122,6 +5123,7 @@ function BrowserPagePane({
|
||||
onClick={() => (markup.isActive ? markup.cancel() : void markup.start())}
|
||||
disabled={isBlankTab || grab.state !== 'idle'}
|
||||
active={markup.isActive}
|
||||
surfaceActive={isActive}
|
||||
/>
|
||||
|
||||
<Button
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import React from 'react'
|
||||
import React, { useEffect } from 'react'
|
||||
import { PenTool } from 'lucide-react'
|
||||
import { Badge } from '@/components/ui/badge'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
@@ -10,6 +12,12 @@ export type MarkupDrawButtonProps = {
|
||||
onClick: () => void
|
||||
disabled?: boolean
|
||||
active?: boolean
|
||||
/**
|
||||
* The browser pane's isActive. Required (no default) so every caller must
|
||||
* decide explicitly — inactive/hidden tabs cannot force-open a portaled
|
||||
* layer against a zero-size trigger (which lands at the viewport origin).
|
||||
*/
|
||||
surfaceActive: boolean
|
||||
className?: string
|
||||
}
|
||||
|
||||
@@ -19,45 +27,134 @@ export function MarkupDrawButton({
|
||||
onClick,
|
||||
disabled,
|
||||
active,
|
||||
surfaceActive,
|
||||
className
|
||||
}: MarkupDrawButtonProps): React.JSX.Element {
|
||||
const label = translate('auto.components.browser-pane.markup.drawButton', 'Draw on screenshot')
|
||||
// Why: nudge the button the first time it's usable so users discover the tool.
|
||||
const { hintOpen, dismissHint } = useMarkupDrawHint(!disabled && !active)
|
||||
// Why: nudge the button the first time it's usable on a visible surface so
|
||||
// users discover the tool without pinning a tooltip to a hidden pane.
|
||||
const { hintOpen, dismissHint } = useMarkupDrawHint(surfaceActive && !disabled && !active)
|
||||
// Why: close synchronously off the same `surfaceActive` render instead of
|
||||
// waiting on the hook's effect — BrowserPane sets inert/hidden on the pane
|
||||
// in the same commit isActive flips, so a one-tick delay would let the
|
||||
// popover anchor briefly against an already-hidden trigger.
|
||||
const showHint = hintOpen && surfaceActive
|
||||
|
||||
// Why: Electron <webview> clicks never reach the renderer document, so
|
||||
// Radix outside-click dismiss misses them. Window blur covers focus leaving
|
||||
// the renderer into the guest (same pattern as BrowserImportHintButton).
|
||||
useEffect(() => {
|
||||
if (!showHint) {
|
||||
return
|
||||
}
|
||||
window.addEventListener('blur', dismissHint)
|
||||
return () => window.removeEventListener('blur', dismissHint)
|
||||
}, [dismissHint, showHint])
|
||||
|
||||
const startMarkup = (): void => {
|
||||
dismissHint()
|
||||
onClick()
|
||||
}
|
||||
|
||||
const button = (
|
||||
<Button
|
||||
size="icon"
|
||||
variant={active ? 'default' : 'ghost'}
|
||||
className={cn(
|
||||
className ?? 'h-8 w-8',
|
||||
active && 'bg-foreground/80 text-background hover:bg-foreground/90',
|
||||
// Why: soft ring while the discovery callout is open — points at the
|
||||
// control without the harsh solid highlight of a force-open tooltip.
|
||||
showHint && 'ring-2 ring-ring/45 ring-offset-1 ring-offset-background'
|
||||
)}
|
||||
onClick={startMarkup}
|
||||
disabled={disabled}
|
||||
aria-label={label}
|
||||
aria-pressed={active}
|
||||
>
|
||||
<PenTool className="size-4" />
|
||||
</Button>
|
||||
)
|
||||
|
||||
// Discovery uses a dismissable popover (outside click / Escape / blur / act)
|
||||
// instead of a force-open tooltip — tooltips stay stuck when controlled open
|
||||
// and mis-position against hidden triggers. Styled like BrowserImportHint
|
||||
// (surface + title + body + actions), not an inverted tooltip chip.
|
||||
//
|
||||
// Why: Popover and Tooltip stay mounted as one persistent tree (matching
|
||||
// WorkspaceStatusAppearancePopover's Tooltip-wrapping-Popover pattern)
|
||||
// instead of switching root element types on `hintOpen` — swapping roots
|
||||
// would unmount/remount the trigger button at the exact moments the hint
|
||||
// is meant to draw the user's attention to it.
|
||||
return (
|
||||
<Tooltip open={hintOpen ? true : undefined}>
|
||||
<TooltipTrigger asChild>
|
||||
{/* Why: wrap in a span so tooltip hover still fires when disabled. */}
|
||||
<span className="inline-flex">
|
||||
<Button
|
||||
size="icon"
|
||||
variant={active ? 'default' : 'ghost'}
|
||||
className={cn(
|
||||
className ?? 'h-8 w-8',
|
||||
active && 'bg-foreground/80 text-background hover:bg-foreground/90',
|
||||
// Why: ring the button while the one-time discovery hint is open.
|
||||
hintOpen && 'ring-2 ring-ring ring-offset-1 ring-offset-background'
|
||||
)}
|
||||
onClick={() => {
|
||||
dismissHint()
|
||||
onClick()
|
||||
}}
|
||||
disabled={disabled}
|
||||
aria-label={label}
|
||||
aria-pressed={active}
|
||||
>
|
||||
<PenTool className="size-4" />
|
||||
</Button>
|
||||
</span>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="bottom" sideOffset={4}>
|
||||
{hintOpen
|
||||
? translate(
|
||||
'auto.components.browser-pane.markup.drawHint',
|
||||
'New — draw on the page, then copy it to your agent'
|
||||
)
|
||||
: label}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
<Popover
|
||||
modal={false}
|
||||
open={showHint}
|
||||
onOpenChange={(open) => {
|
||||
if (!open) {
|
||||
dismissHint()
|
||||
}
|
||||
}}
|
||||
>
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<PopoverTrigger asChild>
|
||||
{/* Why: wrap in a span so the trigger still fires when disabled. */}
|
||||
<span className="inline-flex">{button}</span>
|
||||
</PopoverTrigger>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="bottom" sideOffset={4}>
|
||||
{label}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
<PopoverContent
|
||||
side="bottom"
|
||||
align="center"
|
||||
sideOffset={6}
|
||||
className="w-72 p-3"
|
||||
onOpenAutoFocus={(event) => {
|
||||
// Why: don't steal focus from the address bar / webview on first show.
|
||||
event.preventDefault()
|
||||
}}
|
||||
>
|
||||
<div className="space-y-3">
|
||||
<div className="space-y-1.5">
|
||||
<div className="flex items-center gap-2">
|
||||
<Badge
|
||||
variant="secondary"
|
||||
className="h-5 shrink-0 px-1.5 text-[10px] font-semibold tracking-wider uppercase"
|
||||
>
|
||||
{translate('auto.components.browser-pane.markup.drawHintBadge', 'New')}
|
||||
</Badge>
|
||||
<div className="text-sm font-medium text-foreground">{label}</div>
|
||||
</div>
|
||||
<p className="text-xs leading-5 text-muted-foreground">
|
||||
{translate(
|
||||
'auto.components.browser-pane.markup.drawHint',
|
||||
'Draw on the page, then copy the markup to paste into your agent.'
|
||||
)}
|
||||
</p>
|
||||
</div>
|
||||
<div className="flex items-center gap-3">
|
||||
<Button
|
||||
type="button"
|
||||
size="sm"
|
||||
className="h-7 gap-1.5 px-2.5 text-xs"
|
||||
onClick={startMarkup}
|
||||
>
|
||||
<PenTool className="size-3.5" />
|
||||
{translate('auto.components.browser-pane.markup.drawHintTry', 'Try it')}
|
||||
</Button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={dismissHint}
|
||||
className="rounded-sm text-xs text-muted-foreground hover:text-foreground focus-visible:ring-1 focus-visible:ring-ring focus-visible:outline-none"
|
||||
>
|
||||
{translate('auto.components.browser-pane.markup.drawHintDismiss', 'Got it')}
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
</PopoverContent>
|
||||
</Popover>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
/** @vitest-environment happy-dom */
|
||||
import { act, renderHook } from '@testing-library/react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const HINT_KEY = 'orca.browser.markup-draw-hint-seen'
|
||||
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: (selector: (state: { persistedUIReady: boolean }) => unknown) =>
|
||||
selector({ persistedUIReady: true })
|
||||
}))
|
||||
|
||||
import { useMarkupDrawHint } from './use-markup-draw-hint'
|
||||
|
||||
describe('useMarkupDrawHint', () => {
|
||||
beforeEach(() => {
|
||||
window.localStorage.removeItem(HINT_KEY)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
window.localStorage.removeItem(HINT_KEY)
|
||||
})
|
||||
|
||||
it('opens once when eligible and records the seen flag', () => {
|
||||
const { result } = renderHook(() => useMarkupDrawHint(true))
|
||||
|
||||
expect(result.current.hintOpen).toBe(true)
|
||||
expect(window.localStorage.getItem(HINT_KEY)).toBe('true')
|
||||
})
|
||||
|
||||
it('does not open when the surface is not eligible', () => {
|
||||
const { result } = renderHook(() => useMarkupDrawHint(false))
|
||||
|
||||
expect(result.current.hintOpen).toBe(false)
|
||||
expect(window.localStorage.getItem(HINT_KEY)).toBeNull()
|
||||
})
|
||||
|
||||
it('closes when eligibility drops so a hidden pane cannot keep it open', () => {
|
||||
const { result, rerender } = renderHook(({ eligible }) => useMarkupDrawHint(eligible), {
|
||||
initialProps: { eligible: true }
|
||||
})
|
||||
|
||||
expect(result.current.hintOpen).toBe(true)
|
||||
|
||||
rerender({ eligible: false })
|
||||
expect(result.current.hintOpen).toBe(false)
|
||||
})
|
||||
|
||||
it('dismisses without reopening after the first view', () => {
|
||||
const first = renderHook(() => useMarkupDrawHint(true))
|
||||
expect(first.result.current.hintOpen).toBe(true)
|
||||
|
||||
act(() => {
|
||||
first.result.current.dismissHint()
|
||||
})
|
||||
expect(first.result.current.hintOpen).toBe(false)
|
||||
first.unmount()
|
||||
|
||||
const second = renderHook(() => useMarkupDrawHint(true))
|
||||
expect(second.result.current.hintOpen).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -2,11 +2,13 @@ import { useCallback, useEffect, useState } from 'react'
|
||||
import { useAppStore } from '@/store'
|
||||
|
||||
// One-time discovery highlight for the screenshot-markup Draw button. Shows once
|
||||
// per install — the first time the button is usable — so users notice the new
|
||||
// tool. Gated on its own localStorage flag (not a contextual tour), so it fires
|
||||
// for everyone, including users who already finished the capped browser tour.
|
||||
// It stays open until the user acts (clicks Draw) or the button stops being
|
||||
// usable; there is no auto-timeout.
|
||||
// per install — the first time the button is usable and its surface is active —
|
||||
// so users notice the new tool. Gated on its own localStorage flag (not a
|
||||
// contextual tour), so it fires for everyone, including users who already
|
||||
// finished the capped browser tour.
|
||||
//
|
||||
// Stays open until the user dismisses it (outside click / Escape / blur), clicks
|
||||
// Draw, or the button stops being eligible. No auto-timeout.
|
||||
const MARKUP_DRAW_HINT_SEEN_KEY = 'orca.browser.markup-draw-hint-seen'
|
||||
|
||||
// Records the first-ever view and returns whether this call is that first view.
|
||||
@@ -31,9 +33,10 @@ export function useMarkupDrawHint(eligible: boolean): MarkupDrawHint {
|
||||
const [hintOpen, setHintOpen] = useState(false)
|
||||
|
||||
useEffect(() => {
|
||||
// Why: only nudge once the app is ready and the button is usable. If it stops
|
||||
// being usable while showing (grab started, markup open, blank tab), close it
|
||||
// so the forced-open tooltip can't get stuck over a now-disabled button.
|
||||
// Why: only nudge once the app is ready and the button is usable on a
|
||||
// visible surface. If eligibility drops mid-hint (tab switch, grab
|
||||
// started, markup open, blank tab), close it so a forced-open floating
|
||||
// layer can't stick over a hidden or disabled control at (0,0).
|
||||
if (!persistedUIReady || !eligible) {
|
||||
setHintOpen(false)
|
||||
return
|
||||
|
||||
@@ -13028,7 +13028,10 @@
|
||||
"copiedToast": "Markup copied — paste it into your agent ({{value0}})",
|
||||
"copy": "Copy Markup",
|
||||
"drawButton": "Draw on screenshot",
|
||||
"drawHint": "New — draw on the page, then copy it to your agent",
|
||||
"drawHint": "Draw on the page, then copy the markup to paste into your agent.",
|
||||
"drawHintBadge": "New",
|
||||
"drawHintDismiss": "Got it",
|
||||
"drawHintTry": "Try it",
|
||||
"errorAttach": "Could not attach the markup screenshot.",
|
||||
"errorCapture": "Could not capture the page to draw on.",
|
||||
"errorUnavailable": "Screenshot markup is not available on this page.",
|
||||
|
||||
@@ -13028,7 +13028,10 @@
|
||||
"copiedToast": "Marcado copiado: pégalo en tu agente ({{value0}})",
|
||||
"copy": "Copiar marcado",
|
||||
"drawButton": "Dibujar en la captura",
|
||||
"drawHint": "Nuevo: dibuja en la página y cópialo en tu agente",
|
||||
"drawHint": "Dibuja en la página y copia el markup para pegarlo en tu agente.",
|
||||
"drawHintBadge": "Nuevo",
|
||||
"drawHintDismiss": "Entendido",
|
||||
"drawHintTry": "Probar",
|
||||
"errorAttach": "No se pudo adjuntar la captura de pantalla con marcado.",
|
||||
"errorCapture": "No se pudo capturar la página para dibujar.",
|
||||
"errorUnavailable": "El marcado de captura de pantalla no está disponible en esta página.",
|
||||
|
||||
@@ -13028,7 +13028,10 @@
|
||||
"copiedToast": "注釈をコピーしました — エージェントに貼り付けてください({{value0}})",
|
||||
"copy": "注釈をコピー",
|
||||
"drawButton": "スクリーンショットに描き込み",
|
||||
"drawHint": "新機能 — ページに描き込み、エージェントに貼り付けられます",
|
||||
"drawHint": "ページに描き込み、マークアップをコピーしてエージェントに貼り付けられます。",
|
||||
"drawHintBadge": "新機能",
|
||||
"drawHintDismiss": "了解",
|
||||
"drawHintTry": "試してみる",
|
||||
"errorAttach": "注釈付きスクリーンショットを添付できませんでした。",
|
||||
"errorCapture": "描画用にページを取得できませんでした。",
|
||||
"errorUnavailable": "このページではスクリーンショット注釈を利用できません。",
|
||||
|
||||
@@ -13028,7 +13028,10 @@
|
||||
"copiedToast": "마크업을 복사했습니다 — 에이전트에 붙여넣으세요 ({{value0}})",
|
||||
"copy": "마크업 복사",
|
||||
"drawButton": "스크린샷에 그리기",
|
||||
"drawHint": "새 기능 — 페이지에 그린 다음 에이전트에 붙여넣으세요",
|
||||
"drawHint": "페이지에 그린 다음 마크업을 복사해 에이전트에 붙여넣으세요.",
|
||||
"drawHintBadge": "새 기능",
|
||||
"drawHintDismiss": "확인",
|
||||
"drawHintTry": "사용해 보기",
|
||||
"errorAttach": "마크업 스크린샷을 첨부하지 못했습니다.",
|
||||
"errorCapture": "그릴 페이지를 캡처하지 못했습니다.",
|
||||
"errorUnavailable": "이 페이지에서는 스크린샷 마크업을 사용할 수 없습니다.",
|
||||
|
||||
@@ -13028,7 +13028,10 @@
|
||||
"copiedToast": "已复制标注 — 粘贴到你的智能体({{value0}})",
|
||||
"copy": "复制标注",
|
||||
"drawButton": "在截图上绘制",
|
||||
"drawHint": "新功能 — 在页面上绘制,然后复制到你的智能体",
|
||||
"drawHint": "在页面上绘制,然后复制标记并粘贴到你的智能体。",
|
||||
"drawHintBadge": "新功能",
|
||||
"drawHintDismiss": "知道了",
|
||||
"drawHintTry": "试一试",
|
||||
"errorAttach": "无法附加标注后的截图。",
|
||||
"errorCapture": "无法捕获页面进行绘制。",
|
||||
"errorUnavailable": "此页面无法使用截图标注。",
|
||||
|
||||
Reference in New Issue
Block a user