From 02cbfd5a017533356e262e9796eb9c864aeceffb Mon Sep 17 00:00:00 2001 From: xarmian Date: Wed, 30 Sep 2026 16:16:51 -0400 Subject: [PATCH] =?UTF-8?q?fix(web):=2044px=20touch=20targets=20at=20phone?= =?UTF-8?q?=20width:=20strip,=20card=20=E2=8B=AF,=20status=20chip=20(TASK-?= =?UTF-8?q?2244)=20(#1674)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(web): collection strip controls are 44x44 at phone width (TASK-2244 U-a) Dave's ruling (day 83, option 1): at <=768px every control in the collection strip grows visibly to 44x44. A transparent hit area would overlap the neighbour 4px away. Desktop is unchanged, and the e2e pins the base sizes. Co-Authored-By: Claude * fix(web): the mobile view chip drops its visible View: prefix so the strip fits one row (TASK-2244 U-a) The lead's ruling on the 360 checkpoint (option B). The aria-label 'Change view' is unchanged. The e2e pins one row at 390 (list and board) and at 360 (board). It was red on the previous commit's binary at 390 list and 360 board, and it is green here. Co-Authored-By: Claude * fix(web): a mobile card has one 44x44 ⋯ carrying star, copy and reorder (TASK-2244 U-b) Dave's ruling (day 83, option 3). At <=768px the card's star, copy-ref and ⋮ could not each reach 44x44 without overlapping. One ⋯ replaces them on every ItemCard host, including the starred, tags and roles pages, which have no reorder. Star and Copy item ID are its first entries. A starred card shows a passive ★, which is not a control. Desktop is unchanged, and the e2e pins its sizes. Claude-Session: https://claude.ai/code/session_01WiJ1kQCbNUdFRyq2FEi4tL * fix(web): the card status chip gets an invisible 44x44 touch area that spares the tags (TASK-2244 U-c) Dave's ruling (day 83): the status chip gets an invisible ::after extender at <=768px. It grows upward from the chip, into the title (the card link), because a centred one landed 20 of the first tag's 40 sampled points on the chip. The e2e probes both sides with elementFromPoint: the 44x44 area hits the chip, and no point of a tag or of the card ⋯ lands on it. The card legs now seed a collection of their own, because the shared Tasks lane put seeded cards out of view. Claude-Session: https://claude.ai/code/session_01WiJ1kQCbNUdFRyq2FEi4tL * test(web): desktop legs pin only CSS-fixed sizes (TASK-2244) CI failed the desktop legs on text- and glyph-sized widths, which depend on the machine's fonts (view 77.6 vs 75, + New 101.1 vs 93.8, ⚡ 34.4 vs 41, ⋮ 18 vs 8, star 11.9 vs 13). Pinned now: every strip height (28), the three icon buttons' widths (30), and the copy button (22x22). The glyph-sized card controls must stay below 44. Run with the phone rule in force, the same assertions fail on every control. Claude-Session: https://claude.ai/code/session_01WiJ1kQCbNUdFRyq2FEi4tL * fix(web): the phone realtime dot is a badge on the collection menu button, so the strip is one row at 360 in both views (TASK-2244, Dave's gate) --------- Co-authored-by: Claude --- .../task-2244-mobile-touch-targets.spec.ts | 441 ++++++++++++++++++ .../ItemActionsMenu.card.svelte.test.ts | 92 ++++ .../collections/ItemActionsMenu.svelte | 84 +++- .../components/collections/ItemCard.svelte | 90 +++- .../[workspace]/[collection]/+page.svelte | 64 ++- 5 files changed, 729 insertions(+), 42 deletions(-) create mode 100644 web/e2e/task-2244-mobile-touch-targets.spec.ts create mode 100644 web/src/lib/components/collections/ItemActionsMenu.card.svelte.test.ts diff --git a/web/e2e/task-2244-mobile-touch-targets.spec.ts b/web/e2e/task-2244-mobile-touch-targets.spec.ts new file mode 100644 index 000000000..201b666f3 --- /dev/null +++ b/web/e2e/task-2244-mobile-touch-targets.spec.ts @@ -0,0 +1,441 @@ +import { test, expect } from './fixtures'; +import { browserLogin } from './lib/collab-helpers'; +import type { APIRequestContext, Locator, Page } from '@playwright/test'; +import type { SuiteFixture } from './fixtures'; + +/** + * TASK-2244 — touch targets at phone width (Dave's ruling, day 83: options 1+3). + * + * The collection control strip grows VISIBLY to 44x44 at <=768px, so its proof is + * the element box (boundingBox). Desktop is unchanged, pinned to the sizes + * measured on the base commit, so a mobile rule that leaks past its media query + * goes red here rather than in a screenshot nobody compares. + */ + +const MIN = 44; + +function strip(page: Page) { + return page.locator('.header-actions'); +} + +/** Every interactive control the strip renders, by the name a user would see. */ +function stripControls(page: Page, isMobile: boolean): Array<[string, Locator]> { + const s = strip(page); + return [ + [ + 'view', + isMobile ? s.locator('button.view-chip') : s.locator('button.view-dd-trigger'), + ], + ['sort', s.getByRole('button', { name: 'Sort items' })], + ['filters', s.getByRole('button', { name: 'Toggle filters' })], + ['quick actions', s.locator('.quick-actions-menu > .trigger-btn')], + ['collection menu', s.getByRole('button', { name: 'Collection menu' })], + ['new', s.locator('button.new-btn')], + ]; +} + +async function sizes(page: Page, isMobile: boolean) { + const out: Record = {}; + for (const [name, loc] of stripControls(page, isMobile)) { + await expect(loc, `${name} control is rendered`).toHaveCount(1); + const box = await loc.boundingBox(); + expect(box, `${name} has a box`).not.toBeNull(); + out[name] = { w: Math.round(box!.width * 10) / 10, h: Math.round(box!.height * 10) / 10 }; + } + return out; +} + +for (const view of ['list', 'board'] as const) { + test(`TASK-2244: every strip control is at least 44x44 on mobile (${view})`, async ({ + page, + fixture, + }, testInfo) => { + test.skip(testInfo.project.name !== 'mobile-chromium', 'the ruling is <=768px'); + + await browserLogin(page); + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/tasks?view=${view}`); + await expect(strip(page)).toBeVisible(); + + const measured = await sizes(page, true); + for (const [name, { w, h }] of Object.entries(measured)) { + expect.soft(w, `${name} width (${w}x${h})`).toBeGreaterThanOrEqual(MIN); + expect.soft(h, `${name} height (${w}x${h})`).toBeGreaterThanOrEqual(MIN); + } + }); +} + +/** + * The strip stays ONE row where it fits (the lead's ruling on the 360 checkpoint, + * option B: the chip drops its visible "View:" prefix). A future control or a + * longer label would otherwise re-wrap it silently, since every size assertion + * above still passes on two rows. The realtime dot is a badge on the collection + * ⋯ button's corner, so it can never wrap onto a row of its own: list at 360 used + * to wrap it alone, which Dave's gate called awkward, and is now held to one row + * too. + */ +const ONE_ROW: Array<{ width: number; view: 'list' | 'board'; withDot: boolean }> = [ + { width: 390, view: 'list', withDot: true }, + { width: 390, view: 'board', withDot: true }, + { width: 360, view: 'board', withDot: true }, + { width: 360, view: 'list', withDot: true }, +]; + +for (const { width, view, withDot } of ONE_ROW) { + test(`TASK-2244: the strip is one row at ${width}px (${view})`, async ({ page, fixture }, testInfo) => { + test.skip(testInfo.project.name !== 'mobile-chromium', 'phone widths'); + + await page.setViewportSize({ width, height: 844 }); + await browserLogin(page); + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/tasks?view=${view}`); + await expect(strip(page)).toBeVisible(); + + // Same row = each control's vertical centre lies inside the first + // control's span. Not equal tops: on one row, sub-pixel layout moves a + // top by 1px (measured on base, 61 vs 60), and a wrap moves it by a row. + const boxes = []; + for (const [name, loc] of stripControls(page, true)) { + const box = await loc.boundingBox(); + expect(box, `${name} has a box`).not.toBeNull(); + boxes.push({ name, top: box!.y, bottom: box!.y + box!.height, mid: box!.y + box!.height / 2 }); + } + const rowTop = boxes[0].top; + for (const b of boxes) { + expect(b.mid, `${b.name} is on the first row`).toBeGreaterThan(boxes[0].top); + expect(b.mid, `${b.name} is on the first row`).toBeLessThan(boxes[0].bottom); + } + + if (withDot) { + // The dot rides the collection ⋯ button's corner: its box lies inside + // the button's, so it adds no flex item and cannot wrap. + // Located by its status role, which the old flex-item placement had + // too, so this leg fails on that layout for the wrap, not the selector. + const dot = await strip(page).locator('[role="status"][aria-label^="Live updates"] .sse-state-dot').boundingBox(); + const menu = await page.getByRole('button', { name: 'Collection menu' }).boundingBox(); + expect(dot, 'the realtime dot has a box').not.toBeNull(); + expect(menu, 'the collection menu button has a box').not.toBeNull(); + expect(dot!.y, 'the realtime dot shares the row').toBeLessThan(rowTop + MIN); + expect(dot!.x, 'the dot sits on the menu button').toBeGreaterThanOrEqual(menu!.x); + expect(dot!.x + dot!.width, 'the dot sits on the menu button').toBeLessThanOrEqual(menu!.x + menu!.width); + expect(dot!.y, 'the dot sits on the menu button').toBeGreaterThanOrEqual(menu!.y); + expect(dot!.y + dot!.height, 'the dot sits on the menu button').toBeLessThanOrEqual(menu!.y + menu!.height); + // It keeps its status region and label. + await expect(strip(page).locator('[role="status"][aria-label^="Live updates: "]')).toHaveCount(1); + } + }); +} + +/** + * Desktop is unchanged. Pinned: what the CSS fixes. Every strip control is 28px + * tall, and sort, filters and the collection menu are 30px wide. A text-sized + * width (the view label, "+ New Task", the ⚡ trigger) depends on the machine's + * fonts: CI measured 77.6 / 101.1 / 34.4 where this box measured 75 / 93.8 / 41, + * so those are not pinned. The height is the leak signal, since the phone rule + * makes every control 44 tall. + */ +const DESKTOP_FIXED: Record = { + view: { h: 28 }, + sort: { w: 30, h: 28 }, + filters: { w: 30, h: 28 }, + 'quick actions': { h: 28 }, + 'collection menu': { w: 30, h: 28 }, + new: { h: 28 }, +}; + +for (const view of ['list', 'board'] as const) { + test(`TASK-2244: desktop strip sizes are unchanged (${view})`, async ({ page, fixture }, testInfo) => { + test.skip(testInfo.project.name !== 'desktop-chromium', 'the desktop leg'); + + await browserLogin(page); + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/tasks?view=${view}`); + await expect(strip(page)).toBeVisible(); + + const measured = await sizes(page, false); + for (const [name, want] of Object.entries(DESKTOP_FIXED)) { + expect.soft(measured[name].h, `${name} height`).toBe(want.h); + if (want.w !== undefined) expect.soft(measured[name].w, `${name} width`).toBe(want.w); + } + }); +} + +// ── U-b: the card's controls ──────────────────────────────────────────────── + +function authHeaders(fixture: SuiteFixture) { + return { Authorization: `Bearer ${fixture.apiToken}`, 'Content-Type': 'application/json' }; +} + +/** + * Three open items in one board lane, so the middle one has every reorder entry, + * in a collection of the test's OWN. The shared Tasks lane collects every other + * leg's items (and a repeat's), which put a seeded card out of view and tied its + * sort_order with strangers'. + */ +async function seedLane(request: APIRequestContext, fixture: SuiteFixture, tag: string) { + const stamp = `${Date.now()}${Math.floor(Math.random() * 1000)}`; + const schema = JSON.stringify({ + fields: [ + { key: 'status', label: 'Status', type: 'select', options: ['open', 'done'], default: 'open', terminal_options: ['done'] }, + ], + }); + const coll = await request.post(`/api/v1/workspaces/${fixture.workspaceSlug}/collections`, { + headers: authHeaders(fixture), + data: { name: `T2244 ${tag} ${stamp}`, prefix: `TT${stamp.slice(-5)}`, schema }, + }); + expect(coll.ok(), await coll.text()).toBeTruthy(); + const collSlug = (await coll.json()).slug as string; + + const titles = ['a', 'b', 'c'].map((x) => `T2244 ${tag} ${x} ${stamp}`); + const slugs: string[] = []; + for (const title of titles) { + const res = await request.post(`/api/v1/workspaces/${fixture.workspaceSlug}/collections/${collSlug}/items`, { + headers: authHeaders(fixture), + data: { title, fields: JSON.stringify({ status: 'open' }), content: '' }, + }); + expect(res.ok(), await res.text()).toBeTruthy(); + slugs.push((await res.json()).slug); + } + // Fresh items tie on sort_order, and a tie renders in no fixed order, so the + // lane order is set explicitly: a, b, c. + for (const [i, slug] of slugs.entries()) { + const res = await request.patch(`/api/v1/workspaces/${fixture.workspaceSlug}/items/${slug}`, { + headers: authHeaders(fixture), + data: { sort_order: (i + 1) * 10 }, + }); + expect(res.ok(), await res.text()).toBeTruthy(); + } + return { titles, slugs, collSlug }; +} + +function card(page: Page, title: string) { + return page.locator('.item-card').filter({ has: page.locator('.card-title', { hasText: title }) }); +} + +async function isStarred(request: APIRequestContext, fixture: SuiteFixture, slug: string) { + const res = await request.get(`/api/v1/workspaces/${fixture.workspaceSlug}/items/${slug}/star`, { + headers: authHeaders(fixture), + }); + expect(res.ok(), await res.text()).toBeTruthy(); + return (await res.json()).starred as boolean; +} + +for (const view of ['list', 'board'] as const) { + test(`TASK-2244: a mobile card has ONE 44x44 ⋯ and no star, copy or ⋮ (${view})`, async ({ + page, + fixture, + request, + }, testInfo) => { + test.skip(testInfo.project.name !== 'mobile-chromium', 'the ruling is <=768px'); + const { titles, collSlug } = await seedLane(request, fixture, `shape-${view}`); + + await browserLogin(page); + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/${collSlug}?view=${view}`); + const c = card(page, titles[1]); + await expect(c).toBeVisible(); + + await expect(c.locator('.star-btn'), 'no star button on mobile').toHaveCount(0); + await expect(c.locator('.copy-ref-btn'), 'no copy button on mobile').toHaveCount(0); + await expect(c.locator('.iam-trigger:not(.card)'), 'no reorder ⋮ on mobile').toHaveCount(0); + const more = c.locator('.iam-trigger.card'); + await expect(more).toHaveCount(1); + const box = await more.boundingBox(); + expect(box).not.toBeNull(); + expect(box!.width, `⋯ width (${box!.width}x${box!.height})`).toBeGreaterThanOrEqual(MIN); + expect(box!.height, `⋯ height (${box!.width}x${box!.height})`).toBeGreaterThanOrEqual(MIN); + }); +} + +test('TASK-2244: the mobile card ⋯ stars, copies the ID and reorders (board)', async ({ + page, + fixture, + request, +}, testInfo) => { + test.skip(testInfo.project.name !== 'mobile-chromium', 'the ruling is <=768px'); + const { titles, slugs, collSlug } = await seedLane(request, fixture, 'wire'); + + await page.context().grantPermissions(['clipboard-read', 'clipboard-write']); + await browserLogin(page); + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/${collSlug}?view=board`); + const c = card(page, titles[2]); + await expect(c).toBeVisible(); + const more = c.locator('.iam-trigger.card'); + + // Star: the server records it, and the card shows the passive mark. + expect(await isStarred(request, fixture, slugs[2]), 'precondition: not starred').toBe(false); + await expect(c.locator('.starred-mark')).toHaveCount(0); + await more.tap(); + await page.getByRole('menuitem', { name: 'Star', exact: true }).tap(); + await expect.poll(() => isStarred(request, fixture, slugs[2])).toBe(true); + await expect(c.locator('.starred-mark'), 'a starred card shows the passive mark').toBeVisible(); + await expect(page, 'the tap did not navigate').toHaveURL(new RegExp(`/${collSlug}\\?view=board`)); + + // Copy: the clipboard receives the ref the card shows. + const ref = (await c.locator('.item-ref').innerText()).trim(); + await page.evaluate(() => navigator.clipboard.writeText('T2244-SENTINEL')); + await more.tap(); + await page.getByRole('menuitem', { name: 'Copy item ID' }).tap(); + await expect.poll(() => page.evaluate(() => navigator.clipboard.readText())).toBe(ref); + + // Reorder: the card moves to the top of its lane, on screen and on the + // server (a reload after the write's own response, never after the tap). + const lane = page.locator('.board-view .item-card .card-title'); + const mine = async () => { + const all = await lane.allInnerTexts(); + return all.map((t) => titles.indexOf(t.trim())).filter((i) => i >= 0); + }; + const before = await mine(); + expect(before[0], `precondition: the card is not already first (${before})`).not.toBe(2); + await more.tap(); + await page.getByRole('menuitem', { name: 'Move to top' }).tap(); + const moved = [2, ...before.filter((i) => i !== 2)]; + await expect.poll(mine, 'on screen').toEqual(moved); + // persistReorder PATCHes whichever rows' sort_order changes, not always the + // moved card, so the server's answer is read by reloading until it agrees. + await expect + .poll( + async () => { + await page.reload(); + await expect(lane.first()).toBeVisible(); + return mine(); + }, + { message: 'after a reload (the server order)', timeout: 15000 }, + ) + .toEqual(moved); +}); + +test('TASK-2244: on a host without reorder the mobile ⋯ holds only Star and Copy (starred page)', async ({ + page, + fixture, + request, +}, testInfo) => { + test.skip(testInfo.project.name !== 'mobile-chromium', 'the ruling is <=768px'); + const { titles, slugs } = await seedLane(request, fixture, 'starred'); + const star = await request.post(`/api/v1/workspaces/${fixture.workspaceSlug}/items/${slugs[0]}/star`, { + headers: authHeaders(fixture), + }); + expect(star.ok(), await star.text()).toBeTruthy(); + + await browserLogin(page); + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/starred`); + const c = card(page, titles[0]); + await expect(c).toBeVisible(); + await expect(c.locator('.starred-mark')).toBeVisible(); + await c.locator('.iam-trigger.card').tap(); + const items = page.getByRole('menuitem'); + await expect(items).toHaveText(['Unstar', 'Copy item ID'].map((t) => new RegExp(t))); +}); + +/** + * Desktop card controls are unchanged. Pinned: the copy button, whose CSS fixes + * it at 22x22. The star and the ⋮ are glyph-sized and depend on the machine's + * fonts (CI measured ⋮ 18 and star 11.9 wide where this box measured 8 and 13), + * so for them the check is that they stayed small: the phone rule would make + * one 44x44 ⋯ in their place. + */ +for (const view of ['list', 'board'] as const) { + test(`TASK-2244: desktop card controls are unchanged (${view})`, async ({ page, fixture, request }, testInfo) => { + test.skip(testInfo.project.name !== 'desktop-chromium', 'the desktop leg'); + const { titles, collSlug } = await seedLane(request, fixture, `desk-${view}`); + + await browserLogin(page); + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/${collSlug}?view=${view}`); + const c = card(page, titles[1]); + await expect(c).toBeVisible(); + + for (const sel of ['.star-btn', '.copy-ref-btn', '.iam-trigger']) { + await expect(c.locator(sel), `${sel} is rendered on desktop`).toHaveCount(1); + } + const copy = (await c.locator('.copy-ref-btn').boundingBox())!; + expect({ w: copy.width, h: copy.height }, 'copy button').toEqual({ w: 22, h: 22 }); + for (const sel of ['.star-btn', '.iam-trigger']) { + const b = (await c.locator(sel).boundingBox())!; + expect.soft(b.width, `${sel} width stays small`).toBeLessThan(MIN); + expect.soft(b.height, `${sel} height stays small`).toBeLessThan(MIN); + } + await expect(c.locator('.iam-trigger.card'), 'no card ⋯ on desktop').toHaveCount(0); + await expect(c.locator('.starred-mark'), 'no passive mark on desktop').toHaveCount(0); + }); +} + +// ── U-c: the status chip's invisible extender ─────────────────────────────── + +/** + * The extender does not change the chip's box, so boundingBox cannot prove it. + * The instrument is elementFromPoint, from both sides: + * 1. every point of the 44x44 area hits the chip. The area is centred + * horizontally and grows UPWARD from the chip's bottom edge, because a + * centred square took the top of the first tag (measured on this leg); + * 2. no point of a neighbour control's box (both tag buttons below, and the + * card ⋯ above) lands on the chip. Asked that way, not as "every point + * hits the neighbour": a rounded tag's corner pixel hits its row, with or + * without any extender, and that is not what the ruling is about. A + * centred extender DID land 20 of tag 0's 40 sampled points on the chip + * (list and board alike; the upward one lands 0), which is the measurement + * that made it upward-only. + * The card carries tags on purpose: without them the row below is plain text, + * and half 2 would pass on any extender, however large. + */ +type Box = { x: number; y: number; w: number; h: number }; + +/** Sample the box every 4px; `want` says whether a hit on `el` is what is sought. */ +async function probe(target: Locator, box: Box, want: 'hits' | 'avoids') { + return target.evaluate( + (el, [b, mode]) => { + const bad: string[] = []; + const step = 4; + for (let x = b.x + 0.5; x < b.x + b.w; x += step) { + for (let y = b.y + 0.5; y < b.y + b.h; y += step) { + const hit = document.elementFromPoint(x, y); + const onEl = !!hit && (hit === el || el.contains(hit)); + if (onEl !== (mode === 'hits')) { + bad.push(`${Math.round(x)},${Math.round(y)}→${hit?.className || hit?.tagName}`); + } + } + } + return bad; + }, + [box, want] as const, + ); +} + +for (const view of ['list', 'board'] as const) { + test(`TASK-2244: the status chip is tappable across 44x44 and takes nothing from the tags (${view})`, async ({ + page, + fixture, + request, + }, testInfo) => { + test.skip(testInfo.project.name !== 'mobile-chromium', 'the ruling is <=768px'); + const { titles, slugs, collSlug } = await seedLane(request, fixture, `chip-${view}`); + const tagged = await request.patch(`/api/v1/workspaces/${fixture.workspaceSlug}/items/${slugs[1]}`, { + headers: authHeaders(fixture), + data: { tags: JSON.stringify(['t2244a', 't2244b']) }, + }); + expect(tagged.ok(), await tagged.text()).toBeTruthy(); + + await browserLogin(page); + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/${collSlug}?view=${view}`); + const c = card(page, titles[1]); + await expect(c).toBeVisible(); + // Addressed without the new wrapper class, so the leg also runs on base. + const chip = c.locator('.card-meta button', { hasText: /^\s*open\s*$/i }); + await expect(chip, 'the status chip is a picker on this card').toHaveCount(1); + const tags = c.locator('.card-tag'); + await expect(tags, 'precondition: the tag row the extender must not cover').toHaveCount(2); + + const b = (await chip.boundingBox())!; + const area = { x: b.x + b.width / 2 - MIN / 2, y: b.y + b.height - MIN, w: MIN, h: MIN }; + expect(await probe(chip, area, 'hits'), 'points in the 44x44 area that miss the chip').toEqual([]); + + const neighbours: Array<[string, Locator]> = [ + ['tag 0', tags.nth(0)], + ['tag 1', tags.nth(1)], + ['card ⋯', c.locator('.iam-trigger.card')], + ]; + for (const [name, n] of neighbours) { + if ((await n.count()) === 0) continue; // base has no card ⋯; half 1 already fails there + const nb = (await n.boundingBox())!; + expect( + await probe(chip, { x: nb.x, y: nb.y, w: nb.width, h: nb.height }, 'avoids'), + `points of ${name} that land on the status chip`, + ).toEqual([]); + } + }); +} diff --git a/web/src/lib/components/collections/ItemActionsMenu.card.svelte.test.ts b/web/src/lib/components/collections/ItemActionsMenu.card.svelte.test.ts new file mode 100644 index 000000000..2feac569c --- /dev/null +++ b/web/src/lib/components/collections/ItemActionsMenu.card.svelte.test.ts @@ -0,0 +1,92 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { render, cleanup, screen, fireEvent } from '@testing-library/svelte'; +import type { Item } from '$lib/types'; +import ItemActionsMenu from './ItemActionsMenu.svelte'; + +/** + * TASK-2244 — the card's ONE touch-sized ⋯ at phone width (Dave's ruling, day + * 83). Star, copy-ref and the reorder ⋮ become entries of this menu. + * + * The entry set depends on the HOST: board and list wire reorder, but the + * starred, tags and roles pages do not. Before this change the menu rendered + * nothing without `onReorder`, so on those pages a mobile card would have had + * no way to star or copy at all. The cases below cover each combination, plus + * the desktop shape (no `card`), which must stay the reorder-only ⋮. + */ + +const item = { id: 'i1', slug: 'one', title: 'One', fields: '{}', tags: '[]' } as unknown as Item; + +afterEach(() => cleanup()); + +async function openAndList() { + await fireEvent.click(screen.getByRole('button', { expanded: false })); + return screen.getAllByRole('menuitem').map((el) => el.textContent?.trim() ?? ''); +} + +describe('ItemActionsMenu card mode (TASK-2244)', () => { + it('without reorder (starred/tags/roles pages): only Star and Copy', async () => { + render(ItemActionsMenu, { + props: { item, label: 'One', card: { starred: false, onToggleStar: () => {}, onCopyRef: () => {} } }, + }); + const trigger = screen.getByRole('button', { name: 'Actions for One' }); + expect(trigger.textContent?.trim()).toBe('⋯'); + const entries = await openAndList(); + expect(entries.map((e) => e.replace(/^\S+\s*/, ''))).toEqual(['Star', 'Copy item ID']); + }); + + it('with reorder (board/list): Star, Copy, then the reorder entries', async () => { + render(ItemActionsMenu, { + props: { + item, + label: 'One', + onReorder: () => {}, + card: { starred: true, onToggleStar: () => {}, onCopyRef: () => {} }, + }, + }); + const entries = (await openAndList()).map((e) => e.replace(/^\S+\s*/, '')); + expect(entries).toEqual(['Unstar', 'Copy item ID', 'Move to top', 'Move up', 'Move down', 'Move to bottom']); + }); + + it('an item with no ref offers no Copy entry', async () => { + render(ItemActionsMenu, { + props: { item, label: 'One', card: { starred: false, onToggleStar: () => {} } }, + }); + const entries = (await openAndList()).map((e) => e.replace(/^\S+\s*/, '')); + expect(entries).toEqual(['Star']); + }); + + it('the entries call their handlers', async () => { + const onToggleStar = vi.fn(); + const onCopyRef = vi.fn(); + const onReorder = vi.fn(); + render(ItemActionsMenu, { + props: { item, label: 'One', onReorder, card: { starred: false, onToggleStar, onCopyRef } }, + }); + for (const [name, spy] of [ + [/Star/, onToggleStar], + [/Copy item ID/, onCopyRef], + [/Move to top/, onReorder], + ] as const) { + await fireEvent.click(screen.getByRole('button', { expanded: false })); + await fireEvent.click(screen.getByRole('menuitem', { name })); + expect(spy).toHaveBeenCalledTimes(1); + } + expect(onReorder).toHaveBeenCalledWith('top'); + }); + + it('desktop shape (no card): the reorder-only ⋮, unchanged', async () => { + render(ItemActionsMenu, { props: { item, label: 'One', onReorder: () => {} } }); + const trigger = screen.getByRole('button', { name: 'Reorder One' }); + expect(trigger.textContent?.trim()).toBe('⋮'); + const entries = (await openAndList()).map((e) => e.replace(/^\S+\s*/, '')); + expect(entries).toEqual(['Move to top', 'Move up', 'Move down', 'Move to bottom']); + }); + + // Base rendered a ⋮ here whose entries called an absent onReorder; only + // ItemCard's `{#if onReorderItem}` kept it off screen. Now the menu itself + // renders nothing when it has no entry to offer. + it('no card and no reorder renders nothing', () => { + render(ItemActionsMenu, { props: { item, label: 'One' } }); + expect(screen.queryByRole('button')).toBeNull(); + }); +}); diff --git a/web/src/lib/components/collections/ItemActionsMenu.svelte b/web/src/lib/components/collections/ItemActionsMenu.svelte index 3090b7a75..184bfb081 100644 --- a/web/src/lib/components/collections/ItemActionsMenu.svelte +++ b/web/src/lib/components/collections/ItemActionsMenu.svelte @@ -43,8 +43,12 @@ interface Props { item: Item; - /** Fire the vertical reorder. The host has the item + group context bound. */ - onReorder: (dir: ReorderDirection) => void; + /** + * Fire the vertical reorder. The host has the item + group context bound. + * Omitted when the host does not reorder (the card's mobile ⋯ on the + * starred, tags and roles pages), and then no reorder entries render. + */ + onReorder?: (dir: ReorderDirection) => void; /** * Fire an adjacent-column move (board only). Only wired by BoardView; * omitted by every other host, so left/right never fire elsewhere. @@ -56,9 +60,21 @@ disabledDirs?: Set; /** Accessible label suffix, e.g. the item title. */ label?: string; + /** + * The card's ONE touch-sized ⋯ at phone width (TASK-2244, Dave's ruling, + * day 83): star, copy-ref and the ⋮ are not rendered there, and their + * actions become the first entries of this menu. The trigger is 44x44. + * Desktop never sets it, so the reorder-only ⋮ is unchanged there. + */ + card?: { + starred: boolean; + onToggleStar: () => void; + /** Absent when the item has no ref to copy. */ + onCopyRef?: () => void; + }; } - let { item, onReorder, onMove, horizontal = false, disabledDirs, label }: Props = $props(); + let { item, onReorder, onMove, horizontal = false, disabledDirs, label, card }: Props = $props(); interface Action { dir: MenuDirection; @@ -76,10 +92,20 @@ { dir: 'right', icon: '→', text: 'Move right' } ]; let actions = $derived( - [...VERTICAL_ACTIONS, ...(horizontal ? HORIZONTAL_ACTIONS : [])].filter( - (a) => !disabledDirs?.has(a.dir) - ) + onReorder + ? [...VERTICAL_ACTIONS, ...(horizontal ? HORIZONTAL_ACTIONS : [])].filter( + (a) => !disabledDirs?.has(a.dir) + ) + : [] ); + let menuLabel = $derived( + card ? (label ? `Actions for ${label}` : 'Item actions') : label ? `Reorder ${label}` : 'Reorder item' + ); + + function cardPick(run: () => void) { + open = false; + run(); + } let open = $state(false); let triggerEl = $state(); @@ -94,7 +120,7 @@ if (dir === 'left' || dir === 'right') { onMove?.(dir); } else { - onReorder(dir); + onReorder?.(dir); } } @@ -109,20 +135,21 @@ } -{#if actions.length > 0} +{#if card || actions.length > 0} + {#if card} + cardPick(card.onToggleStar)}> + {card.starred ? 'Unstar' : 'Star'} + + {#if card.onCopyRef} + {@const copy = card.onCopyRef} + cardPick(copy)}>Copy item ID + {/if} + {#if actions.length > 0} + + {/if} + {/if} {#each actions as a (a.dir)} pick(a.dir)}>{a.text} {/each} @@ -167,6 +206,27 @@ color: var(--text-primary); } + /* The card's mobile ⋯ (TASK-2244): a 44x44 box, glyph unchanged in size. + Negative block margins keep the box from making the card's top row taller; + the card padding it reaches into holds nothing tappable. */ + .iam-trigger.card { + display: inline-flex; + align-items: center; + justify-content: center; + width: 44px; + height: 44px; + margin: -12px -8px -12px 0; + padding: 0; + font-size: 1.25em; + opacity: 0.7; + } + + /* Same rule as ItemDetail's ⋯ menu divider; the class is scoped per file. */ + .menu-divider { + border-top: 1px solid var(--border-subtle); + margin: 5px 4px; + } + .iam-trigger:focus-visible { opacity: 1; outline: 2px solid var(--accent-blue); diff --git a/web/src/lib/components/collections/ItemCard.svelte b/web/src/lib/components/collections/ItemCard.svelte index 62f545877..0386980f6 100644 --- a/web/src/lib/components/collections/ItemCard.svelte +++ b/web/src/lib/components/collections/ItemCard.svelte @@ -5,6 +5,7 @@ import { isRelationType } from '$lib/items/relationFieldTypes'; import { parseFields, parseSchema, parseTags, formatItemRef, itemUrlId } from '$lib/types'; import { starredStore } from '$lib/stores/starred.svelte'; + import { viewport } from '$lib/stores/breakpoint.svelte'; import { workspaceStore } from '$lib/stores/workspace.svelte'; import { copyToClipboard } from '$lib/utils/clipboard'; import { relativeTime } from '$lib/utils/markdown'; @@ -170,6 +171,9 @@ async function copyRef(e: MouseEvent) { e.preventDefault(); e.stopPropagation(); + await writeRef(); + } + async function writeRef() { if (!itemRef) return; const ok = await copyToClipboard(itemRef); if (ok) { @@ -214,6 +218,7 @@ {#if itemRef} {itemRef} + {#if !viewport.isMobile} + {/if} {copied ? `Copied ${itemRef}` : ''} {/if} + {#if viewport.isMobile} + + {#if starred} + ★ + {/if} + onReorderItem?.(item, dir) : undefined} + onMove={onReorderItem && onMoveItem ? (dir) => onMoveItem?.(item, dir) : undefined} + card={{ + starred, + onToggleStar: () => starredStore.toggle(wsSlug, item.slug, item.id), + onCopyRef: itemRef ? () => void writeRef() : undefined, + }} + /> + {:else} {#if viewSheetOpen} @@ -4070,6 +4073,16 @@ aria-label="Collection menu" title="More" >⋯ + + {#if viewport.isMobile} + + + + {/if} (collMenuOpen = false)} @@ -4104,14 +4117,6 @@ {/if} - - {#if viewport.isMobile} - - - - {/if} @@ -4702,20 +4707,16 @@ flex-wrap: wrap; } - /* Mobile realtime badge (IDEA-2297). .header-actions is width:100% under - 768px, so the auto margin pushes the dot to the trailing edge — the - "justified across from the toolbar" placement — without disturbing - justify-content for the buttons themselves. */ - .sse-mobile { + /* Mobile realtime badge (IDEA-2297, TASK-2244): pinned to the corner of + the collection ⋯ button, so the strip keeps one row at 360 on both + views. .menu-anchor is the button's positioned ancestor. */ + .sse-badge { + position: absolute; + top: 4px; + right: 4px; display: inline-flex; - align-items: center; - margin-left: auto; - flex-shrink: 0; - /* Without this the bare 8px dot sits flush against the content edge, - while every control beside it insets its glyph by the same --space-3 - its box uses horizontally. Padding (not margin) so the dot's touch - target still reaches the edge. */ - padding-right: var(--space-3); + pointer-events: none; + line-height: 0; } /* Uniform control height across the action bar (IDEA-2297). Before this the @@ -5092,11 +5093,26 @@ column has ~56px left — not enough for the dot plus its inset plus 12px gaps, and the dot wrapped onto a second line, re-creating the wasted row IDEA-2297 removed. Six gaps × 8px - saved keeps the bar on one line down to ~340px. Only the gaps - shrink — the controls stay 28px, so touch targets are unchanged. */ + saved keeps the bar on one line down to ~340px. */ gap: var(--space-1); } + /* Touch targets (TASK-2244, Dave's ruling day 83): every strip control is + at least 44x44 here, VISIBLY. A transparent hit area on a 28px bordered + button would overlap its neighbour 4px away, so a tap in the gap would + be ambiguous; the buttons themselves grow. Desktop keeps 28px, and the + e2e pins both. */ + .view-chip, + .new-btn, + .toolbar-icon-btn, + .header-actions :global(.quick-actions-menu > .trigger-btn) { + height: 44px; + min-width: 44px; + } + .toolbar-icon-btn { + width: 44px; + } + .new-btn-label { display: none; }