diff --git a/.changeset/7974-swipe-direction-honours-declared-set.md b/.changeset/7974-swipe-direction-honours-declared-set.md new file mode 100644 index 0000000000..d1f0ed2fcd --- /dev/null +++ b/.changeset/7974-swipe-direction-honours-declared-set.md @@ -0,0 +1,30 @@ +--- +'@object-ui/mobile': minor +--- + +`useSpecGesture` fires a swipe on MEMBERSHIP of the declared direction set +(objectui#7974, maintainer ruling of decision batch #70). + +`SwipeGestureConfig.direction` is declared `SpecSwipeDirection[]` — a set — and the +hook now honours the whole of it: it recognizes the any-direction move past the +threshold and fires only when the DETECTED direction is a member of the declared +set. It used to read `direction[0]` and fuse that one element into a +direction-specific recognizer, so every element after the first was declared and +then never honoured. + +**Breaking in two narrow ways, deliberately.** + +- A declared set with more than one direction now fires for all of its members. + `direction: ['left', 'up']` used to fire on a left swipe only. +- An EMPTY or absent `swipe` sub-object now fires for nothing. It used to fall + back to a left-swipe recognizer, so a config that declared no direction at all + silently recognized one. + +**The lenient cast is gone, and nothing replaces it (AGENTS.md #0.1).** The hook +also read a SCALAR `direction` through an `as string` cast, which the declared +array type rejects — a second, undeclared contract that let the shipped +`@example` (which passed `direction: 'left'`) appear to work when hand-tested. +That example now passes `direction: ['left']`, and no runtime path accepts a +scalar. A tree-wide scan of `packages/`, `apps/` and `examples/` for scalar call +sites — run with a firing control that planted one and found it — reported zero, +so no caller has to change for this. diff --git a/packages/mobile/src/__tests__/gesture-spec-parity.test.tsx b/packages/mobile/src/__tests__/gesture-spec-parity.test.tsx index 405d693f6d..a7cb796578 100644 --- a/packages/mobile/src/__tests__/gesture-spec-parity.test.tsx +++ b/packages/mobile/src/__tests__/gesture-spec-parity.test.tsx @@ -72,8 +72,12 @@ describe('the declared type drives recognition', () => { expect(recognizerFor({ type: 'rotate' })).toBe('rotate'); }); - it('swipe resolves per configured direction', () => { - expect(recognizerFor({ type: 'swipe', swipe: { direction: ['up'] } })).toBe('swipe-up'); + it('swipe recognizes ANY direction — the declared set is filtered, not fused into the recognizer', () => { + // It used to fuse `direction[0]` into a direction-specific recognizer, which + // is how every element after the first was declared and never honoured + // (objectui#7974). Membership of the declared set is pinned behaviourally in + // `spec-gesture-direction-set.test.tsx`, against the real recognizer. + expect(recognizerFor({ type: 'swipe', swipe: { direction: ['up'] } })).toBe('pan'); }); it('legacy configs without a type still resolve from their sub-object', () => { diff --git a/packages/mobile/src/__tests__/spec-gesture-direction-set.test.tsx b/packages/mobile/src/__tests__/spec-gesture-direction-set.test.tsx new file mode 100644 index 0000000000..852958e98a --- /dev/null +++ b/packages/mobile/src/__tests__/spec-gesture-direction-set.test.tsx @@ -0,0 +1,113 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `swipe` fires on MEMBERSHIP of the declared direction set (objectui#7974). + * + * `SwipeGestureConfig.direction` is declared `SpecSwipeDirection[]` — a set. + * The hook used to read `direction[0]` and fuse it into one direction-specific + * recognizer, so every element after the first was declared and then never + * honoured, and an empty set silently recognized a LEFT swipe. It also carried + * an `as string` cast that let a scalar `direction` through the runtime even + * though the declared type rejects it. + * + * These run against the real `useGesture` — the recognizer choice and the + * membership filter are two halves of one behaviour, and mocking the + * recognizer would pin only the half that lives in this file. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render } from '@testing-library/react'; +import type { SpecGestureConfig, SwipeGestureConfig } from '@object-ui/types'; +import { useSpecGesture } from '../useSpecGesture'; + +function Surface(props: { + config: SpecGestureConfig; + onSwipe?: (direction: string) => void; + onGesture?: (context: { type: string; direction?: string }) => void; +}) { + const ref = useSpecGesture(props); + return
; +} + +/** A touch event jsdom does not construct: the handlers read `touches[0]` and + * `changedTouches[0]`, and nothing else on the event. */ +function fireTouch(el: HTMLElement, type: 'touchstart' | 'touchend', x: number, y: number) { + const event = new Event(type, { bubbles: true }); + const list = [{ clientX: x, clientY: y }]; + Object.defineProperty(event, 'touches', { value: list }); + Object.defineProperty(event, 'changedTouches', { value: list }); + el.dispatchEvent(event); +} + +/** Drag from the centre by (dx, dy) — past any threshold these cases declare. */ +function swipe(el: HTMLElement, dx: number, dy: number) { + fireTouch(el, 'touchstart', 200, 200); + fireTouch(el, 'touchend', 200 + dx, 200 + dy); +} + +const config = (swipeConfig: SwipeGestureConfig): SpecGestureConfig => ({ + type: 'swipe', + enabled: true, + swipe: swipeConfig, +}); + +describe('swipe honours the whole declared direction set', () => { + it('fires for EVERY member, not just the first', () => { + const onSwipe = vi.fn(); + const { getByTestId } = render( + , + ); + const surface = getByTestId('surface'); + + swipe(surface, -120, 0); + expect(onSwipe, 'the first member of the declared set').toHaveBeenCalledWith('left'); + + onSwipe.mockClear(); + swipe(surface, 0, -120); + expect(onSwipe, 'a member after the first — this is what `direction[0]` dropped').toHaveBeenCalledWith('up'); + }); + + it('does not fire for a direction the set does not declare', () => { + const onSwipe = vi.fn(); + const { getByTestId } = render( + , + ); + + swipe(getByTestId('surface'), 120, 0); + expect(onSwipe, 'a right swipe is outside the declared set').not.toHaveBeenCalled(); + }); + + it('an EMPTY declared set fires for nothing — it does not default to left', () => { + const onSwipe = vi.fn(); + const { getByTestId } = render( + , + ); + + swipe(getByTestId('surface'), -120, 0); + expect(onSwipe, 'nothing is declared, so nothing is a member').not.toHaveBeenCalled(); + }); + + it('the fallback reports the SPEC gesture and the detected direction', () => { + const onGesture = vi.fn(); + const { getByTestId } = render( + , + ); + + swipe(getByTestId('surface'), 0, 120); + expect(onGesture).toHaveBeenCalledWith( + expect.objectContaining({ type: 'swipe', direction: 'down' }), + ); + }); + + it('the declared type admits no scalar — nothing re-softens what the cast used to accept', () => { + // @ts-expect-error — `direction` is `SpecSwipeDirection[]`; a scalar is the + // shape the removed `as string` cast used to let through at runtime. + const scalar: SwipeGestureConfig = { direction: 'left', threshold: 80 }; + expect(scalar.direction).toBe('left'); + }); +}); diff --git a/packages/mobile/src/useSpecGesture.ts b/packages/mobile/src/useSpecGesture.ts index 5d859d44a6..489483d9db 100644 --- a/packages/mobile/src/useSpecGesture.ts +++ b/packages/mobile/src/useSpecGesture.ts @@ -28,21 +28,16 @@ export interface UseSpecGestureOptions { onGesture?: (context: { type: string; direction?: string; scale?: number; rotation?: number }) => void; } -const SWIPE_DIRECTION_MAP: Record = { - left: 'swipe-left', - right: 'swipe-right', - up: 'swipe-up', - down: 'swipe-down', -}; - /** * `SPEC_GESTURE_TYPES` (the retired `ui/touch` vocabulary, owned by * `@object-ui/types` since objectstack#4988) → the recognizer * {@link GestureType} `useGesture` implements. Note the two sides are * different vocabularies, which is why this map exists at all: the retired * spec's `drag` and `pan` are one recognizer - * (any-direction move past the threshold); `swipe` resolves per configured - * direction, so it maps through {@link SWIPE_DIRECTION_MAP} instead. + * (any-direction move past the threshold); `swipe` declares a SET of + * directions, so it recognizes on that same any-direction move and fires only + * when the DETECTED direction is a member of the declared set — no single + * recognizer name carries that, which is why its entry below is a placeholder. * Exported for the spec-parity test. * * Before #2942 the hook never read `config.type` at all — it branched on @@ -51,7 +46,7 @@ const SWIPE_DIRECTION_MAP: Record = { * fell through to the `'tap'` initializer and fired on a tap. */ export const SPEC_GESTURE_TYPE_MAP: Record = { - swipe: 'swipe-left', // per-direction; resolved via SWIPE_DIRECTION_MAP + swipe: 'swipe-left', // placeholder; the hook recognizes any direction and filters by the declared set pinch: 'pinch', long_press: 'long-press', double_tap: 'double-tap', @@ -69,7 +64,7 @@ export const SPEC_GESTURE_TYPE_MAP: Record = { * @example * ```tsx * const ref = useSpecGesture({ - * config: { type: 'swipe', enabled: true, swipe: { direction: 'left', threshold: 80 } }, + * config: { type: 'swipe', enabled: true, swipe: { direction: ['left'], threshold: 80 } }, * onSwipe: (dir) => console.log('Swiped', dir), * }); * return
Swipe me
; @@ -103,12 +98,24 @@ export function useSpecGesture( switch (declared) { case 'swipe': { - const dir = Array.isArray(config.swipe?.direction) - ? config.swipe?.direction[0] - : (config.swipe?.direction as string | undefined); - gestureType = (dir ? SWIPE_DIRECTION_MAP[dir] : undefined) ?? 'swipe-left'; + // `SwipeGestureConfig.direction` is declared as a SET + // (`SpecSwipeDirection[]`), so recognition is the any-direction move past + // the threshold and the swipe fires only when the DETECTED direction is a + // MEMBER of that set. `direction[0]` would honour one element of a declared + // many; a scalar `direction` is rejected by the declared type and nothing + // re-admits it here (AGENTS.md #0.1). An empty or absent set declares no + // direction, so it fires for none. + const declaredDirections: readonly string[] = config.swipe?.direction ?? []; + gestureType = 'pan'; threshold = config.swipe?.threshold; - onGesture = (ctx) => (onSwipe ? onSwipe(ctx.direction ?? dir ?? 'left') : fallback({ type: 'swipe', ...ctx })); + onGesture = (ctx) => { + const detected = ctx.direction; + if (detected === undefined || !declaredDirections.includes(detected)) return; + // `type` goes LAST: the recognizer's own type travels inside `ctx` at + // runtime, and this callback reports the spec gesture, not the recognizer. + if (onSwipe) onSwipe(detected); + else fallback({ ...ctx, type: 'swipe' }); + }; break; } case 'long_press': diff --git a/scripts/__tests__/check-doc-example-types.test.ts b/scripts/__tests__/check-doc-example-types.test.ts index 84f12194df..dbe3e1f61b 100644 --- a/scripts/__tests__/check-doc-example-types.test.ts +++ b/scripts/__tests__/check-doc-example-types.test.ts @@ -402,7 +402,7 @@ describe('the real ledger', () => { // ── the card's own acceptance criterion ────────────────────────────────────── -describe('objectui#7974 — the defect this gate was filed for', () => { +describe('objectui#7974 — the defect this gate was filed for, after the repair landed', () => { const key = 'packages/mobile/src/useSpecGesture.ts useSpecGesture #1'; it('its example is IN the compiled tier — the gate reaches the block the card named', () => { @@ -410,16 +410,27 @@ describe('objectui#7974 — the defect this gate was filed for', () => { expect(census.blocks.map((b) => ledgerKey(b))).toContain(key); }); - it('the scalar `direction` the card measured is what the block still carries', () => { + it('the block now passes the ARRAY its declared type asks for, not the scalar the card measured', () => { const source = fs.readFileSync(path.join(repoRoot, 'packages/mobile/src/useSpecGesture.ts'), 'utf8'); - expect(source).toContain("direction: 'left'"); + expect(source).toContain("direction: ['left']"); + expect(source, 'the scalar the card measured').not.toContain("direction: 'left'"); }); - it('its row records TS2322 by NUMBER and names the card that owns the repair', () => { + it('its row no longer declares TS2322, and no longer names a card that owns a repair', () => { const row = UNGATED_EXAMPLES[key]; expect(row).toBeDefined(); - expect(row.codes).toContain(2322); - expect(row.card).toBe('objectui#7974'); + expect(row.codes).not.toContain(2322); + expect(row.card).toBeNull(); + }); + + it('the row SURVIVED the repair rather than being deleted — the block still returns outside a function', () => { + // The row's own instruction said to delete it when the card landed. Deleting + // it would have been an UNDECLARED FAILURE, not a green: repairing the + // example removed the TS2322 half and left the hook-body excerpt its two + // siblings in the same package are declared for. + expect(UNGATED_EXAMPLES[key].codes).toEqual([1108]); + expect(UNGATED_EXAMPLES['packages/mobile/src/useGesture.ts useGesture #1'].codes).toEqual([1108]); + expect(UNGATED_EXAMPLES['packages/mobile/src/useTouchTarget.ts useTouchTarget #1'].codes).toEqual([1108]); }); it('the row is the ONLY thing keeping this green — remove it and the block is an undeclared failure', () => { @@ -430,7 +441,7 @@ describe('objectui#7974 — the defect this gate was filed for', () => { expect(findings.map((f) => f.reason)).toEqual(['undeclared-failure']); }); - it("when that lane repairs the example the row goes STALE, so the debt cannot outlive the defect", () => { + it('when the block is made self-contained the row goes STALE, so the debt cannot outlive the defect', () => { const { findings } = judge({ results: [{ key, codes: [] }], ledger: { [key]: UNGATED_EXAMPLES[key] }, diff --git a/scripts/check-doc-example-types.mjs b/scripts/check-doc-example-types.mjs index 4230f63274..e5bcd1a611 100644 --- a/scripts/check-doc-example-types.mjs +++ b/scripts/check-doc-example-types.mjs @@ -204,13 +204,17 @@ * the empty object it is today. Each row names what the example references, so * the row goes stale the moment the example is made self-contained. * - * ⚠️ objectui#7974 is OPEN, and its row is the reason this gate ships with a - * ledger rather than a green: `packages/mobile/src/useSpecGesture.ts` still - * carries the scalar `direction` on `main`, its card is on another lane's queue - * (`domain:ui`, `pm:queue`), and this gate may not fix it. The row records - * TS2322 by number. When that lane repairs the example the row goes STALE and - * reddens on THEIR pull request, which is the hand-off working as designed, not - * a defect in it. + * ⚠️ objectui#7974 has LANDED, and the hand-off worked exactly as this paragraph + * said it would: the row recorded TS2322 by number, repairing the example turned + * the row stale, and it reddened on that lane's pull request rather than here. + * What the repair did NOT do is retire the row — its own text said "delete this + * row when that card lands", and that instruction was falsified by measurement. + * The example now passes the array `SpecSwipeDirection[]` declares, so TS2322 is + * gone; its `return` still sits outside any function, so TS1108 remains and the + * block is still a declared fragment, alongside the other two hook-body excerpts + * in `packages/mobile`. A row is RE-DERIVED when half its diagnostics are paid + * off, not deleted — deleting it would have made the block an undeclared + * failure, which is red for a different reason. * * ## The template-literal half of objectui#8258 * @@ -751,10 +755,10 @@ export const UNGATED_EXAMPLES = { 'a hook-body excerpt: its `return` sits outside any function, so the block is a fragment by shape', }, 'packages/mobile/src/useSpecGesture.ts useSpecGesture #1': { - card: 'objectui#7974', - codes: [1108, 2322], + card: null, + codes: [1108], reason: - 'the scalar `swipe.direction` this example passes is rejected by the declared `SpecSwipeDirection[]` (TS2322). objectui#7974 owns BOTH halves — the example and the lenient cast that hides it — and is on another lane. Delete this row when that card lands; the block also returns outside a function (TS1108), a hook-body excerpt', + 'a hook-body excerpt: its `return` sits outside any function, so the block is a fragment by shape', }, 'packages/mobile/src/useTouchTarget.ts useTouchTarget #1': { card: null,