diff --git a/docs/02-data-model-events.md b/docs/02-data-model-events.md index 609b0df..15fd165 100644 --- a/docs/02-data-model-events.md +++ b/docs/02-data-model-events.md @@ -249,11 +249,19 @@ extension sorts after it — so a gap can always be subdivided, however often. Consequences worth knowing: -- Dropping a page **onto** another one clears its key: in its new level it - sorts by title until somebody drags it into place. That is predictable, and - it avoids carrying a key from a level where it meant something else. +- Choosing a **parent** clears the key: in its new level the page sorts by + title until somebody puts it in place. That is predictable, and it avoids + carrying a key from a level where it meant something else. It holds for every + way of choosing one — dropping the page onto a row, and the move menu's "in" + and "Move to…" alike ([06](06-ui-information-architecture.md)) — so the tree + does not depend on which input device moved the page. A key is written only + where a position within a level is actually chosen. - Two pages whose titles normalise identically compare equal; the slug breaks - the tie, so every client shows the same order. + the tie, so every client shows the same order. A position *between* two of + them is then not expressible as a key at all, which the move menu's up and + down have to answer for: they step past the whole run of equal keys rather + than into it. One row further than asked, and a move — the alternative is a + key the page already has, published as nothing at all. - **Open:** the order is per page, so a *level* cannot be sorted in one go, and reordering needs a signature per page moved. - **Open:** nothing cleans up the placement of a deleted page. A `31818` whose diff --git a/docs/06-ui-information-architecture.md b/docs/06-ui-information-architecture.md index a443d25..94a7110 100644 --- a/docs/06-ui-information-architecture.md +++ b/docs/06-ui-information-architecture.md @@ -192,9 +192,53 @@ suffocates in a column that narrow, so it gets `max-w-4xl`. The page itself carries **no** move action any more: a picker on the page was tried and dropped again — dragging in the tree says where a page ends up - far better than a list of slugs can. **Open:** moving therefore has no - keyboard path, and none on a touch screen either, where HTML5 drag & drop - does not fire. + far better than a list of slugs can. + + **Without a pointer that can drag (CON-15).** Dragging stays the primary + gesture, but it is not a reachable one: HTML5 drag & drop has no keyboard + path at all, and on a touch screen it does not fire even once — there, + moving a page was impossible rather than awkward. Every row therefore + carries a **move button** (`src/ui/PageMoveMenu.tsx`), a real button in the + tab order and under a finger, opening a menu with two panels: + + - the four steps every outline editor has — **up**, **down**, **in** under + the sibling above, **out** to directly behind the parent. The two that + change a page's parent name it ("Move under Handbook", "Move out of + Handbook"); up and down stay generic, because the row they pass is the + one directly above or below and pointing at it adds nothing. A step with + nowhere to go is drawn disabled rather than reporting an error after the + click — and so is one that would land the page exactly where it already + hangs, which `useMovePage` would otherwise drop without a word, leaving an + enabled entry that does nothing. Where each one lands is + `src/domain/move-tree.ts`. + - **Move to…**, the list of every page it may be filed under plus the top + level, in tree order and filterable by name. This is what dragging has no + equivalent of, deliberately: a drag can only end where the pointer can + reach, a list can name a row that is scrolled away or folded shut. Left + out are the page itself, its own subtree and the parent it already has — + the same three a drop refuses. + + A step that picks a *parent* ("in", "Move to…") writes **no** order key and + lets the new level sort by title, which is exactly what dropping onto that + row does. The tree must not look different depending on whether a mouse or + the keyboard moved the page. A key is written only where a position within a + level is genuinely being chosen: up, down and out. + + The trigger is hidden until the row is hovered or something inside it is + focused, like the "+" on the Pages heading — the tree is read far more often + than it is rearranged. A coarse pointer has no hover to reveal it with, so + `pointer-coarse` leaves it permanently visible there; without that line the + touch half of this would have shipped invisible. + + The open panel is **portalled into the body** and positioned against the + window rather than drawn inside the row, for two reasons that both bite + exactly where the menu matters most. The tree scrolls in an + `overflow-y-auto` container, which clips anything absolutely positioned + inside it: a row in the lower part of the bar would open a menu with its + lower half — the destination list — cut away. And every row is a + `draggable` element, so a press inside the menu (selecting filter text, + sliding onto an entry) would be handed to the row as the start of a drag. + The panel flips above the row when the window has no room below it. 4. **Settings** — one row to `/settings/spaces`, the entry into the settings hub (below). Stays visible even while already inside `/settings/*`, where the bar shows the hub's own nav (Profile/Spaces) instead of zones 2 and 3. diff --git a/src/domain/move-tree.test.ts b/src/domain/move-tree.test.ts new file mode 100644 index 0000000..f4effd4 --- /dev/null +++ b/src/domain/move-tree.test.ts @@ -0,0 +1,178 @@ +import { describe, expect, it } from 'vitest' +import { buildPages, buildTree, flattenTree, orderKeyOf } from './pages' +import { planMove, siblingsOf } from './move-tree' +import type { MoveDirection } from './move-tree' +import type { Page } from './pages' +import type { Revision } from './revision' + +function rev(partial: Partial & { id: string }): Revision { + return { + author: 'alice', + createdAt: 1000, + group: 'engineering', + slug: partial.id, + title: partial.id, + parentSlug: null, + order: null, + parentRevs: [], + summary: null, + content: '', + ...partial, + } +} + +/** A tree from `slug:parent` pairs, with each page's title as its own slug. */ +function tree(...spec: [slug: string, parent: string | null][]): Page[] { + return buildPages(spec.map(([slug, parent]) => rev({ id: slug, parentSlug: parent }))) +} + +/** + * The move applied, as `buildPages` would hand the result back. + * + * The re-sort is the point, not bookkeeping: `planMove` reads a level by + * filtering an already globally sorted array, which is what `buildPages` + * produces and what `space.pages` always is. A test that changed one key and + * left the array where it was would be asking the function a question it never + * gets in the app — and would answer the second move from the tree as it + * looked before the first. + */ +function applyMove(pages: Page[], slug: string, direction: MoveDirection): Page[] { + const move = planMove(pages, slug, direction) + if (!move) throw new Error(`${direction} was not available for ${slug}`) + return pages + .map((page) => + page.slug === slug ? { ...page, parentSlug: move.parentSlug, order: move.order } : page, + ) + .sort((a, b) => { + const left = orderKeyOf(a) + const right = orderKeyOf(b) + if (left !== right) return left < right ? -1 : 1 + return a.slug < b.slug ? -1 : 1 + }) +} + +/** + * The tree somebody would see after the move, indented — an order key is not + * what anybody is checking, the row it puts the page on is. + */ +function after(pages: Page[], slug: string, direction: MoveDirection): string[] { + return flattenTree(buildTree(applyMove(pages, slug, direction))).map( + (node) => `${' '.repeat(node.depth)}${node.slug}`, + ) +} + +const FLAT = tree(['a', null], ['b', null], ['c', null], ['d', null]) + +/** + * A level whose three titles all normalise to the same key, so all three sort + * equal and only the slug tells them apart (docs/02). Reachable without + * trying: a rename keeps the page's slug (`src/ui/PageEditor.tsx`), so two + * pages can end up named the same thing. + */ +const TIED = buildPages([ + rev({ id: 'a1', title: 'Setup' }), + rev({ id: 'b2', title: 'setup' }), + rev({ id: 'c3', title: 'SETUP' }), +]) + +describe('siblingsOf', () => { + it('keeps the level in the order the tree draws it', () => { + expect(siblingsOf(FLAT, null).map((page) => page.slug)).toEqual(['a', 'b', 'c', 'd']) + const nested = tree(['a', null], ['x', 'a'], ['y', 'a']) + expect(siblingsOf(nested, 'a').map((page) => page.slug)).toEqual(['x', 'y']) + }) +}) + +describe('planMove — up and down', () => { + it('swaps with the row above, and with the row below', () => { + expect(after(FLAT, 'c', 'up')).toEqual(['a', 'c', 'b', 'd']) + expect(after(FLAT, 'b', 'down')).toEqual(['a', 'c', 'b', 'd']) + }) + + it('reaches the first and the last position, not just the middle', () => { + expect(after(FLAT, 'b', 'up')).toEqual(['b', 'a', 'c', 'd']) + expect(after(FLAT, 'c', 'down')).toEqual(['a', 'b', 'd', 'c']) + }) + + it('is a round trip: up and back down leaves the order it found', () => { + expect(after(applyMove(FLAT, 'c', 'up'), 'c', 'down')).toEqual(['a', 'b', 'c', 'd']) + expect(after(applyMove(FLAT, 'b', 'down'), 'b', 'up')).toEqual(['a', 'b', 'c', 'd']) + }) + + it('has nowhere to go at the ends of a level', () => { + expect(planMove(FLAT, 'a', 'up')).toBeNull() + expect(planMove(FLAT, 'd', 'down')).toBeNull() + }) + + it('counts siblings, not rows: a subtree in between is stepped over whole', () => { + const nested = tree(['a', null], ['b', null], ['deep', 'b'], ['c', null]) + // `a` moving down passes `b` and everything hanging under it in one step + expect(after(nested, 'a', 'down')).toEqual(['b', ' deep', 'a', 'c']) + }) + + it('stays where it is when the level holds only one page', () => { + const only = tree(['a', null], ['x', 'a']) + expect(planMove(only, 'x', 'up')).toBeNull() + expect(planMove(only, 'x', 'down')).toBeNull() + }) +}) + +describe('planMove — siblings that share an order key', () => { + it('steps past the whole run rather than landing in a gap that is not there', () => { + // One row would be the better answer and is not expressible as a key: all + // three sort equal, so there is nothing between them to aim at. + expect(after(TIED, 'a1', 'down')).toEqual(['b2', 'c3', 'a1']) + }) + + it('never hands back the key the page already has', () => { + const moved = applyMove(TIED, 'a1', 'down') + const back = planMove(moved, 'a1', 'up') + // The step that publishes nothing is the one that hurts: `useMovePage` + // drops a move that changes neither parent nor key silently, so the entry + // looks enabled and does nothing at all. + expect(back?.order).not.toBe(moved.find((page) => page.slug === 'a1')!.order) + expect(after(moved, 'a1', 'up')).toEqual(['a1', 'b2', 'c3']) + }) +}) + +describe('planMove — in and out', () => { + it('files the page under the sibling above it', () => { + expect(after(FLAT, 'b', 'in')).toEqual(['a', ' b', 'c', 'd']) + }) + + it('writes no key of its own — the new level sorts it by its title', () => { + // The same answer dropping the page onto that row with a mouse gives, so + // the tree does not depend on which input device moved the page. + const nested = tree(['a', null], ['x', 'a'], ['y', 'a'], ['b', null]) + expect(planMove(nested, 'b', 'in')).toEqual({ parentSlug: 'a', order: null }) + expect(after(nested, 'b', 'in')).toEqual(['a', ' b', ' x', ' y']) + }) + + it('has no sibling above it to go in under', () => { + expect(planMove(FLAT, 'a', 'in')).toBeNull() + }) + + it('puts the page directly behind its parent, not at the end of that level', () => { + const nested = tree(['a', null], ['x', 'a'], ['b', null], ['c', null]) + expect(after(nested, 'x', 'out')).toEqual(['a', 'x', 'b', 'c']) + }) + + it('keeps the page ahead of its parent-level neighbour it was never behind', () => { + const nested = tree(['a', null], ['x', 'a'], ['y', 'a'], ['b', null]) + // both children come out one after the other and stay in their order + expect(after(applyMove(nested, 'x', 'out'), 'y', 'out')).toEqual(['a', 'y', 'x', 'b']) + }) + + it('cannot come out of the top level', () => { + expect(planMove(FLAT, 'a', 'out')).toBeNull() + }) + + it('takes the page and its own subtree along, in and out', () => { + const nested = tree(['a', null], ['b', null], ['deep', 'b'], ['deeper', 'deep']) + expect(after(nested, 'b', 'in')).toEqual(['a', ' b', ' deep', ' deeper']) + }) + + it('answers null for a slug the space does not have', () => { + expect(planMove(FLAT, 'nope', 'up')).toBeNull() + }) +}) diff --git a/src/domain/move-tree.ts b/src/domain/move-tree.ts new file mode 100644 index 0000000..d6f44e0 --- /dev/null +++ b/src/domain/move-tree.ts @@ -0,0 +1,149 @@ +import { keyBetween } from './order' +import { orderKeyOf } from './pages' +import type { Page } from './pages' + +/** + * The four moves an outliner has, expressed as placements. + * + * Dragging says where a page ends up better than any list of slugs can — which + * is why the page's own move action was dropped (docs/06). But HTML5 drag & + * drop has no keyboard path at all, and on a touch screen it does not fire + * even once: there, moving a page was simply impossible. These four are the + * vocabulary every outline editor uses for the same job, and every one of them + * is a single, predictable step somebody can repeat and watch. + * + * Kept here rather than in the sidebar because "where does this page land" is + * a question about the tree, not about a menu — and because the interesting + * part, the order key, is only checkable in isolation. + */ +export type MoveDirection = + /** swap with the sibling above */ + | 'up' + /** swap with the sibling below */ + | 'down' + /** become a child of the sibling above (sorted by title, like a drop onto it) */ + | 'in' + /** leave the parent and follow directly behind it */ + | 'out' + +/** + * Where a move puts the page: whose child it becomes, and at which key. + * + * `order: null` means "no key of its own" — the new level then sorts it by its + * title (src/domain/order.ts). That is what picking a *parent* produces, and + * it is deliberately the same answer dropping a page onto a row gives: the + * tree must not end up looking different depending on whether the page was + * moved with a mouse or with the keyboard. A key is only written where a + * position within a level is genuinely being chosen — up, down and out. + */ +export type TreeMove = { parentSlug: string | null; order: string | null } + +/** + * One level, in the order it is drawn. `pages` comes from `buildPages`, which + * has already sorted globally by order key — so filtering keeps that order and + * no second sort is needed here. + */ +export function siblingsOf(pages: Page[], parentSlug: string | null): Page[] { + return pages.filter((page) => page.parentSlug === parentSlug) +} + +/** + * Where `slug` would land, or `null` when the move has nowhere to go: the top + * row of a level cannot go up, the bottom row cannot go down, a page at the + * root cannot come further out, and a page with no sibling above it has + * nothing to move in under — and when the step would land the page exactly + * where it already hangs. The menu disables exactly those entries, so an + * unavailable move is visible before it is tried rather than reported as an + * error afterwards, or worse, silently dropped by `useMovePage`. + * + * A caller still has to check `canMoveUnder`: not for these four — none of + * them can reach into the page's own subtree, because a sibling and a parent + * are never descendants — but because `useMovePage` is the one place that + * publishes, and it checks everything it publishes. + */ +export function planMove(pages: Page[], slug: string, direction: MoveDirection): TreeMove | null { + const page = pages.find((entry) => entry.slug === slug) + if (!page) return null + + const level = siblingsOf(pages, page.parentSlug) + const index = level.findIndex((entry) => entry.slug === slug) + if (index === -1) return null + + const previous = level[index - 1] + const next = level[index + 1] + + const move = ((): TreeMove | null => { + switch (direction) { + case 'up': { + if (!previous) return null + // Between the two rows above it: the one it swaps with, and whatever is + // above *that*. Writing "the key of the row above" would collide rather + // than overtake. + const above = level[index - 2] + const ceiling = above ? orderKeyOf(above) : null + return { + parentSlug: page.parentSlug, + // A tie above it has no gap to land in (see `tied`), so the step + // goes in front of the whole run instead. Further than one row, but + // it is a move; asking for the gap would hand back the key the page + // already has. + order: tied(ceiling, orderKeyOf(previous)) + ? keyBetween(null, orderKeyOf(previous)) + : keyBetween(ceiling, orderKeyOf(previous)), + } + } + case 'down': { + if (!next) return null + const below = level[index + 2] + const floor = below ? orderKeyOf(below) : null + return { + parentSlug: page.parentSlug, + order: tied(orderKeyOf(next), floor) + ? keyBetween(orderKeyOf(next), null) + : keyBetween(orderKeyOf(next), floor), + } + } + case 'in': { + if (!previous) return null + // No key: this step chooses a parent, not a position, and dropping the + // same page onto the same row with a mouse chooses no position either. + // See `TreeMove` — one destination, one result, whatever moved it. + return { parentSlug: previous.slug, order: null } + } + case 'out': { + const parent = pages.find((entry) => entry.slug === page.parentSlug) + if (!parent) return null + // Directly behind the parent in the parent's own level, which is where + // the row visually already is — the step is outwards, not downwards. + const uncles = siblingsOf(pages, parent.parentSlug) + const after = uncles[uncles.findIndex((entry) => entry.slug === parent.slug) + 1] + return { + parentSlug: parent.parentSlug, + order: keyBetween(orderKeyOf(parent), after ? orderKeyOf(after) : null), + } + } + } + })() + + // A step that publishes the placement the page already has is worse than one + // that is greyed out: `useMovePage` drops it as a no-op without a word, so + // the entry looks enabled and does nothing at all when clicked. Reporting it + // as "nowhere to go" is the same answer the first and last row of a level + // already get. + if (move && move.parentSlug === page.parentSlug && move.order === page.order) return null + return move +} + +/** + * Whether two neighbouring keys leave no gap between them. + * + * `keyBetween` answers "behind the pair" for equal bounds, which is the right + * answer for a drop — a gesture that pointed at a place between two rows that + * do not have one. For a *step* it is not: the page is asked to overtake one + * row and would pass both, and the step back then computes the very same key + * and moves nothing. Two siblings share a key whenever their titles normalise + * identically and neither has been placed by hand (docs/02). + */ +function tied(before: string | null, after: string | null): boolean { + return before !== null && after !== null && before >= after +} diff --git a/src/ui/PageMoveMenu.test.tsx b/src/ui/PageMoveMenu.test.tsx new file mode 100644 index 0000000..2516911 --- /dev/null +++ b/src/ui/PageMoveMenu.test.tsx @@ -0,0 +1,353 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { act } from 'react' +import { createRoot } from 'react-dom/client' +import type { Root } from 'react-dom/client' +import { buildPages } from '../domain/pages' +import type { Page } from '../domain/pages' +import type { Revision } from '../domain/revision' +import type { TreeMove } from '../domain/move-tree' +import { planMove } from '../domain/move-tree' +import { PageMoveMenu } from './PageMoveMenu' +import { moveEntries, moveTargets } from './move-page' + +/** + * The menu is the only way to move a page without a pointer that can drag, so + * what is pinned here is that it is reachable at all: a real button, entries + * that say which page they move past, and the ones that lead nowhere disabled + * rather than silently failing. docs/06-ui-information-architecture.md + */ +function rev(partial: Partial & { id: string }): Revision { + return { + author: 'alice', + createdAt: 1000, + group: 'engineering', + slug: partial.id, + title: partial.id.toUpperCase(), + parentSlug: null, + order: null, + parentRevs: [], + summary: null, + content: '', + ...partial, + } +} + +function tree(...spec: [slug: string, parent: string | null][]): Page[] { + return buildPages(spec.map(([slug, parent]) => rev({ id: slug, parentSlug: parent }))) +} + +const PAGES = tree(['a', null], ['b', null], ['x', 'b']) + +/** + * Every root is torn down again afterwards. An open menu of a root left + * mounted keeps listening for the pointerdown that dismisses it, and its panel + * lives in `document.body` rather than in the host — emptying the body under + * it leaves React holding a node it can no longer remove. + */ +const mounted: Root[] = [] + +function render( + page: Page, + pages = PAGES, + onMove: (move: TreeMove) => void = vi.fn(), + busy = false, +) { + const host = document.createElement('div') + document.body.appendChild(host) + const root = createRoot(host) + mounted.push(root) + act(() => { + root.render() + }) + return host +} + +/** + * Typing into a controlled input. Assigning `value` alone never reaches React: + * it tracks the value through its own setter and reads an unchanged one as no + * change, so the native setter has to be called before the event is fired. + */ +function typeInto(field: HTMLInputElement, value: string) { + const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set! + setter.call(field, value) + field.dispatchEvent(new Event('input', { bubbles: true })) +} + +const pageOf = (slug: string, pages = PAGES) => pages.find((page) => page.slug === slug)! +const trigger = (host: HTMLElement) => host.querySelector('button')! +/** + * The open panel is portalled into `document.body` and is deliberately not + * below the row any more (`PageMoveMenu.tsx`), so everything inside it is + * looked for in the document rather than in the host the row was rendered + * into. + */ +const panel = () => document.querySelector('[role="menu"],[role="dialog"]') as HTMLElement | null +const items = () => [...document.querySelectorAll('[role="menuitem"]')] as HTMLButtonElement[] +const field = () => document.querySelector('input') as HTMLInputElement +const press = (target: Element, key: string) => + target.dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true })) + +describe('moveEntries', () => { + it('names the page a move would land under or come out of', () => { + const labels = moveEntries(PAGES, 'x').map((entry) => entry.label) + expect(labels).toContain('Move out of B') + + const deep = tree(['a', null], ['b', null], ['c', null]) + expect(moveEntries(deep, 'c').map((entry) => entry.label)).toContain('Move under B') + }) + + it('keeps the generic label for a move that has nowhere to go', () => { + const entries = moveEntries(PAGES, 'a') + const up = entries.find((entry) => entry.direction === 'up')! + expect(up.move).toBeNull() + expect(up.label).toBe('Move up') + }) +}) + +describe('PageMoveMenu', () => { + beforeEach(() => { + document.body.innerHTML = '' + }) + + afterEach(() => { + act(() => { + for (const root of mounted.splice(0)) root.unmount() + }) + document.body.innerHTML = '' + }) + + it('is a button, not a gesture — the whole point of it', () => { + const host = render(pageOf('b')) + expect(trigger(host).getAttribute('aria-label')).toBe('Move B') + expect(trigger(host).getAttribute('aria-haspopup')).toBe('menu') + expect(trigger(host).disabled).toBe(false) + }) + + it('opens on a click and offers the four steps, plus the long way', () => { + const host = render(pageOf('b')) + act(() => trigger(host).click()) + expect(items().map((item) => item.textContent)).toEqual([ + 'Move up', + 'Move down', + 'Move under A', + 'Move out', + 'Move to…', + ]) + }) + + it('disables the steps that lead nowhere instead of failing on the click', () => { + const host = render(pageOf('b')) + act(() => trigger(host).click()) + const [up, down, moveIn, out] = items() + expect(up!.disabled).toBe(false) // B can swap with A + expect(down!.disabled).toBe(true) // nothing below it + expect(moveIn!.disabled).toBe(false) + expect(out!.disabled).toBe(true) // already at the top level + }) + + it('hands the destination up and closes again', () => { + const onMove = vi.fn() + const host = render(pageOf('x'), PAGES, onMove) + act(() => trigger(host).click()) + const out = items().find((item) => item.textContent?.startsWith('Move out'))! + act(() => out.click()) + expect(onMove).toHaveBeenCalledWith({ parentSlug: null, order: expect.any(String) }) + expect(items()).toHaveLength(0) + }) + + it('hangs the open menu off the body, not off the row', () => { + // The tree scrolls in an `overflow-y-auto` container and every row is a + // drag source. Inside either, the panel is clipped at the bottom of the + // bar and a press in it starts a drag — so it is portalled out. + const host = render(pageOf('b')) + act(() => trigger(host).click()) + expect(panel()!.parentElement).toBe(document.body) + expect(host.contains(panel())).toBe(false) + }) + + it('hands up the step the domain planned, for each of the four', () => { + const level = tree(['a', null], ['b', null], ['c', null]) + for (const [label, direction] of [ + ['Move up', 'up'], + ['Move down', 'down'], + ['Move under A', 'in'], + ] as const) { + const onMove = vi.fn() + const host = render(pageOf('b', level), level, onMove) + act(() => trigger(host).click()) + act(() => items().find((item) => item.textContent === label)!.click()) + expect(onMove).toHaveBeenCalledWith(planMove(level, 'b', direction)) + act(() => mounted.pop()!.unmount()) + host.remove() + } + }) + + it('opens on ArrowDown, so the keyboard never has to guess at a click', () => { + const host = render(pageOf('b')) + act(() => press(trigger(host), 'ArrowDown')) + expect(items()).not.toHaveLength(0) + expect(document.activeElement).toBe(items()[0]) + }) + + it('closes on Escape and gives the focus back to the trigger', () => { + const host = render(pageOf('b')) + act(() => trigger(host).click()) + act(() => { + items()[0]!.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }), + ) + }) + expect(items()).toHaveLength(0) + expect(document.activeElement).toBe(trigger(host)) + }) + + it('walks the entries with the arrow keys, skipping the disabled ones', () => { + const host = render(pageOf('b')) + act(() => trigger(host).click()) + // opening already focused the first usable entry + expect(document.activeElement).toBe(items()[0]) + act(() => { + document.activeElement!.dispatchEvent( + new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true }), + ) + }) + // "Move down" and "Move out" are disabled for B, so "Move under A" is next + expect(document.activeElement?.textContent).toBe('Move under A') + }) + + it('reaches the last destination with ArrowUp out of the filter field', () => { + // The field is not a menu entry, so "one step up from where I am" has no + // meaning there — counting from index -1 stopped one short of the end. + const host = render(pageOf('x')) + act(() => trigger(host).click()) + act(() => items().find((item) => item.textContent === 'Move to…')!.click()) + expect(document.activeElement).toBe(field()) + act(() => press(field(), 'ArrowUp')) + expect(document.activeElement).toBe(items()[items().length - 1]) + }) + + it('keeps the focus when a click elsewhere closes it', () => { + const host = render(pageOf('b')) + act(() => trigger(host).click()) + const outside = document.body.appendChild(document.createElement('div')) + act(() => { + outside.dispatchEvent(new Event('pointerdown', { bubbles: true })) + }) + expect(items()).toHaveLength(0) + // Not on : the keyboard would otherwise start over at the top of + // the page after every dismissed menu. + expect(document.activeElement).toBe(trigger(host)) + }) + + it('opens at the four steps again, with the filter it was left with cleared', () => { + const host = render(pageOf('x')) + act(() => trigger(host).click()) + act(() => items().find((item) => item.textContent === 'Move to…')!.click()) + act(() => typeInto(field(), 'zz')) + act(() => trigger(host).click()) + act(() => trigger(host).click()) + expect(document.querySelector('input')).toBeNull() + expect(items().map((item) => item.textContent)).toContain('Move to…') + }) + + it('has nothing to offer for the only page in a space, and says so', () => { + const alone = tree(['a', null]) + const host = render(pageOf('a', alone), alone) + expect(trigger(host).disabled).toBe(true) + expect(trigger(host).title).toContain('nowhere to move it') + }) + + it('offers every page it may land under, and the top level when it is nested', () => { + const host = render(pageOf('x')) + act(() => trigger(host).click()) + act(() => items().find((item) => item.textContent === 'Move to…')!.click()) + expect(items().map((item) => item.textContent)).toEqual(['Top level', 'A']) + }) + + it('files the page under the destination that was picked', () => { + const onMove = vi.fn() + const host = render(pageOf('x'), PAGES, onMove) + act(() => trigger(host).click()) + act(() => items().find((item) => item.textContent === 'Move to…')!.click()) + act(() => items().find((item) => item.textContent === 'A')!.click()) + // a parent, not a position: the new level sorts it by title, exactly as a + // drop onto that row would + expect(onMove).toHaveBeenCalledWith({ parentSlug: 'a', order: null }) + }) + + it('filters the destinations by name, and says so when none is left', () => { + const host = render(pageOf('x')) + act(() => trigger(host).click()) + act(() => items().find((item) => item.textContent === 'Move to…')!.click()) + act(() => typeInto(field(), 'zz')) + expect(items()).toHaveLength(0) + expect(panel()!.textContent).toContain('no page matches') + }) + + it('closes the whole menu on Escape, not just the destination list', () => { + const host = render(pageOf('x')) + act(() => trigger(host).click()) + act(() => items().find((item) => item.textContent === 'Move to…')!.click()) + act(() => { + field().dispatchEvent( + new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }), + ) + }) + expect(panel()).toBeNull() + expect(document.activeElement).toBe(trigger(host)) + }) + + it('shuts the trigger while a move of its own is still in flight', () => { + // One signature per step, and the relay has not answered yet. A second + // click would plan the next step against the tree as it still looks, so + // the entry it offers is the one that has just been published. + const host = render(pageOf('b'), PAGES, vi.fn(), true) + expect(trigger(host).disabled).toBe(true) + act(() => trigger(host).click()) + expect(panel()).toBeNull() + }) + + it('does not hang the filter field inside a role="menu"', () => { + // `menu` admits menu items and nothing else, and a textbox in one is read + // out by some screen readers and skipped by others. The panel around the + // field is a dialog; the destinations below it are the menu. + const host = render(pageOf('x')) + act(() => trigger(host).click()) + act(() => items().find((item) => item.textContent === 'Move to…')!.click()) + const menu = document.querySelector('[role="menu"]')! + expect(menu.querySelector('input')).toBeNull() + expect(field().closest('[role="menu"]')).toBeNull() + expect(field().closest('[role="dialog"]')).not.toBeNull() + }) +}) + +describe('moveTargets', () => { + it('indents each destination by its depth in the tree', () => { + const deep = tree(['a', null], ['b', null], ['x', 'b'], ['deep', 'x']) + // `a` reads as a child of the top-level entry it is listed under, so the + // list is a tree rather than a flat run of names: top level 0, the pages + // below it one deeper than the node they hang from. + expect(moveTargets(deep, 'deep')).toEqual([ + { slug: null, title: 'Top level', depth: 0 }, + { slug: 'a', title: 'A', depth: 1 }, + { slug: 'b', title: 'B', depth: 1 }, + ]) + }) + + it('leaves out the page itself, its own subtree and the parent it has', () => { + const deep = tree(['a', null], ['b', null], ['x', 'b'], ['deep', 'x']) + // for `b`: not itself, not x or deep (its subtree), and it has no parent + expect(moveTargets(deep, 'b').map((target) => target.slug)).toEqual(['a']) + }) + + it('offers the top level only to a page that is not already there', () => { + expect(moveTargets(PAGES, 'x')[0]).toMatchObject({ slug: null, title: 'Top level' }) + expect(moveTargets(PAGES, 'a').some((target) => target.slug === null)).toBe(false) + }) + + it('has nothing to offer when the space holds a single page', () => { + const alone = tree(['a', null]) + expect(moveTargets(alone, 'a')).toEqual([]) + }) +}) diff --git a/src/ui/PageMoveMenu.tsx b/src/ui/PageMoveMenu.tsx new file mode 100644 index 0000000..3545439 --- /dev/null +++ b/src/ui/PageMoveMenu.tsx @@ -0,0 +1,345 @@ +import { useCallback, useEffect, useId, useRef, useState } from 'react' +import { createPortal } from 'react-dom' +import type { TreeMove } from '../domain/move-tree' +import type { Page } from '../domain/pages' +import { canMoveSomewhere, moveEntries, moveTargets } from './move-page' +import { INPUT } from './controls' +import { MoveIcon, PageIcon } from './icons' + +/** + * The keyboard's and the touch screen's way of moving a page. + * + * Dragging in the tree is the better gesture and stays the primary one — it + * says where a page ends up better than any list can (docs/06). What it is + * not is reachable: HTML5 drag & drop has no keyboard path, and on a touch + * screen it does not fire at all, so moving a page there was not merely + * awkward, it was impossible. + * + * One menu answers both. Its trigger is a real button, so it is in the tab + * order and under a finger. It has two panels: the four steps an outline + * editor has, for a neighbour, and a list of every page the move may land + * under, for the other end of the wiki. Reimplementing the drag on pointer + * events would have given touch a gesture and the keyboard still nothing. + * + * The trigger is hidden until the row is hovered or something inside it is + * focused — the tree is read far more often than it is rearranged, the same + * reasoning as the "+" on the Pages heading. On a coarse pointer there is no + * hover to reveal it with, so there it is simply always visible. + */ + +/** `w-60`, as a number: the panel is positioned by hand, not laid out. */ +const MENU_WIDTH = 240 +/** Between the row and the panel. */ +const GAP = 4 +/** Never flush against the edge of the window. */ +const EDGE = 8 +/** + * Below this the panel is not worth opening downwards: the filter field and + * the first destinations have to be visible without scrolling, or the list + * that is the point of the menu is the first thing to be cut off. + */ +const ROOM = 220 + +/** Where the panel goes, in viewport coordinates. */ +type Anchor = { left: number; top?: number; bottom?: number; maxHeight: number } + +export function PageMoveMenu({ + page, + pages, + busy, + onMove, +}: { + page: Page + pages: Page[] + busy: boolean + onMove: (move: TreeMove) => void +}) { + const [open, setOpen] = useState(false) + /** the four steps, or the list of destinations */ + const [picking, setPicking] = useState(false) + const [filter, setFilter] = useState('') + const [anchor, setAnchor] = useState(null) + const triggerRef = useRef(null) + const menuRef = useRef(null) + const menuId = useId() + + // Only while the menu is open. Both walk the whole tree, and there is one of + // these per row: computing them for a shut menu was the sidebar re-rendering + // the entire wiki twice per page on every event the relay pushed. What the + // closed trigger needs is one bit, and `canMoveSomewhere` is the cheap way + // to it. + const available = canMoveSomewhere(pages, page.slug) + const entries = open ? moveEntries(pages, page.slug) : [] + const targets = open ? moveTargets(pages, page.slug) : [] + + const needle = filter.trim().toLowerCase() + const shown = needle ? targets.filter((t) => t.title.toLowerCase().includes(needle)) : targets + + /** + * The panel hangs under the trigger, but it is not *inside* it: see the + * portal below. So it is measured off the trigger's box and flipped above + * the row when the window has no room below — the rows near the bottom of + * the tree are the ones a long destination list matters most for. + */ + // Both of these read refs and call setters only, so they are stable for the + // life of the row — which is what lets the window listeners below hold on to + // one copy instead of being torn down and rebuilt on every keystroke in the + // filter field. + const anchorNow = useCallback((): Anchor => { + const rect = triggerRef.current?.getBoundingClientRect() + if (!rect) return { left: EDGE, top: EDGE, maxHeight: ROOM } + const below = window.innerHeight - rect.bottom - GAP - EDGE + const above = rect.top - GAP - EDGE + const flip = below < ROOM && above > below + return { + // Right-aligned with the trigger, pulled back in when that would hang + // the panel off either edge of the window. + left: Math.max( + EDGE, + Math.min(rect.right - MENU_WIDTH, window.innerWidth - MENU_WIDTH - EDGE), + ), + ...(flip ? { bottom: window.innerHeight - rect.top + GAP } : { top: rect.bottom + GAP }), + maxHeight: Math.max(ROOM, flip ? above : below), + } + }, []) + + const close = useCallback((restoreFocus: boolean) => { + setOpen(false) + setPicking(false) + setFilter('') + // Only when the focus is still in the menu that is going away — otherwise + // it lands on and the keyboard starts over at the top of the page. + // A click elsewhere brings its own focus and has to keep it. + if (restoreFocus) triggerRef.current?.focus() + }, []) + + // Measuring in the handler rather than in a layout effect: the panel is then + // placed in the same render that opens it, so it never paints in a corner + // first, and the effect that moves the focus into it finds it already there. + const openMenu = () => { + setAnchor(anchorNow()) + setPicking(false) + setFilter('') + setOpen(true) + } + + const send = (move: TreeMove) => { + close(true) + onMove(move) + } + + // Opening puts the focus inside: on the first usable entry, or on the filter + // field once the list is up. A menu that opens with nothing focused is one + // the keyboard has to find its way into first. + useEffect(() => { + if (!open) return + const field = menuRef.current?.querySelector('input') + if (field) { + field.focus() + return + } + menuRef.current?.querySelector('button:not([disabled])')?.focus() + }, [open, picking]) + + // A panel positioned against the window has to be told when the window + // moves under it. `capture`, because the tree scrolls in a container of its + // own and a scroll event does not bubble out of it. + useEffect(() => { + if (!open) return + const follow = () => setAnchor(anchorNow()) + window.addEventListener('resize', follow) + window.addEventListener('scroll', follow, true) + return () => { + window.removeEventListener('resize', follow) + window.removeEventListener('scroll', follow, true) + } + }, [open, anchorNow]) + + // Clicking anywhere else closes it. `pointerdown` rather than `click`, so it + // is gone before whatever was clicked reacts — otherwise a click on another + // row's trigger closes this menu and opens nothing. + useEffect(() => { + if (!open) return + const onPointerDown = (event: PointerEvent) => { + const target = event.target as Node + if (menuRef.current?.contains(target) || triggerRef.current?.contains(target)) return + close(menuRef.current?.contains(document.activeElement) ?? false) + } + window.addEventListener('pointerdown', onPointerDown) + return () => window.removeEventListener('pointerdown', onPointerDown) + }, [open, close]) + + const onMenuKeyDown = (event: React.KeyboardEvent) => { + if (event.key === 'Escape') { + // Not swallowed by the sidebar or a parent: this is the innermost thing + // Escape can mean while the menu is up. + event.stopPropagation() + close(true) + return + } + if (event.key !== 'ArrowDown' && event.key !== 'ArrowUp') return + event.preventDefault() + const items = [ + ...(menuRef.current?.querySelectorAll('button[role="menuitem"]') ?? []), + ].filter((item) => !item.disabled) + if (items.length === 0) return + const from = items.indexOf(document.activeElement as HTMLButtonElement) + const step = event.key === 'ArrowDown' ? 1 : -1 + // Wrapping: a four-entry menu with two of them disabled is one keypress + // from either end whichever way it wraps. The filter field is not one of + // the entries, so from there the two keys mean the first and the last — + // counting a step from "index -1" would land one short of the end. + const to = + from === -1 + ? step === 1 + ? 0 + : items.length - 1 + : (from + step + items.length) % items.length + items[to]!.focus() + } + + const itemClass = + 'flex h-8 w-full items-center gap-2 rounded-md px-2.5 text-left text-sm text-fg-muted ' + + 'hover:bg-surface-hover hover:text-fg focus-visible:bg-surface-hover focus-visible:text-fg ' + + 'disabled:pointer-events-none disabled:opacity-40' + + const panelClass = + 'z-30 rounded-lg border border-line bg-surface-2 p-1 shadow-lg overflow-y-auto scroll-slim' + const panelStyle = { + position: 'fixed' as const, + width: MENU_WIDTH, + left: anchor?.left, + top: anchor?.top, + bottom: anchor?.bottom, + maxHeight: anchor?.maxHeight, + } + + const panel = picking ? ( + // Not `role="menu"` while the filter is in it: a menu may only contain + // menu items, and a textbox inside one is read out by some screen readers + // and skipped by others. A dialog holding a field and a menu is the same + // thing to look at and an honest description of it. + + ) : ( + {open ? ( @@ -537,6 +575,7 @@ function TreeBranch({ forcedOpen={forcedOpen} onToggle={onToggle} dnd={dnd} + moves={moves} /> ) : null} diff --git a/src/ui/move-page.ts b/src/ui/move-page.ts index 315ffba..7303f56 100644 --- a/src/ui/move-page.ts +++ b/src/ui/move-page.ts @@ -2,7 +2,9 @@ import { useState } from 'react' import { classifyRejection } from '../nostr/client' import { publishPlacement } from '../nostr/publish-placement' import { useSession } from '../session/session' -import { canMoveUnder } from '../domain/pages' +import { buildTree, canMoveUnder, descendantSlugs, flattenTree } from '../domain/pages' +import { planMove } from '../domain/move-tree' +import type { MoveDirection, TreeMove } from '../domain/move-tree' import type { Page } from '../domain/pages' /** @@ -26,10 +28,12 @@ export type MovePage = { } /** - * Moving a page. Drag & drop in the sidebar is the only gesture that gets - * here, but the rules for it — signed in, no move into one's own subtree, the - * relay's literal reason on a rejection — are worth keeping out of the tree - * rendering. + * Moving a page. Both ways in end up here — dragging in the sidebar and the + * move menu (`src/ui/PageMoveMenu.tsx`), which is the keyboard's and the touch + * screen's path to the same placement. The rules — signed in, no move into + * one's own subtree, the relay's literal reason on a rejection — are worth + * keeping out of the tree rendering, and there must be exactly one copy of + * them however the move was asked for. */ export function useMovePage(relayUrl: string, groupId: string, pages: Page[]): MovePage { const { session, ensureSamePubkey } = useSession() @@ -95,3 +99,101 @@ export function useMovePage(relayUrl: string, groupId: string, pages: Page[]): M return { move, busySlug, error, setError, signedIn: session.status === 'signed-in' } } + +/** + * The four entries of the move menu, with the names of the pages they would + * move past — so the menu reads as what will happen rather than as four bare + * directions. A move that has nowhere to go keeps its generic label: there is + * no page to name, and the entry is drawn disabled anyway. + * + * Here rather than in `PageMoveMenu.tsx` because these are words, not a + * component, and next to `useMovePage` because the two answer the same question + * from opposite ends: this one what a move would do, that one what the relay + * said when it was done. The arithmetic is in `src/domain/move-tree.ts`. + */ +const MOVE_LABEL: Record = { + up: 'Move up', + down: 'Move down', + in: 'Move in', + out: 'Move out', +} + +const MOVE_ORDER: MoveDirection[] = ['up', 'down', 'in', 'out'] + +export type MoveEntry = { + direction: MoveDirection + label: string + /** null when the move has nowhere to go */ + move: TreeMove | null +} + +export function moveEntries(pages: Page[], slug: string): MoveEntry[] { + const page = pages.find((entry) => entry.slug === slug) ?? null + const parent = page?.parentSlug + ? (pages.find((entry) => entry.slug === page.parentSlug) ?? null) + : null + + return MOVE_ORDER.map((direction) => { + const move = planMove(pages, slug, direction) + let label = MOVE_LABEL[direction] + if (move && direction === 'in') { + const into = pages.find((entry) => entry.slug === move.parentSlug) + if (into) label = `Move under ${into.title}` + } + if (move && direction === 'out' && parent) label = `Move out of ${parent.title}` + return { direction, label, move } + }) +} + +/** + * Whether the page has anywhere at all to go — the one thing about a move the + * menu needs while it is still closed, for the state of its trigger. + * + * Answering it by building the entries and the target list is what it looked + * like it should be, and it costs `flattenTree` plus a `descendantSlugs` walk + * per candidate parent — O(n²) — for every row of the tree, on every snapshot + * the relay pushes, for a menu that is shut. This is the same answer in one + * pass: a page with a parent can always leave it (out, or up to the top level + * when that parent has been deleted), and a page at the top level can move + * wherever any page outside its own subtree is — that page is either a sibling + * it can step past or a parent it can be filed under. + */ +export function canMoveSomewhere(pages: Page[], slug: string): boolean { + const page = pages.find((entry) => entry.slug === slug) + if (!page) return false + if (page.parentSlug !== null) return true + const own = descendantSlugs(pages, slug) + return pages.some((entry) => entry.slug !== slug && !own.has(entry.slug)) +} + +/** A page the move menu offers as a new parent, at its depth in the tree. */ +export type MoveTarget = { slug: string | null; title: string; depth: number } + +/** + * Every page `slug` may be filed under, plus the top level, in tree order. + * + * The four steps reach a neighbour; this reaches the other end of the wiki, + * which with a mouse is one drag and without one would otherwise be a dozen + * repeats of "move down". Dragging has no equivalent of it — that is the point, + * not an oversight: a drag can only end somewhere the pointer can get to, and a + * list can name a row that is scrolled away or folded shut. + * + * What is left out is left out for the same reasons a drop is refused rather + * than reported as an error afterwards: the page itself, its own subtree (the + * branch would point into itself and drop out of the tree), and the parent it + * already has, because filing it there publishes an event that changes nothing. + */ +export function moveTargets(pages: Page[], slug: string): MoveTarget[] { + const page = pages.find((entry) => entry.slug === slug) + if (!page) return [] + + const targets: MoveTarget[] = [] + if (page.parentSlug !== null) targets.push({ slug: null, title: 'Top level', depth: 0 }) + + for (const node of flattenTree(buildTree(pages))) { + if (node.slug === slug || node.slug === page.parentSlug) continue + if (!canMoveUnder(pages, slug, node.slug)) continue + targets.push({ slug: node.slug, title: node.title, depth: node.depth + 1 }) + } + return targets +}