diff --git a/src/app/features/room/RoomInput.tsx b/src/app/features/room/RoomInput.tsx index 41f67d03..2d63cbf1 100644 --- a/src/app/features/room/RoomInput.tsx +++ b/src/app/features/room/RoomInput.tsx @@ -116,7 +116,7 @@ import { useTheme } from '../../hooks/useTheme'; import { useRoomCreatorsTag } from '../../hooks/useRoomCreatorsTag'; import { usePowerLevelTags } from '../../hooks/usePowerLevelTags'; import { useComposingCheck } from '../../hooks/useComposingCheck'; -import { PhoneHitSpread32 } from '../../styles/PhoneHit.css'; +import { PhoneHitSpread24, PhoneHitSpread32 } from '../../styles/PhoneHit.css'; interface RoomInputProps { editor: Editor; @@ -606,7 +606,7 @@ export const RoomInput = forwardRef( style={{ padding: `${config.space.S200} ${config.space.S300} 0` }} > setReplyDraft(undefined)} variant="SurfaceVariant" size="300" diff --git a/src/app/phoneHitTestKit.test.ts b/src/app/phoneHitTestKit.test.ts new file mode 100644 index 00000000..92f1b23f --- /dev/null +++ b/src/app/phoneHitTestKit.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from 'vitest'; +import { drawnPx, expectSizeTie, iconButtons } from './phoneHitTestKit'; + +describe('phone hit test kit', () => { + it('reads { + const src = + ''; + expect(iconButtons(src).map((b) => b.tag)).toEqual([ + '', + '', + ]); + }); + + it('derives a drawn size from literal size props, defaults being 400', () => { + expect(drawnPx('', '')).toEqual({ px: 24 }); + expect(drawnPx('', '')).toEqual({ px: 40 }); + }); + + it('reads { + expect( + drawnPx('', '') + ).toEqual({ + px: 24, + }); + }); + + it('calls a size undetermined when props are spread into the IconButton or the Icon', () => { + expect(drawnPx('', '')).toHaveProperty('undetermined'); + expect(drawnPx('', '')).toHaveProperty( + 'undetermined' + ); + expect(drawnPx('', '')).toHaveProperty( + 'undetermined' + ); + expect(drawnPx('', '')).toHaveProperty('undetermined'); + }); + + it('fails a file with no IconButton, which would otherwise pass vacuously', () => { + expect(() => expectSizeTie('styles/PhoneHit.css.ts')).toThrow(/no IconButton/); + }); +}); diff --git a/src/app/phoneHitTestKit.ts b/src/app/phoneHitTestKit.ts new file mode 100644 index 00000000..a29dcc22 --- /dev/null +++ b/src/app/phoneHitTestKit.ts @@ -0,0 +1,171 @@ +// Shared helpers for the phone hit-area tests (phoneTapTargets.test.ts and one phoneHit..test.ts per later +// group of IconButtons). Not a test file: it registers nothing, the test files call it. +import { readFileSync } from 'fs'; +import { expect } from 'vitest'; +import zlib from 'zlib'; +import { compile, processVanillaFile } from '@vanilla-extract/integration'; + +// The CSS a .css.ts file emits, read back from vanilla-extract's own compile (as pages/auth/footerBand.test.ts does; +// vanilla-extract gzips the inlined source only when it is large, so both forms are read). +const compiled = async (file: string) => { + const filePath = `${process.cwd()}/src/app/${file}`; + const { source } = await compile({ filePath, cwd: process.cwd(), identOption: 'debug' }); + return processVanillaFile({ source, filePath, identOption: 'debug' }); +}; + +/** The class list each export of a .css.ts file resolves to (a composed class lists the classes it composes). */ +export const classLists = async (file: string): Promise> => + Object.fromEntries( + [...(await compiled(file)).matchAll(/^export var (\w+) = '([^']*)';$/gm)].map((m) => [ + m[1], + m[2].split(' '), + ]) + ); + +export const emitted = async (file: string) => { + const code = await compiled(file); + // A file with recipes inlines one source per rule group; read them all. + return [...code.matchAll(/source=#?([A-Za-z0-9+/=]+)/g)] + .map((m) => { + const bytes = Buffer.from(m[1], 'base64'); + return (bytes[0] === 0x1f && bytes[1] === 0x8b ? zlib.gunzipSync(bytes) : bytes).toString(); + }) + .join('\n'); +}; + +export type Decls = Record; +const parse = (body: string): Decls => + Object.fromEntries( + body + .split('\n') + .map((l) => l.trim().replace(/;$/, '')) + .filter((l) => /^[a-z-]+: -?\d+px$/.test(l)) + .map((l) => [l.split(':')[0], Number(l.split(': ')[1].replace('px', ''))]) + ); + +/** Pixel declarations of the rules whose selector matches, inside or outside a media query. */ +export const rulesFor = (css: string, selector: RegExp, media: string | undefined): Decls[] => { + const scope = media + ? [...css.matchAll(/@media ([^{]+) \{\n([\s\S]*?\n)\}(?:\n|$)/g)] + .filter((m) => m[1] === media) + .map((m) => m[2]) + .join('\n') + : css.replace(/@media [^{]+ \{\n[\s\S]*?\n\}(?:\n|$)/g, ''); + return [...scope.matchAll(/^\s*([^{}\n]+) \{\n([^}]*)\}/gm)] + .filter((m) => selector.test(m[1])) + .map((m) => parse(m[2])); +}; + +/** The source of a file under src/app. */ +export const readSource = (file: string) => + readFileSync(`${process.cwd()}/src/app/${file}`, 'utf8'); + +/** The end of a JSX opening tag starting at `from`: its closing > outside any {} expression. */ +const tagEnd = (src: string, from: number) => { + let depth = 0; + let j = from; + for (; j < src.length; j += 1) { + if (src[j] === '{') depth += 1; + else if (src[j] === '}') depth -= 1; + else if (src[j] === '>' && depth === 0) break; + } + return j; +}; + +/** Every outside any {} expression, and + * what it wraps (empty when self-closing). */ +export const iconButtons = (src: string) => + [...src.matchAll(//])/g)].map((m) => { + const start = m.index ?? 0; + const end = tagEnd(src, start); + const tag = src.slice(start, end + 1); + const close = src.indexOf('', end); + return { tag, body: tag.endsWith('/>') || close === -1 ? '' : src.slice(end + 1, close) }; + }); + +export const iconButtonTags = (src: string) => iconButtons(src).map((b) => b.tag); + +const phoneHitOf = (tag: string) => tag.match(/className=\{[^}]*?\b(PhoneHit\w*)\b/)?.[1] ?? null; + +/** The class each IconButton must carry, in source order: the variant is chosen by the button's drawn size and + * whether a clipping container or neighbours surround it, so a swapped variant is a wrong hit area. Also checks + * the name is the one PhoneHit.css.ts exports, not a local of the same name. */ +export const expectPhoneHitClasses = (file: string, want: string[]) => { + const src = readSource(file); + expect(iconButtonTags(src).map(phoneHitOf)).toEqual(want); + [...new Set(want)].forEach((name) => + expect(src).toMatch( + new RegExp( + `import \\{(?:[^}]*,)?\\s*${name}\\s*(?:,[^}]*)?\\} from '(\\.\\./)+styles/PhoneHit\\.css'` + ) + ) + ); +}; + +// IconButton `size` -> padding on each side, Icon `size` -> px (folds 2.7.2). Both default to 400. +const BUTTON_PAD: Record = { '300': 4, '400': 8, '500': 12, '600': 16 }; +const ICON_PX: Record = { + '50': 16, + '100': 18, + '200': 20, + '300': 22, + '400': 24, + '500': 28, +}; + +/** The size prop of a tag: its literal value, the default when absent, or undefined when it is an expression. */ +const sizeProp = (tag: string) => { + // props spread in may carry a size of their own + if (tag.includes('{...')) return undefined; + const m = tag.match(/\ssize=(?:"(\w+)"|\{([^}]*)\})/); + if (!m) return '400'; + return m[1] ?? m[2].match(/^'(\w+)'$/)?.[1]; +}; + +/** The px an IconButton draws, from its size prop and the Icon inside; undefined with a reason when the source + * does not say (a variable size, no Icon, Icons of different sizes). */ +export const drawnPx = (tag: string, body: string): { px: number } | { undetermined: string } => { + const pad = BUTTON_PAD[sizeProp(tag) ?? '']; + if (pad === undefined) + return { undetermined: 'size of the IconButton is not a literal (or props are spread in)' }; + const icons = [...body.matchAll(//])/g)].map((m) => { + const t = body.slice(m.index, tagEnd(body, m.index ?? 0) + 1); + return ICON_PX[sizeProp(t) ?? '']; + }); + if (icons.length === 0) return { undetermined: 'no inside' }; + if (icons.some((p) => p === undefined)) + return { undetermined: 'size of an Icon is not a literal (or props are spread in)' }; + if (new Set(icons).size > 1) return { undetermined: 'Icons of different sizes inside' }; + return { px: icons[0] + 2 * pad }; +}; + +/** Per file, per IconButton index (source order): why its drawn size cannot be derived from the source. */ +export type SizeAllow = Record>; + +/** A button carrying PhoneHitSpread or PhoneHitRoom draws N px. Plain PhoneHit has no number to tie. An + * undetermined size passes only with an entry in `allow`, and an entry for a size that can be derived fails. */ +export const expectSizeTie = (file: string, allow: SizeAllow = {}) => { + const allowed = allow[file] ?? {}; + const seen = new Set(); + const buttons = iconButtons(readSource(file)); + expect(buttons.length, `${file} has no IconButton`).toBeGreaterThan(0); + buttons.forEach(({ tag, body }, i) => { + const variant = phoneHitOf(tag)?.match(/^PhoneHit(?:Spread|Room)(\d+)$/); + if (!variant) { + expect(allowed[i], `${file} #${i} is allow-listed but has no sized variant`).toBeUndefined(); + return; + } + const drawn = drawnPx(tag, body); + if ('undetermined' in drawn) { + expect( + allowed[i], + `${file} #${i}: ${drawn.undetermined}; add it to the allow-list` + ).toBeDefined(); + seen.add(i); + return; + } + expect(allowed[i], `${file} #${i} is allow-listed but its size is derivable`).toBeUndefined(); + expect(Number(variant[1]), `${file} #${i} draws ${drawn.px} px`).toBe(drawn.px); + }); + expect(Object.keys(allowed).map(Number).sort()).toEqual([...seen].sort()); +}; diff --git a/src/app/phoneTapTargets.test.ts b/src/app/phoneTapTargets.test.ts index a6459ade..5a5394e7 100644 --- a/src/app/phoneTapTargets.test.ts +++ b/src/app/phoneTapTargets.test.ts @@ -1,46 +1,13 @@ -import { readFileSync } from 'fs'; import { describe, expect, it } from 'vitest'; -import zlib from 'zlib'; -import { compile, processVanillaFile } from '@vanilla-extract/integration'; import { MOBILE_BREAKPOINT } from './hooks/useScreenSize'; - -// The CSS a .css.ts file emits, read back from vanilla-extract's own compile (as pages/auth/footerBand.test.ts does; -// vanilla-extract gzips the inlined source only when it is large, so both forms are read). -const emitted = async (file: string) => { - const filePath = `${process.cwd()}/src/app/${file}`; - const { source } = await compile({ filePath, cwd: process.cwd(), identOption: 'debug' }); - const code = await processVanillaFile({ source, filePath, identOption: 'debug' }); - // A file with recipes inlines one source per rule group; read them all. - return [...code.matchAll(/source=#?([A-Za-z0-9+/=]+)/g)] - .map((m) => { - const bytes = Buffer.from(m[1], 'base64'); - return (bytes[0] === 0x1f && bytes[1] === 0x8b ? zlib.gunzipSync(bytes) : bytes).toString(); - }) - .join('\n'); -}; - -type Decls = Record; -const parse = (body: string): Decls => - Object.fromEntries( - body - .split('\n') - .map((l) => l.trim().replace(/;$/, '')) - .filter((l) => /^[a-z-]+: -?\d+px$/.test(l)) - .map((l) => [l.split(':')[0], Number(l.split(': ')[1].replace('px', ''))]) - ); - -/** Pixel declarations of the rules whose selector matches, inside or outside a media query. */ -const rulesFor = (css: string, selector: RegExp, media: string | undefined): Decls[] => { - const scope = media - ? [...css.matchAll(/@media ([^{]+) \{\n([\s\S]*?\n)\}(?:\n|$)/g)] - .filter((m) => m[1] === media) - .map((m) => m[2]) - .join('\n') - : css.replace(/@media [^{]+ \{\n[\s\S]*?\n\}(?:\n|$)/g, ''); - return [...scope.matchAll(/^\s*([^{}\n]+) \{\n([^}]*)\}/gm)] - .filter((m) => selector.test(m[1])) - .map((m) => parse(m[2])); -}; +import { + classLists, + emitted, + expectPhoneHitClasses, + expectSizeTie, + rulesFor, + type SizeAllow, +} from './phoneHitTestKit'; const PHONE = `screen and (max-width: ${MOBILE_BREAKPOINT}px)`; const TAP = 44; @@ -166,8 +133,25 @@ describe('phone icon-button hit areas', () => { // the menu around them clips a hit area drawn outside its box) // each class owns its own margin: (44 - drawn size) / 2 const own = (size: number) => margins(new RegExp(`PhoneHit_PhoneHitSpread${size}__`)); - expect([own(24), own(26), own(32), own(40)]).toEqual([[10], [9], [6], [2]]); - expect(rulesFor(css, /PhoneHit_PhoneHitRoom26__/, PHONE)).toEqual([{ 'margin-block': 9 }]); + expect([24, 26, 28, 32, 36, 40].map(own)).toEqual([[10], [9], [8], [6], [4], [2]]); + // the Room variants open the block axis by the same amount, on top of their Spread class + const room = (size: number) => + rulesFor(css, new RegExp(`PhoneHit_PhoneHitRoom${size}__`), PHONE); + expect([room(26), room(36)]).toEqual([[{ 'margin-block': 9 }], [{ 'margin-block': 4 }]]); + }); + + it('compose a Room variant from the Spread class of the same size, which composes PhoneHit', async () => { + const lists = await classLists('styles/PhoneHit.css.ts'); + const named = (list: string[], name: string) => list.filter((c) => c.includes(`_${name}__`)); + [26, 36].forEach((size) => { + const room = lists[`PhoneHitRoom${size}`]; + expect( + named(room, `PhoneHitSpread${size}`), + `Room${size} composes Spread${size}` + ).toHaveLength(1); + expect(named(room, 'PhoneHit'), `Room${size} composes PhoneHit`).toHaveLength(1); + }); + expect(lists.PhoneHitRoom36.some((c) => c.includes('_PhoneHitSpread26__'))).toBe(false); }); it("make a message's reaction chip 44 px to tap by opening 9 px above and below it", async () => { @@ -189,22 +173,6 @@ describe('phone icon-button hit areas', () => { }); describe('icon buttons in the room view', () => { - // The opening tag of every outside any {} expression. - const iconButtonTags = (src: string) => { - const tags: string[] = []; - for (let i = src.indexOf('' && depth === 0) break; - } - tags.push(src.slice(i, j + 1)); - } - return tags; - }; - // The class each IconButton must carry, in source order: the variant is chosen by the button's drawn size // and whether a clipping container or neighbours surround it, so a swapped variant is a wrong hit area. const expected: Record = { @@ -214,7 +182,7 @@ describe('icon buttons in the room view', () => { 'PhoneHitSpread32', // report dialog close ...Array(6).fill('PhoneHitRoom26'), // message options bar, inside a clipping menu ], - 'features/room/RoomInput.tsx': Array(6).fill('PhoneHitSpread32'), + 'features/room/RoomInput.tsx': ['PhoneHitSpread24', ...Array(5).fill('PhoneHitSpread32')], // reply cancel is a 16 px icon, the rest 24 px 'features/room/RoomViewHeader.tsx': Array(6).fill('PhoneHitSpread40'), // The chat path's other buttons. Two neighbours in a row: Spread32. One on its own: the plain PhoneHit. 'features/room/message/MessageEditor.tsx': ['PhoneHitSpread32', 'PhoneHitSpread32'], // toolbar, emoji @@ -232,21 +200,20 @@ describe('icon buttons in the room view', () => { 'pages/auth/login/PasswordLoginForm.tsx': ['PhoneHit'], }; + // IconButtons whose drawn size the source does not say (file -> index in source order -> why). Everything else + // is derived from the size props, so a Spread32 on a 24 px button fails. + const sizeAllow: SizeAllow = { + 'features/room/message/Message.tsx': { + 0: 'quick-reaction button wraps an emoji , no Icon; 32 px by its CSS class', + }, + }; + it.each(Object.entries(expected))( '%s: each IconButton carries its own PhoneHit class', - (file, want) => { - const src = readFileSync(`${process.cwd()}/src/app/${file}`, 'utf8'); - expect( - iconButtonTags(src).map((t) => t.match(/className=\{[^}]*?\b(PhoneHit\w*)\b/)?.[1] ?? null) - ).toEqual(want); - // the name is the one PhoneHit.css.ts exports, not a local of the same name - [...new Set(want)].forEach((name) => - expect(src).toMatch( - new RegExp( - `import \\{(?:[^}]*,)?\\s*${name}\\s*(?:,[^}]*)?\\} from '(\\.\\./)+styles/PhoneHit\\.css'` - ) - ) - ); - } + (file, want) => expectPhoneHitClasses(file, want) + ); + + it.each(Object.keys(expected))('%s: each sized variant equals the px its button draws', (file) => + expectSizeTie(file, sizeAllow) ); }); diff --git a/src/app/styles/PhoneHit.css.ts b/src/app/styles/PhoneHit.css.ts index 8403c126..9863851b 100644 --- a/src/app/styles/PhoneHit.css.ts +++ b/src/app/styles/PhoneHit.css.ts @@ -29,21 +29,36 @@ export const PhoneHit = style({ selectors: { '&::after': hitArea }, }); +const spreads = new Map(); + /** An icon button in a row, `visual` px wide: the row opens up so its 44 px hit area meets its neighbours'. */ -const spread = (visual: number) => - style( +const spread = (visual: number) => { + const cls = style( [PhoneHit, { '@media': { [PHONE]: { marginInline: (PHONE_TAP - visual) / 2 } } }], `PhoneHitSpread${visual}` ); + spreads.set(visual, cls); + return cls; +}; export const PhoneHitSpread24 = spread(24); export const PhoneHitSpread26 = spread(26); +export const PhoneHitSpread28 = spread(28); export const PhoneHitSpread32 = spread(32); +export const PhoneHitSpread36 = spread(36); export const PhoneHitSpread40 = spread(40); /** For a button in a clipping container (a Menu's overflow hidden eats a hit area drawn outside the box): the row - * opens up on both axes, so the 44 px sits inside the container. */ -export const PhoneHitRoom26 = style([ - PhoneHitSpread26, - { '@media': { [PHONE]: { marginBlock: (PHONE_TAP - 26) / 2 } } }, -]); + * opens up on both axes, so the 44 px sits inside the container. It is the Spread class of the same size plus the + * block margin, so the two axes cannot disagree about the size. */ +const room = (visual: number) => { + const spreadClass = spreads.get(visual); + if (!spreadClass) throw new Error(`PhoneHitRoom${visual} needs a PhoneHitSpread${visual}`); + return style( + [spreadClass, { '@media': { [PHONE]: { marginBlock: (PHONE_TAP - visual) / 2 } } }], + `PhoneHitRoom${visual}` + ); +}; + +export const PhoneHitRoom26 = room(26); +export const PhoneHitRoom36 = room(36);