From 49fbca18802d8bcc9feec889d8e8fea8cbf49907 Mon Sep 17 00:00:00 2001 From: Lester Liang <153183032+lesterlxt@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:21:25 +1000 Subject: [PATCH] fix(desktop): prevent accidental composer popout --- .../hooks/use-composer-popout.test.tsx | 45 +++++++++++++ .../composer/hooks/use-composer-popout.ts | 4 +- .../composer/hooks/use-popout-drag.test.tsx | 66 +++++++++++++++++++ .../chat/composer/hooks/use-popout-drag.ts | 15 ++++- .../src/app/settings/appearance-settings.tsx | 11 +++- apps/desktop/src/i18n/ar.ts | 2 + apps/desktop/src/i18n/en.ts | 2 + apps/desktop/src/i18n/ja.ts | 2 + apps/desktop/src/i18n/types.ts | 2 + apps/desktop/src/i18n/zh-hant.ts | 2 + apps/desktop/src/i18n/zh.ts | 2 + .../store/composer-popout-preference.test.ts | 65 ++++++++++++++++++ apps/desktop/src/store/composer-popout.ts | 38 ++++++++++- 13 files changed, 250 insertions(+), 6 deletions(-) create mode 100644 apps/desktop/src/app/chat/composer/hooks/use-composer-popout.test.tsx create mode 100644 apps/desktop/src/app/chat/composer/hooks/use-popout-drag.test.tsx create mode 100644 apps/desktop/src/store/composer-popout-preference.test.ts diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-popout.test.tsx b/apps/desktop/src/app/chat/composer/hooks/use-composer-popout.test.tsx new file mode 100644 index 0000000000..91721c8e0b --- /dev/null +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-popout.test.tsx @@ -0,0 +1,45 @@ +import { cleanup, render, screen } from '@testing-library/react' +import { useRef } from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { $composerPopoutGesturesEnabled, $composerPopoutZones } from '@/store/composer-popout' + +import { useComposerPopout } from './use-composer-popout' + +vi.mock('@/components/pane-shell/pane-visibility', () => ({ + usePaneGroup: () => 'test-zone', + usePaneVisible: () => true +})) + +vi.mock('@/hooks/use-resize-observer', () => ({ useResizeObserver: () => undefined })) +vi.mock('@/store/windows', () => ({ isSecondaryWindow: () => false })) + +function PopoutAffordanceHarness() { + const composerRef = useRef(null) + const { popoutAllowed } = useComposerPopout({ composerRef }) + + return ( +
{popoutAllowed &&
} + ) +} + +describe('useComposerPopout', () => { + beforeEach(() => { + $composerPopoutZones.set({}) + $composerPopoutGesturesEnabled.set(true) + }) + + afterEach(() => { + cleanup() + $composerPopoutZones.set({}) + $composerPopoutGesturesEnabled.set(true) + }) + + it('removes the pop-out affordance when gestures are disabled', () => { + $composerPopoutGesturesEnabled.set(false) + + render() + + expect(screen.queryByTestId('drag-region')).toBeNull() + }) +}) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-popout.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-popout.ts index b326025bb9..c304a71279 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-popout.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-popout.ts @@ -5,6 +5,7 @@ import { usePaneGroup, usePaneVisible } from '@/components/pane-shell/pane-visib import { useResizeObserver } from '@/hooks/use-resize-observer' import { triggerHaptic } from '@/lib/haptics' import { + $composerPopoutGesturesEnabled, $composerPopoutZone, clampPopoutPosition, getComposerPopoutZone, @@ -121,8 +122,9 @@ function usePopoutPlacement( * docked: a floating composer makes no sense in a scratch window. */ export function useComposerPopout({ composerRef }: UseComposerPopoutOptions) { - const popoutAllowed = !isSecondaryWindow() const groupId = usePaneGroup() + const gesturesEnabled = useStore($composerPopoutGesturesEnabled) + const popoutAllowed = gesturesEnabled && !isSecondaryWindow() const zone = useStore(useMemo(() => $composerPopoutZone(groupId), [groupId])) const poppedOut = zone.poppedOut && popoutAllowed diff --git a/apps/desktop/src/app/chat/composer/hooks/use-popout-drag.test.tsx b/apps/desktop/src/app/chat/composer/hooks/use-popout-drag.test.tsx new file mode 100644 index 0000000000..fbc4eaefce --- /dev/null +++ b/apps/desktop/src/app/chat/composer/hooks/use-popout-drag.test.tsx @@ -0,0 +1,66 @@ +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { useRef } from 'react' +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { useComposerPopoutGestures } from './use-popout-drag' + +function GestureHarness({ onPopOut }: { onPopOut: () => void }) { + const composerRef = useRef(null) + + const { onPointerDown } = useComposerPopoutGestures({ + composerRef, + groupId: 'test-zone', + onDock: vi.fn(), + onPopOut, + poppedOut: false, + position: { bottom: 24, right: 24 } + }) + + return ( +
+
+
+
+ select me +
+
+ + ) +} + +function dragUp(target: Element) { + fireEvent.pointerDown(target, { button: 0, clientX: 100, clientY: 100, pointerId: 7 }) + fireEvent.pointerMove(window, { clientX: 100, clientY: 60, pointerId: 7 }) + fireEvent.pointerUp(window, { clientX: 100, clientY: 60, pointerId: 7 }) +} + +afterEach(cleanup) + +describe('useComposerPopoutGestures', () => { + it('never peels the composer out when a text-selection drag starts in the rich editor', () => { + const onPopOut = vi.fn() + render() + + dragUp(screen.getByTestId('editable-text')) + + expect(onPopOut).not.toHaveBeenCalled() + }) + + it('does not arm a dock peel from the composer surface', () => { + const onPopOut = vi.fn() + render() + + dragUp(screen.getByTestId('surface')) + + expect(onPopOut).not.toHaveBeenCalled() + }) + + it('still peels out from the dedicated drag region', () => { + const onPopOut = vi.fn() + render() + + dragUp(screen.getByTestId('drag-region')) + + expect(onPopOut).toHaveBeenCalledOnce() + }) +}) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-popout-drag.ts b/apps/desktop/src/app/chat/composer/hooks/use-popout-drag.ts index e4a53889e5..a1f3ce3c9c 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-popout-drag.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-popout-drag.ts @@ -49,7 +49,16 @@ function gestureTargetOk(target: EventTarget | null) { return false } - return !target.closest('button, a, input, textarea, select, [role="menuitem"], [data-radix-popper-content-wrapper]') + return !target.closest( + 'button, a, input, textarea, select, [contenteditable]:not([contenteditable="false"]), [data-slot="composer-rich-input"], [role="menuitem"], [data-radix-popper-content-wrapper]' + ) +} + +/** Docked composer only peels from its exposed grab ring. Pointer events from + * the editor and composer surface bubble through the root too, but they belong + * to text selection and controls, never to the pop-out gesture. */ +function isDockDragPlatform(target: EventTarget | null) { + return target instanceof Element && Boolean(target.closest('[data-slot="composer-drag-region"]')) } /** Floating composer's 5px outer frame — grab here to drag without long-press. */ @@ -217,6 +226,10 @@ export function useComposerPopoutGestures({ return } + if (!poppedOut && !isDockDragPlatform(event.target)) { + return + } + stateRef.current = { armed: false, mode: poppedOut ? 'float' : 'dock', diff --git a/apps/desktop/src/app/settings/appearance-settings.tsx b/apps/desktop/src/app/settings/appearance-settings.tsx index 579f21b3b7..425b8a86d1 100644 --- a/apps/desktop/src/app/settings/appearance-settings.tsx +++ b/apps/desktop/src/app/settings/appearance-settings.tsx @@ -13,6 +13,7 @@ import { selectableCardClass } from '@/lib/selectable-card' import { normalize } from '@/lib/text' import { cn } from '@/lib/utils' import { $backdrop, setBackdrop } from '@/store/backdrop' +import { $composerPopoutGesturesEnabled, setComposerPopoutGesturesEnabled } from '@/store/composer-popout' import { $embedAllowed, $embedMode, clearEmbedAllowed, type EmbedMode, setEmbedMode } from '@/store/embed-consent' import { $activeGatewayProfile, $profiles, normalizeProfileKey } from '@/store/profile' import { $reactionsEnabled, setReactionsEnabled } from '@/store/reactions-enabled' @@ -28,7 +29,7 @@ import { $marketplaceInstalls, isUserTheme, removeUserTheme } from '@/themes/use import { MODE_OPTIONS } from './constants' import { PetSettings } from './pet-settings' -import { ListRow, SectionHeading, SettingsContent } from './primitives' +import { ListRow, SectionHeading, SettingsContent, ToggleRow } from './primitives' import { TerminalFontSetting } from './terminal-font-setting' function ThemePreview({ name, mode }: { name: string; mode: 'light' | 'dark' }) { @@ -255,6 +256,7 @@ export function AppearanceSettings() { const zoomPercent = useStore($zoomPercent) const embedMode = useStore($embedMode) const embedAllowed = useStore($embedAllowed) + const composerPopoutGesturesEnabled = useStore($composerPopoutGesturesEnabled) const translucency = useStore($translucency) const reactionsEnabled = useStore($reactionsEnabled) const backdrop = useStore($backdrop) @@ -502,6 +504,13 @@ export function AppearanceSettings() { title={a.backdropTitle} /> + + import('./composer-popout') + +describe('composer pop-out preference', () => { + beforeEach(() => { + window.localStorage.clear() + vi.resetModules() + }) + + it('docks every floating zone, preserves positions, and persists the lock', async () => { + const first = await loadStore() + + first.setComposerPopoutPosition('left', { bottom: 100, right: 100 }) + first.setComposerPoppedOut('left', true) + first.setComposerPopoutPosition('right', { bottom: 200, right: 200 }) + first.setComposerPoppedOut('right', true) + + first.setComposerPopoutGesturesEnabled(false) + + expect(first.$composerPopoutGesturesEnabled.get()).toBe(false) + expect(first.getComposerPopoutZone('left')).toEqual({ + poppedOut: false, + position: { bottom: 100, right: 100 } + }) + expect(first.getComposerPopoutZone('right')).toEqual({ + poppedOut: false, + position: { bottom: 200, right: 200 } + }) + expect(window.localStorage.getItem(GESTURES_KEY)).toBe('false') + expect(window.localStorage.getItem(LEGACY_ENABLED_KEY)).toBe('false') + + vi.resetModules() + const reloaded = await loadStore() + + expect(reloaded.$composerPopoutGesturesEnabled.get()).toBe(false) + expect(reloaded.getComposerPopoutZone('left').poppedOut).toBe(false) + expect(reloaded.getComposerPopoutZone('right').poppedOut).toBe(false) + }) + + it('normalizes stale floating zones when the persisted preference is disabled', async () => { + window.localStorage.setItem(GESTURES_KEY, 'false') + window.localStorage.setItem( + ZONES_KEY, + JSON.stringify({ stale: { poppedOut: true, position: { bottom: 48, right: 64 } } }) + ) + + const store = await loadStore() + + expect(store.getComposerPopoutZone('stale')).toEqual({ + poppedOut: false, + position: { bottom: 48, right: 64 } + }) + }) + + it('keeps pop-out gestures enabled by default', async () => { + const store = await loadStore() + + expect(store.$composerPopoutGesturesEnabled.get()).toBe(true) + }) +}) diff --git a/apps/desktop/src/store/composer-popout.ts b/apps/desktop/src/store/composer-popout.ts index a5d3b5ab6d..ea6ec7e464 100644 --- a/apps/desktop/src/store/composer-popout.ts +++ b/apps/desktop/src/store/composer-popout.ts @@ -1,14 +1,17 @@ import { atom, computed, type ReadableAtom } from 'nanostores' -import { persistString, storedString } from '@/lib/storage' +import { persistBoolean, persistString, storedBoolean, storedString } from '@/lib/storage' const POPOUT_STORAGE_KEY = 'hermes.desktop.composerPopout.zones.v1' +const POPOUT_GESTURES_ENABLED_STORAGE_KEY = 'hermes.desktop.composerPopout.gesturesEnabled' // Pre-zone keys: one flag + one position for the whole window. Read at load to // seed the first zone the user touches (see `legacySeed`), never written again. const LEGACY_ENABLED_KEY = 'hermes.desktop.composerPopout.enabled' const LEGACY_POSITION_KEY = 'hermes.desktop.composerPopout.position' +const gesturesEnabledAtLoad = storedBoolean(POPOUT_GESTURES_ENABLED_STORAGE_KEY, true) + /** Where the floating composer's bottom-right corner sits, measured as an inset * from the viewport's bottom/right edges. Anchoring to the bottom-right keeps * the box visually pinned to its default corner as the window resizes and as @@ -63,7 +66,7 @@ function load(): Record { const zone = value as null | Partial if (typeof zone?.poppedOut === 'boolean' && isPosition(zone.position)) { - out[id] = { poppedOut: zone.poppedOut, position: { ...zone.position } } + out[id] = { poppedOut: gesturesEnabledAtLoad && zone.poppedOut, position: { ...zone.position } } } } } catch { @@ -80,6 +83,7 @@ function load(): Record { * put it), while a split zone beside them keeps its own — popping out on the * left doesn't fling a composer out of the right. */ export const $composerPopoutZones = atom>(load()) +export const $composerPopoutGesturesEnabled = atom(gesturesEnabledAtLoad) /** Write-through to storage. Called explicitly — NOT on every store change: a * drag updates the position once per frame, and serializing every zone to @@ -91,7 +95,9 @@ const persistZones = () => persistString(POPOUT_STORAGE_KEY, JSON.stringify($com * — but ONLY until they touch any zone. Once real per-zone state exists, that * is the truth, and a zone split later starts docked like any other. */ let legacySeed: PopoutZoneState | null = - storedString(LEGACY_ENABLED_KEY) === 'true' ? { poppedOut: true, position: legacyPosition() } : null + gesturesEnabledAtLoad && storedString(LEGACY_ENABLED_KEY) === 'true' + ? { poppedOut: true, position: legacyPosition() } + : null const zoneState = (zones: Record, groupId: string): PopoutZoneState => zones[groupId] ?? legacySeed ?? DEFAULT_ZONE @@ -223,6 +229,32 @@ export function setComposerPoppedOut(groupId: string, value: boolean) { persistZones() } +export function setComposerPopoutGesturesEnabled(value: boolean) { + $composerPopoutGesturesEnabled.set(value) + persistBoolean(POPOUT_GESTURES_ENABLED_STORAGE_KEY, value) + + if (value) { + return + } + + // Turning the feature off is an immediate lock-to-dock action for every + // layout zone, not merely a promise to ignore the next gesture. Keep each + // zone's resting position so opting back in restores where the user put it. + const zones = $composerPopoutZones.get() + + const docked = Object.fromEntries( + Object.entries(zones).map(([groupId, zone]) => [groupId, { ...zone, poppedOut: false }]) + ) + + legacySeed = null + $composerPopoutZones.set(docked) + persistZones() + + // Neutralize the pre-zone singleton migration seed too, otherwise a reload + // with no materialized zones could revive an old floating composer. + persistBoolean(LEGACY_ENABLED_KEY, false) +} + /** Move this zone's box. Used per-frame during a drag, so it only writes the * in-memory store by default; pass `persist` for the resting position on * release. Returns the clamped position so callers can sync their live ref. */