From 66b54c4f49db4f6d77bcd1f3fbe1f5dfa382a8bf Mon Sep 17 00:00:00 2001 From: Micko Cabacungan <53090061+nightlaro@users.noreply.github.com> Date: Fri, 26 Jun 2026 19:55:02 -0700 Subject: [PATCH] fix(pet): shrink overlay drag area to hug the pet content (#6437) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(pet): shrink overlay drag area to hug the pet content The pet overlay's `pointer-events-auto` handle used `size-full`, so the entire `size × size` box (default 180px) was draggable even where the pet was transparent. Non-square pets left a large dead-zone: the bundled Claudino (320×180) renders 180×101 in the box, leaving a ~79px empty but draggable band; OpenCode/Gremlin (252×320) render 142×180, leaving ~38px on the left. Restructure the overlay into three layers: the outer box and a new centering layer stay `pointer-events-none`; only an innermost `w-fit`/`h-fit` wrapper opts back into pointer events and carries the drag handlers, cursor, bob animation, and `touch-action`. A `min-w`/`min-h` floor keeps it grabbable during the image-load window when the wrapper would otherwise collapse to 0×0. Also size the `DetectedSpriteFrame` canvas to one fixed footprint bounding the largest scaled frame (instead of the full box) so detected sprites tighten too, with frames re-centered within that footprint. Drag math, viewport clamping, and persisted position are unchanged — they remain keyed to the outer box, so dragging, edge-clamping, and saved positions behave exactly as before. * fix(pet): cap the img fallback at the pet size The w-fit/h-fit drag wrapper is fit-content, so the img's max-w/h-full had no fixed box to resolve against and rendered at intrinsic size, overflowing the persisted size box that clamping still assumes. Cap explicitly with maxWidth/maxHeight: size. Addresses CodeRabbit review on #6437. * test(pet): cover overlay hit area render branches Co-authored-by: Orca --------- Co-authored-by: Jinwoo-H Co-authored-by: Orca --- .../src/components/pet/PetOverlay.tsx | 124 +++++++----- .../pet/pet-overlay-hit-area.test.tsx | 177 ++++++++++++++++++ 2 files changed, 256 insertions(+), 45 deletions(-) create mode 100644 src/renderer/src/components/pet/pet-overlay-hit-area.test.tsx diff --git a/src/renderer/src/components/pet/PetOverlay.tsx b/src/renderer/src/components/pet/PetOverlay.tsx index 7dfc27c37..e55004a47 100644 --- a/src/renderer/src/components/pet/PetOverlay.tsx +++ b/src/renderer/src/components/pet/PetOverlay.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useId, useRef, useState } from 'react' +import { useCallback, useEffect, useId, useMemo, useRef, useState } from 'react' import { usePrefersReducedMotion } from '@/hooks/usePrefersReducedMotion' import { usePetUrl } from './usePetUrl' import type { DetectedSpriteCacheEntry } from './pet-blob-cache' @@ -113,6 +113,20 @@ function DetectedSpriteFrame({ // intended speed; default to 8 only when the manifest didn't declare one. const fps = detected.fps > 0 ? detected.fps : 8 + // Why: size the canvas to one fixed footprint bounding the largest scaled + // frame so the drag wrapper hugs the pet instead of a maxSize square. A + // single size across frames avoids the jitter a per-frame resize would cause. + const { footprintW, footprintH } = useMemo(() => { + let w = 0 + let h = 0 + for (const f of detected.frames) { + const s = Math.min(maxSize / f.w, maxSize / f.h) + w = Math.max(w, f.w * s) + h = Math.max(h, f.h * s) + } + return { footprintW: Math.max(1, Math.round(w)), footprintH: Math.max(1, Math.round(h)) } + }, [detected, maxSize]) + useEffect(() => { const canvas = canvasRef.current if (!canvas) { @@ -122,22 +136,31 @@ function DetectedSpriteFrame({ if (!ctx) { return } - canvas.width = maxSize - canvas.height = maxSize + canvas.width = footprintW + canvas.height = footprintH // Why: reset playback when the underlying sprite changes so the new // animation starts from frame 0 rather than wherever the prior one stopped. frameIndexRef.current = 0 lastTimeRef.current = 0 + if (detected.frames.length === 0) { + ctx.clearRect(0, 0, canvas.width, canvas.height) + return + } let raf = 0 const draw = (): void => { const f = detected.frames[frameIndexRef.current % detected.frames.length] const bmp = detected.bitmaps[frameIndexRef.current % detected.bitmaps.length] + if (!f || !bmp) { + return + } ctx.imageSmoothingEnabled = false ctx.clearRect(0, 0, canvas.width, canvas.height) const scale = Math.min(maxSize / f.w, maxSize / f.h) const w = f.w * scale const h = f.h * scale - ctx.drawImage(bmp, (maxSize - w) / 2, (maxSize - h) / 2, w, h) + // Why: center each frame within the fixed footprint so frames of differing + // sizes stay aligned without resizing the canvas per frame. + ctx.drawImage(bmp, (footprintW - w) / 2, (footprintH - h) / 2, w, h) } const tick = (now: number): void => { const dt = now - lastTimeRef.current @@ -160,12 +183,12 @@ function DetectedSpriteFrame({ cancelAnimationFrame(raf) } } - }, [detected, animate, maxSize, fps]) + }, [detected, animate, footprintW, footprintH, maxSize, fps]) return ( ) } @@ -363,9 +386,9 @@ export function PetOverlay(): React.JSX.Element { } return ( - // Why: the wrapper is fixed-positioned and pointer-events-none so app - // chrome stays interactive; only the pet itself opts back in to - // pointer events so the user can press and drag it around. + // Why: the outer box and middle layer stay pointer-events-none so app chrome + // stays interactive; only the innermost wrapper opts in and shrink-wraps its + // content, so the grab/drag hit area hugs the pet, not the full square box.
-
- + {sprite ? ( + + ) : detected ? ( + + ) : ( + // Why: cap explicitly at the pet size — the w-fit/h-fit wrapper is + // fit-content, so max-w/h-full has no fixed box to resolve against + // and the image would otherwise render at its intrinsic size and + // overflow the persisted size box that clamping still assumes. + )} - - {sprite ? ( - - ) : detected ? ( - - ) : ( - - )} +
) diff --git a/src/renderer/src/components/pet/pet-overlay-hit-area.test.tsx b/src/renderer/src/components/pet/pet-overlay-hit-area.test.tsx new file mode 100644 index 000000000..adb257ce4 --- /dev/null +++ b/src/renderer/src/components/pet/pet-overlay-hit-area.test.tsx @@ -0,0 +1,177 @@ +// @vitest-environment happy-dom + +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { DetectedSpriteCacheEntry } from './pet-blob-cache' +import type { CustomPet } from '../../../../shared/types' + +type PetUrlState = + | { url: string; ready: boolean; sprite: null; detected: null } + | { + url: string + ready: boolean + sprite: NonNullable + detected: null + } + | { url: string; ready: boolean; sprite: null; detected: DetectedSpriteCacheEntry } + +const defaultPetUrlState: PetUrlState = { + url: 'data:image/png;base64,', + ready: true, + sprite: null, + detected: null +} + +const petUrlMock = vi.hoisted(() => ({ + current: { + url: 'data:image/png;base64,', + ready: true, + sprite: null, + detected: null + } as PetUrlState +})) + +// Why: keep the render focused on the overlay's layout structure — the real +// store + pet-url resolution pull in IPC/asset loading we don't need to assert +// the hit-area invariant. +vi.mock('../../store', () => { + const storeState = { + petSize: 180, + agentStatusByPaneKey: {}, + agentStatusEpoch: 0, + retainedAgentsByPaneKey: {} + } + const useAppStore = Object.assign( + (selector: (state: unknown) => unknown) => selector(storeState), + { getState: () => storeState } + ) + return { useAppStore } +}) + +vi.mock('./usePetUrl', () => ({ + usePetUrl: () => petUrlMock.current +})) + +import { PetOverlay } from './PetOverlay' + +function renderOverlay(): { root: Root; container: HTMLDivElement } { + const container = document.createElement('div') + document.body.appendChild(container) + const root = createRoot(container) + act(() => { + root.render() + }) + return { root, container } +} + +function setPetUrlState(state: PetUrlState): void { + petUrlMock.current = state +} + +function getDragHandle(container: HTMLElement): HTMLElement { + const fixedEl = container.querySelector('.fixed') + const sizeFullEl = container.querySelector('.size-full') + const grabEls = container.querySelectorAll('.pointer-events-auto') + + expect(fixedEl).not.toBeNull() + expect(sizeFullEl).not.toBeNull() + // Exactly one element opts into pointer events: the content-fit grab handle. + expect(grabEls.length).toBe(1) + const grabEl = grabEls[0] as HTMLElement + + // The outer box and the size-full middle layer must stay pointer-events-none + // so they never become a grab surface around the rendered pet. + expect(fixedEl?.className).toContain('pointer-events-none') + expect(sizeFullEl?.className).toContain('pointer-events-none') + expect(sizeFullEl?.className).not.toContain('pointer-events-auto') + + // The grab handle (the element carrying the pointer handlers) is NOT the + // full-size box: it does not carry size-full and sits nested inside it. + expect(grabEl.className).not.toContain('size-full') + expect(grabEl).not.toBe(sizeFullEl) + expect(sizeFullEl?.contains(grabEl)).toBe(true) + + // The drag affordances (cursor + touch-action) live on that same handle, + // confirming it is the pointer-handler element rather than the box. + expect(grabEl.style.cursor).toBe('grab') + expect(grabEl.style.touchAction).toBe('none') + + return grabEl +} + +describe('PetOverlay drag hit area', () => { + let root: Root | null = null + let container: HTMLDivElement | null = null + + afterEach(() => { + if (root) { + act(() => root?.unmount()) + } + container?.remove() + root = null + container = null + setPetUrlState(defaultPetUrlState) + }) + + it('keeps the grab/drag region off the full-size square box', () => { + ;({ root, container } = renderOverlay()) + + const grabEl = getDragHandle(container) + const img = grabEl.querySelector('img') as HTMLImageElement | null + expect(img).not.toBeNull() + expect(img?.style.maxWidth).toBe('180px') + expect(img?.style.maxHeight).toBe('180px') + }) + + it('renders manifest sprites with a content-sized drag handle child', () => { + setPetUrlState({ + url: 'data:image/png;base64,', + ready: true, + sprite: { + frameWidth: 252, + frameHeight: 320, + columns: 4, + rows: 1, + sheetWidth: 1008, + sheetHeight: 320, + fps: 8, + defaultAnimation: 'idle', + animations: { idle: { row: 0, frames: 4 } } + }, + detected: null + }) + + ;({ root, container } = renderOverlay()) + + const grabEl = getDragHandle(container) + const spriteEl = grabEl.querySelector('div[style*="background-image"]') as HTMLElement | null + expect(spriteEl).not.toBeNull() + expect(spriteEl?.style.width).toBe('141.75px') + expect(spriteEl?.style.height).toBe('180px') + }) + + it('renders detected sprites with a fixed content footprint smaller than the square box', () => { + setPetUrlState({ + url: 'data:image/png;base64,', + ready: true, + sprite: null, + detected: { + frames: [ + { x: 0, y: 0, w: 252, h: 320 }, + { x: 0, y: 0, w: 126, h: 320 } + ], + bitmaps: [] as ImageBitmap[], + fps: 8 + } + }) + + ;({ root, container } = renderOverlay()) + + const grabEl = getDragHandle(container) + const canvas = grabEl.querySelector('canvas') as HTMLCanvasElement | null + expect(canvas).not.toBeNull() + expect(canvas?.style.width).toBe('142px') + expect(canvas?.style.height).toBe('180px') + }) +})