Fix ink, lasso, crop and text box walk bugs

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<HTMLElement>('[data-ink-selection-bar]')!.style.display = 'none';
+    const side = chrome.querySelector<HTMLElement>('[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<HTMLElement>(`[data-ink-corner="${corner}"]`)!;
+      expect(centerHit(handle), corner).toBe(handle);
+    }
+    frame.stop();
+    chrome.remove();
+    chrome.style.cssText = '';
+    selection = { blocks: [], strokes: [] };
+  });
+});

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));
     }
   }
 }

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));

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;
   }

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<HTMLButtonElement>('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: [] });

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<File, 'type' | 'name'>): 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<typeof box> => !!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<MountedPage, 'title' | 'viewport'>): 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';
 }

app/src/features/page/layout/actions.ts

@@ -10,14 +10,17 @@ 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';
 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<PageActions, 'objectCommand' | 'objectE
   stop(): void;
 }
 
+/** Where a new text box goes: 16 units below the last focused block, or in the view's top left, clear of the title. */
+function newTextBoxPoint(parts: LayoutParts, lastFocused: BlockId | null): Point {
+  const { layer, viewport } = parts;
+  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();
+  const x = camera.scrollX / camera.zoom + 48;
+  const at = { layer, viewport, title: parts.title ?? null };
+  return { x, y: clearOfBlocks(at, x, Math.max(camera.scrollY / camera.zoom + 24, belowTitle(at)), TEXT_BOX) };
+}
+
 export function createPageLayout(parts: LayoutParts): PageLayout {
-  const { page, sync, layer, pool, flow, viewport } = parts;
+  const { page, sync, layer, pool, flow } = parts;
   const channel = viewChannel({ ...page.initial.view }, flow);
   let reading = parts.reading;
   let lastFocused: BlockId | null = null;
@@ -123,21 +145,13 @@ export function createPageLayout(parts: LayoutParts): PageLayout {
     pool.mount(id, { kind: 'start' }, 'target');
   };
 
-  /** 16 units below the last focused block, or the view's top left. */
-  const newTextBoxPoint = (): Point => {
-    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');

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<MountedPage, 'layer'>,
+  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<typeof box> => !!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<MountedPage, 'title' | 'viewport'>): 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;
+}

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, {

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({