diff --git a/app/src/features/ink/view/selectionFrame.test.tsx b/app/src/features/ink/view/selectionFrame.test.tsx index 016856b00..200e18855 100644 --- a/app/src/features/ink/view/selectionFrame.test.tsx +++ b/app/src/features/ink/view/selectionFrame.test.tsx @@ -1,11 +1,13 @@ // T3-4: the lasso's actions exist only while something is selected; with nothing selected the frame (and so every // action in it) is not displayed, which also takes it out of the accessibility tree. With a selection the actions show. import { afterEach, describe, expect, it, vi } from 'vitest'; +import { initFlags } from '../../../app/flags'; import { makeStroke } from '../geometry/fixtures'; import type { InkHost, InkSelection } from './host'; import { attachSelectionFrame, createFrameTool } from './selection'; import { chooseTool } from './state'; import type { InkSurface } from './surface'; +import './view.module.css'; let selection: InkSelection = { blocks: [], strokes: [] }; const listeners = new Set<() => void>(); @@ -159,3 +161,39 @@ describe('a text box selected alone (Beta 6, owner bugs 5 and 8)', () => { chrome.remove(); }); }); + +describe('the lasso handles on a thin selection (A2-A09-5)', () => { + it('leaves every resize handle on top, with the rewrap handle clear of them', () => { + initFlags('dev', { 'ink.handwriting': true, 'intel.handwriting': true }); + chrome.style.cssText = + 'position:absolute;left:0;top:0;width:600px;height:300px;--space-1:2px;--space-2:4px;--space-3:8px;--space-4:12px;--space-5:16px'; + document.body.append(chrome); + const line = makeStroke('line', [ + { x: 100, y: 100 }, + { x: 400, y: 110 }, + ]); + const thin = { + ...surface, + strokes: (ids: readonly string[]) => (ids.includes('line') ? [line] : []), + } as unknown as InkSurface; + selection = { blocks: [], strokes: ['line'] }; + const frame = attachSelectionFrame(host, thin); + // The bar is not under test, and unstyled it is tall enough to cover the top handles. + chrome.querySelector('[data-ink-selection-bar]')!.style.display = 'none'; + const side = chrome.querySelector('[data-ink-side]')!; + expect(side).not.toBeNull(); + const centerHit = (element: Element) => { + const box = element.getBoundingClientRect(); + return document.elementFromPoint(box.left + box.width / 2, box.top + box.height / 2); + }; + expect(centerHit(side)).toBe(side); + for (const corner of ['nw', 'ne', 'se', 'sw']) { + const handle = chrome.querySelector(`[data-ink-corner="${corner}"]`)!; + expect(centerHit(handle), corner).toBe(handle); + } + frame.stop(); + chrome.remove(); + chrome.style.cssText = ''; + selection = { blocks: [], strokes: [] }; + }); +}); diff --git a/app/src/features/ink/view/view.module.css b/app/src/features/ink/view/view.module.css index bc8a1cef9..07f227beb 100644 --- a/app/src/features/ink/view/view.module.css +++ b/app/src/features/ink/view/view.module.css @@ -165,11 +165,15 @@ inset-block-start: calc(100% + var(--space-3)); } - /* The handle on the right side of the lasso's frame: drag it to rewrap handwriting to a new width. */ + /* + * The handle on the right side of the lasso's frame: drag it to rewrap handwriting to a new width. It sits clear of + * the frame's edge, past the corner handles that are centered on it, so on a thin selection (one line of + * handwriting) it never lies on the bottom right resize handle. + */ :global(.ink-selection-frame) [data-ink-side] { position: absolute; inset-block-start: calc(50% - var(--space-5) / 2); - inset-inline-end: calc(var(--space-5) / -2); + inset-inline-end: calc(var(--space-5) * -1.5 - var(--space-1)); inline-size: var(--space-5); block-size: var(--space-5); border: 1px solid var(--color-accent-primary); @@ -216,7 +220,7 @@ :global(.ink-selection-frame) [data-ink-side]::before { inset-block: calc(-1 * var(--space-4)); - inset-inline: calc(-1 * var(--space-3)) calc(-1 * var(--space-5)); + inset-inline: 0 calc(-1 * var(--space-5)); } } } diff --git a/app/src/features/page/images/cropMode.ts b/app/src/features/page/images/cropMode.ts index dc3faa633..61fa22723 100644 --- a/app/src/features/page/images/cropMode.ts +++ b/app/src/features/page/images/cropMode.ts @@ -76,7 +76,11 @@ class CropSession { this.region.className = styles.cropWindow; this.region.setAttribute('role', 'group'); this.region.setAttribute('aria-label', t('images.cropMode.label')); - this.bright = this.region.appendChild(picture.cloneNode() as HTMLImageElement); + // The bright picture is clipped to the window, but the handles, centered on its edge, are not: a clipped handle + // loses the half of its hit area outside the window, and its center falls on the clip edge (A1-A07-2). + const clip = this.region.appendChild(document.createElement('div')); + clip.className = styles.cropClip; + this.bright = clip.appendChild(picture.cloneNode() as HTMLImageElement); this.bright.alt = ''; frame.append(this.region); const buttons = HANDLES.map((edge) => this.addHandle(edge)); diff --git a/app/src/features/page/images/images.module.css b/app/src/features/page/images/images.module.css index f504189e5..f37bf7f52 100644 --- a/app/src/features/page/images/images.module.css +++ b/app/src/features/page/images/images.module.css @@ -217,10 +217,15 @@ .cropWindow { position: absolute; - overflow: clip; outline: 2px solid var(--color-accent-clay); } + .cropClip { + position: absolute; + inset: 0; + overflow: clip; + } + .cropWindow .picture { opacity: 1; } diff --git a/app/src/features/page/images/images.test.tsx b/app/src/features/page/images/images.test.tsx index 1f8db9e07..6acd1aa7a 100644 --- a/app/src/features/page/images/images.test.tsx +++ b/app/src/features/page/images/images.test.tsx @@ -93,6 +93,18 @@ describe('image blocks', () => { expect(move.edit).toBe('moveBlock'); }); + it('crop handles take a real pointer press at their center, so a mouse or pen drag crops (A1-A07-2)', async () => { + const { handle } = await pageWithImage(); + startCrop(handle); + const buttons = [...handle.element.querySelectorAll('button[aria-label^="Crop from"]')]; + expect(buttons).toHaveLength(8); + for (const button of buttons) { + const box = button.getBoundingClientRect(); + const hit = document.elementFromPoint(box.left + box.width / 2, box.top + box.height / 2); + expect(hit === button, button.getAttribute('aria-label') ?? '').toBe(true); + } + }); + it('crop mode cancels with Escape', async () => { const { page, handle } = await pageWithImage(); const before = page.sent().length; @@ -143,6 +155,26 @@ describe('dragging an image', () => { expect(await movedFrame(page, id, before)).toMatchObject({ x: 100, y: 230 }); }); + it('moves a selected image with a finger drag on its picture, as a mouse does (A1-A07-11)', async () => { + const { page, id, handle } = await pageWithImage(); + selectOnPage({ blocks: [id], strokes: [] }); + const before = page.sent().length; + // Twelve 5 px steps go past the touch slop, where a one-finger pan would take the pointer. + dragFrom(handle.element.querySelector('img')!, 'touch', 60, 30, 12); + expect(await movedFrame(page, id, before)).toMatchObject({ x: 100, y: 230 }); + }); + + it('leaves an unselected image to a finger pan, which does not move it (A1-A07-11)', async () => { + const { page, handle } = await pageWithImage(); + const before = page.sent().length; + dragFrom(handle.element.querySelector('img')!, 'touch', 60, 30, 12); + const moves = page + .sent() + .slice(before) + .flatMap((batch) => batch.edits); + expect(moves.filter((edit) => edit.edit === 'moveBlock')).toEqual([]); + }); + it('resizes it with a finger as far as the finger went (Beta 8 walk A1-A07-8)', async () => { const { page, id } = await pageWithImage(); selectOnPage({ blocks: [id], strokes: [] }); diff --git a/app/src/features/page/images/insert.ts b/app/src/features/page/images/insert.ts index a96618305..a7846d705 100644 --- a/app/src/features/page/images/insert.ts +++ b/app/src/features/page/images/insert.ts @@ -9,6 +9,7 @@ import { announce, showToast } from '../../../ui'; import type { MountedPage } from '../mount'; import { pageSelection, selectOnPage } from '../seams/selectionStore'; import { assetTable } from './assets'; +import { belowTitle, clearOfBlocks, KEEP_BELOW } from '../layout/freeSpot'; import { offerActualSize, sizeFor, sizingFor } from '../qol/pasteSize'; import type { Sizing } from '../qol/pasteSize'; import { FREEFORM_MAX_WIDTH } from './geometry'; @@ -45,7 +46,6 @@ export function isMetafile(file: Pick): boolean { } /** What the shell can convert when page.heicImport is on. */ export const CONVERTIBLE_TYPES = ['image/heic', 'image/heif', 'image/tiff'] as const; -const KEEP_BELOW = 24; const PROGRESS_DELAY = 300; export type Placement = { kind: 'point'; x: number; y: number } | { kind: 'after'; block: BlockId | null }; @@ -136,31 +136,6 @@ export function defaultPlacement(mounted: MountedPage): Placement { return { kind: 'point', x, y: clearOfBlocks(mounted, x, Math.max(0, corner.y, belowTitle(mounted))) }; } -/** The size of a new block's footprint that must not land on a block already there (a file card, a small picture). */ -const FOOTPRINT = { w: 320, h: 96 }; - -/** `y`, or the first y below the blocks that a block at `x`, `y` would cover, so a new block never hides one. */ -function clearOfBlocks(mounted: MountedPage, x: number, y: number): number { - const boxes = mounted.layer - .blocks() - .map((block) => mounted.layer.view(block.id)?.measure()) - .filter((box): box is NonNullable => !!box && box.w > 0 && box.h > 0) - .sort((a, b) => a.y - b.y); - let top = y; - for (const box of boxes) { - const covers = box.x < x + FOOTPRINT.w && box.x + box.w > x && box.y < top + FOOTPRINT.h && box.y + box.h > top; - if (covers) top = box.y + box.h + KEEP_BELOW; - } - return top; -} - -/** The first world y under the title band, so a new block never covers the page title. */ -function belowTitle(mounted: Pick): number { - const band = mounted.title?.element.getBoundingClientRect(); - if (!band || band.height === 0) return 0; - return mounted.viewport.toWorld(band.left, band.bottom).y + KEEP_BELOW; -} - export function isFlow(page: OpenPage): boolean { return page.initial.view?.layout === 'flow'; } diff --git a/app/src/features/page/layout/actions.ts b/app/src/features/page/layout/actions.ts index 1cdc7071e..57293ed90 100644 --- a/app/src/features/page/layout/actions.ts +++ b/app/src/features/page/layout/actions.ts @@ -10,6 +10,7 @@ import type { PageBlockLayer } from '../blocks/blockLayer'; import { DISCARD_DRAFT, markDraft, markEphemeral } from '../blocks/textBlock'; import type { PagePool } from '../pool/pool'; import { byOrder } from '../readingOrder/order'; +import { readingLock } from '../qol/stores'; import { savePageView } from '../runtime'; import type { SyncQueue } from '../sync'; import type { Point } from '../viewport/camera'; @@ -17,7 +18,9 @@ import type { PageActions } from '../viewport/shown'; import { isFloating } from '../blocks/textBlock'; import styles from './layout.module.css'; import type { PageViewport } from '../viewport/viewport'; +import type { TitleBand } from '../title/title'; import type { Flow } from './flow'; +import { belowTitle, clearOfBlocks } from './freeSpot'; import { snapY } from './rules'; /** An RFC 7396 merge patch over a view: null removes a key, an object merges, anything else replaces. */ @@ -53,6 +56,9 @@ function viewChannel(initial: PageViewJson, flow: Flow) { /** A new text box goes this far below the block it follows. */ export const BELOW_GAP = 16; + +/** The size of a new text box, for finding it a free spot. */ +const TEXT_BOX = { w: 120, h: 42 }; /** Half a Normal line: a tapped caret's first line is centered on the tap. */ const HALF_LINE = 12; @@ -63,6 +69,8 @@ export interface LayoutParts { readonly pool: PagePool; readonly flow: Flow; readonly viewport: PageViewport; + /** The title band: a new text box goes under it, not on it. */ + readonly title?: TitleBand | null; readonly compact: boolean; readonly reading: boolean; } @@ -81,8 +89,22 @@ export interface PageLayout extends Omit { - const last = lastFocused ? layer.block(lastFocused) : null; - const element = lastFocused ? layer.view(lastFocused)?.element : null; - if (last?.frame?.x !== undefined && last.frame.y !== undefined && element) { - return { x: last.frame.x, y: last.frame.y + element.offsetHeight + BELOW_GAP }; - } - const camera = viewport.camera(); - return { x: camera.scrollX / camera.zoom + 48, y: camera.scrollY / camera.zoom + 24 }; - }; - const self: PageLayout = { - newTextBox: () => caretAt(newTextBoxPoint(), true), + newTextBox() { + if (readingLock.get()) return announce(t('pageExtras.lock.blocked')); + caretAt(newTextBoxPoint(parts, lastFocused), true); + }, pressEmpty(point) { - if (page.readOnly) return; + if (page.readOnly || readingLock.get()) return; if (reading || channel.get().layout === 'flow') { const last = [...layer.blocks()].reverse().find((block) => block.type === 'text' && !block.frame?.y); if (last) return void pool.mount(last.id, { kind: 'end' }, 'target'); diff --git a/app/src/features/page/layout/freeSpot.ts b/app/src/features/page/layout/freeSpot.ts new file mode 100644 index 000000000..801b33a8e --- /dev/null +++ b/app/src/features/page/layout/freeSpot.ts @@ -0,0 +1,36 @@ +// Free space on a freeform page: where a new floating block can go without covering the title or a block that is +// already there. +import type { MountedPage } from '../mount'; + +/** The gap kept between a new block and what it goes under. */ +export const KEEP_BELOW = 24; + +/** The size of a new block's footprint that must not land on a block already there (a file card, a small picture). */ +const FOOTPRINT = { w: 320, h: 96 }; + +/** `y`, or the first y below the blocks that a block at `x`, `y` would cover, so a new block never hides one. */ +export function clearOfBlocks( + mounted: Pick, + x: number, + y: number, + footprint: { w: number; h: number } = FOOTPRINT, +): number { + const boxes = mounted.layer + .blocks() + .map((block) => mounted.layer.view(block.id)?.measure()) + .filter((box): box is NonNullable => !!box && box.w > 0 && box.h > 0) + .sort((a, b) => a.y - b.y); + let top = y; + for (const box of boxes) { + const covers = box.x < x + footprint.w && box.x + box.w > x && box.y < top + footprint.h && box.y + box.h > top; + if (covers) top = box.y + box.h + KEEP_BELOW; + } + return top; +} + +/** The first world y under the title band, so a new block never covers the page title. */ +export function belowTitle(mounted: Pick): number { + const band = mounted.title?.element.getBoundingClientRect(); + if (!band || band.height === 0) return 0; + return mounted.viewport.toWorld(band.left, band.bottom).y + KEEP_BELOW; +} diff --git a/app/src/features/page/layout/layout.test.tsx b/app/src/features/page/layout/layout.test.tsx index c95ac17a2..80587ac0a 100644 --- a/app/src/features/page/layout/layout.test.tsx +++ b/app/src/features/page/layout/layout.test.tsx @@ -11,6 +11,7 @@ import type { PageFixture } from '../../../services/pages/memory'; import type { BlockJson } from '../../../services/pages/types'; import { mountPage } from '../mount'; import type { MountedPage, MountOptions } from '../mount'; +import { readingLock } from '../qol/stores'; import { textPageFixture } from '../test/fixtures'; const shown: { mounted: MountedPage; container: HTMLElement }[] = []; @@ -149,6 +150,34 @@ describe('page layout', () => { }); }); +describe('a new text box', () => { + it('goes under the title, not on it (A1-A07-5)', async () => { + const { mounted } = await open(textPageFixture('First'), 800, { + title: { text: 'A long page title', changed: null }, + }); + const band = mounted.title!.element.getBoundingClientRect(); + const before = new Set(mounted.layer.blocks().map((block) => block.id)); + mounted.layout.newTextBox(); + const added = mounted.layer.blocks().find((block) => !before.has(block.id))!; + expect(added.frame!.y!).toBeGreaterThanOrEqual(mounted.viewport.toWorld(0, band.bottom).y); + }); + + it('adds no text box while the page is locked for reading (A2-A08-2)', async () => { + const { mounted } = await open(textPageFixture('First')); + const before = mounted.layer.blocks().length; + readingLock.set(true); + try { + mounted.layout.newTextBox(); + mounted.layout.pressEmpty({ x: 300, y: 300 }); + expect(mounted.layer.blocks()).toHaveLength(before); + } finally { + readingLock.set(false); + } + mounted.layout.newTextBox(); + expect(mounted.layer.blocks()).toHaveLength(before + 1); + }); +}); + describe('the title band', () => { it('is a heading named by its text, sends typing as the page title, and Enter moves into the page', async () => { const { mounted, service, page } = await open(textPageFixture('Body text'), 800, { diff --git a/app/src/features/page/mount.ts b/app/src/features/page/mount.ts index 7b72c2919..472bb3cc7 100644 --- a/app/src/features/page/mount.ts +++ b/app/src/features/page/mount.ts @@ -340,7 +340,7 @@ export function mountPage(container: HTMLElement, page: OpenPage, options: Mount keepTextBox(frames, layer, pool, page.readOnly !== null); const title = titleBand(viewport.world, { page, sync, layer, pool }, options); const compact = options.compact ?? false; - const layout = createPageLayout({ page, sync, layer, pool, flow, viewport, compact, reading }); + const layout = createPageLayout({ page, sync, layer, pool, flow, viewport, title, compact, reading }); const chrome = createChrome({ container, viewport, layer, pool }); const keepBelow = createKeepBelow(layer, sync, pool); objects = new Objects({ diff --git a/app/src/features/page/viewport/gestures.ts b/app/src/features/page/viewport/gestures.ts index cf8a047b8..fc9dc2bda 100644 --- a/app/src/features/page/viewport/gestures.ts +++ b/app/src/features/page/viewport/gestures.ts @@ -7,7 +7,9 @@ // gesture ends. While anything holds the camera (Phase 5 does while a pen is down), nothing here moves the page. import { prefersReducedMotion } from '../../../shell/layout/motion'; import type { Point } from './camera'; +import { pageSelection } from '../seams/selectionStore'; import { requestContextMenu } from './menuRequests'; +import { activeTool } from './router'; import type { PointerToolDef, RouterContext } from './router'; import type { PageViewport } from './viewport'; @@ -35,6 +37,17 @@ interface Contact { type Mode = 'none' | 'pan' | 'pinch'; +/** + * Whether a press is on the drag body of a block that is already selected (an image). With the Select tool a + * finger there moves the block, as a mouse does. On any other block, or on empty page, a finger pans the page, and a + * tap selects. The router asks before the block's own press handler selects it, so a first drag still pans. + */ +function onSelectedBody(target: Element | null): boolean { + if (activeTool.get() !== 'select') return false; + const id = target?.closest('[data-block-drag]')?.closest('[data-block-id]')?.dataset.blockId; + return !!id && pageSelection.get().blocks.includes(id); +} + function distance(a: Point, b: Point): number { return Math.hypot(a.x - b.x, a.y - b.y); } @@ -84,7 +97,7 @@ class Gestures implements GestureTool { // drag (images/drag.ts). const target = event.target instanceof Element ? event.target : null; if (this.viewport.held() || target?.closest('[data-handle], [data-own-drag]')) return false; - if (event.pointerType === 'touch') return true; + if (event.pointerType === 'touch') return !onSelectedBody(target); if (event.pointerType === 'pen') return event.button === BARREL || (event.buttons & 2) !== 0; return event.pointerType === 'mouse' && event.button === MIDDLE; } diff --git a/tests/ui/visual/__screenshots__/win32/matrix.spec.ts/page-sampler-editing-wide-dark.png b/tests/ui/visual/__screenshots__/win32/matrix.spec.ts/page-sampler-editing-wide-dark.png index 6621cd306..b79dac1bc 100644 Binary files a/tests/ui/visual/__screenshots__/win32/matrix.spec.ts/page-sampler-editing-wide-dark.png and b/tests/ui/visual/__screenshots__/win32/matrix.spec.ts/page-sampler-editing-wide-dark.png differ diff --git a/tests/ui/visual/__screenshots__/win32/matrix.spec.ts/page-sampler-editing-wide-light.png b/tests/ui/visual/__screenshots__/win32/matrix.spec.ts/page-sampler-editing-wide-light.png index b5b2a8ca1..4628bed4c 100644 Binary files a/tests/ui/visual/__screenshots__/win32/matrix.spec.ts/page-sampler-editing-wide-light.png and b/tests/ui/visual/__screenshots__/win32/matrix.spec.ts/page-sampler-editing-wide-light.png differ