diff --git a/config/scripts/mobile-web-app-source-control-keyboard.test.mjs b/config/scripts/mobile-web-app-source-control-keyboard.test.mjs new file mode 100644 index 00000000000..e28a6af0234 --- /dev/null +++ b/config/scripts/mobile-web-app-source-control-keyboard.test.mjs @@ -0,0 +1,118 @@ +/** + * What the source-control hub and the diff review page measure a keyboard with. + * + * react-native-web's `Keyboard` is a stub: `addListener` returns a subscription that never fires + * and `isVisible()` is always false. A module inside the page that waits for `keyboardDidShow` + * waits for the life of the document, and the software keyboard covers whatever is at the bottom + * of it — the hub's commit bar and the review note composer, both of which are text entry. + * + * So the rule is the seam, not the two call sites it has today: `platform/keyboard-occlusion` is + * the one module in either closure allowed to name the stub, and it has a `.web.ts` sibling that + * reads `visualViewport` instead. + */ +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' +import { mobileWebAppRouteClosure } from './build-mobile-web-app-bundle.mjs' +import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs' + +const mobileDir = fileURLToPath(new URL('../../mobile/', import.meta.url)) +const describeClosure = mobileWebAppDependenciesPresent() ? describe : describe.skip + +const HUB = 'app/h/[hostId]/source-control/[worktreeId].tsx' +const REVIEW = 'app/h/[hostId]/review/[worktreeId].tsx' +const SEAM = 'src/platform/keyboard-occlusion.web.ts' + +/** + * The one module that subscribes to the stub and is not the seam, exempt by name. + * + * `mounted-bottom-drawer.tsx` reads more than a height: `Keyboard.metrics()` for a sheet opened + * over an already-raised keyboard, and each event's `duration` to animate with it. The seam models + * neither, and the drawer is in C1's, C2's, C3's and C5's closures as well as these two, so moving + * it is a change to every page rather than to this domain. Its listeners are inert on the web the + * same way, which is exactly why the composer inside it takes its own padding here. + */ +const SUBSCRIBES_TO_THE_STUB = ['src/components/mounted-bottom-drawer.tsx'] + +/** + * The seam itself, which is the one place allowed to name the stub. + * + * The two files by name rather than everything under `src/platform/`: a later + * `src/platform/.web.ts` that subscribed to `Keyboard` directly would be the same + * defect this census exists for, and a directory-wide exemption would wave it through. + */ +const SEAM_FILES = ['src/platform/keyboard-occlusion.ts', 'src/platform/keyboard-occlusion.web.ts'] + +/** + * `rootDir` is a parameter for the planted case below, which must not write into the real tree: + * `mobile-web-app-web-overrides.test.mjs` lists `mobile/src` in a parallel worker and would see a + * planted file as an unlisted `.web.*` override. `findWebSiblings(rootDir)` takes a root for the + * same reason. + */ +function keyboardSubscribers(closure, rootDir = mobileDir) { + return closure.local + .filter((file) => !SEAM_FILES.includes(file)) + .filter((file) => { + try { + return readFileSync(join(rootDir, file), 'utf8').includes('Keyboard.addListener') + } catch { + return false + } + }) + .sort() +} + +describeClosure( + 'the keyboard the source-control and review pages measure', + () => { + it.each([HUB, REVIEW])('measures it through the seam and nowhere else: %s', async (route) => { + const closure = await mobileWebAppRouteClosure(route) + expect(keyboardSubscribers(closure)).toEqual(SUBSCRIBES_TO_THE_STUB) + }) + + it.each([HUB, REVIEW])('carries the seam, so the rule is not vacuous: %s', async (route) => { + // Without this an empty subscriber list would also be what a closure reaching no keyboard + // code at all produces, and the census would pass against a page that measures nothing. + const closure = await mobileWebAppRouteClosure(route) + expect(closure.local).toContain(SEAM) + }) + + it('names a module under src/platform that is not the seam', async () => { + // The exemption is the two seam files, not their directory: a planted subscriber beside them + // is named, which a `startsWith('src/platform/')` filter would have let through. + // + // Under mkdtemp rather than in `mobile/src/platform/`: the overrides census walks that tree + // in a parallel worker, and a planted `.web.ts` there is an unlisted override to it. + const root = mkdtempSync(join(tmpdir(), 'orca-keyboard-census-')) + const subscriber = + 'import { Keyboard } from "react-native"\nKeyboard.addListener("x", () => {})\n' + try { + mkdirSync(join(root, 'src', 'platform'), { recursive: true }) + // The seam files carry the call too, so the empty result for them is the name exemption + // doing the work rather than the two files happening not to subscribe. + for (const file of ['src/platform/other.web.ts', ...SEAM_FILES]) { + writeFileSync(join(root, file), subscriber) + } + expect( + keyboardSubscribers({ local: ['src/platform/other.web.ts', ...SEAM_FILES] }, root) + ).toEqual(['src/platform/other.web.ts']) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + it('names an exemption that is really in both closures, so it cannot outlive its subject', async () => { + const [hub, review] = await Promise.all([ + mobileWebAppRouteClosure(HUB), + mobileWebAppRouteClosure(REVIEW) + ]) + for (const file of SUBSCRIBES_TO_THE_STUB) { + expect(hub.local, file).toContain(file) + expect(review.local, file).toContain(file) + } + }) + }, + 240_000 +) diff --git a/config/scripts/mobile-web-app-source-control-text-inputs.test.mjs b/config/scripts/mobile-web-app-source-control-text-inputs.test.mjs new file mode 100644 index 00000000000..60b45370d73 --- /dev/null +++ b/config/scripts/mobile-web-app-source-control-text-inputs.test.mjs @@ -0,0 +1,296 @@ +/** + * Every text input the source-control hub and the diff review page reach, and the size it declares. + * + * iOS zooms the page on focus of any input under 16px and does not zoom back out, so the document + * spends the rest of that typing session at a scale other than 1 — which `keyboard-occlusion.web.ts` + * reads as "not a keyboard" on purpose, because geometry cannot separate a zoom from a keyboard. + * One 14px input anywhere in the closure is therefore enough to stop the commit bar and the note + * composer lifting, whatever those two inputs themselves declare. + * + * So the rule is the closure rather than the two seam-served sites: the first version of this fix + * raised those two and left eight others in the same closures at 14px, which made the seam's own + * rationale false page-wide. + */ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' +import { mobileWebAppRouteClosure } from './build-mobile-web-app-bundle.mjs' +import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs' +import { + TEXT_INPUT_FONT_SIZE_SEAM, + textInputFontSizeOffenders, + unresolvedTextInputStyles +} from './mobile-web-app-text-input-font-size-seam.mjs' + +const mobileDir = fileURLToPath(new URL('../../mobile/', import.meta.url)) +const describeClosure = mobileWebAppDependenciesPresent() ? describe : describe.skip + +const HUB = 'app/h/[hostId]/source-control/[worktreeId].tsx' +const REVIEW = 'app/h/[hostId]/review/[worktreeId].tsx' + +/** The seam module itself, which a fixture needs on disk for an import of it to resolve. */ +const SEAM_SOURCE = { + 'src/platform/text-input-font-size.ts': 'export const TEXT_INPUT_FONT_SIZE = 14' +} +const SEAM_IMPORT = "import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size'" + +/** A scratch module tree, so a planted offender never lands in the tree other censuses walk. */ +function plant(files) { + const root = mkdtempSync(join(tmpdir(), 'orca-text-input-census-')) + for (const [path, source] of Object.entries(files)) { + mkdirSync(join(root, path.slice(0, path.lastIndexOf('/'))), { recursive: true }) + writeFileSync(join(root, path), source) + } + return root +} + +describe('the size a text input declares, as the census reads it', () => { + it('names the line the size is set on, which may not be the file the input is in', () => { + const root = plant({ + 'src/ui/Field.tsx': [ + "import { styles } from './field-styles'", + 'export const Field = () => ' + ].join('\n'), + 'src/ui/field-styles.ts': [ + 'export const styles = {', + ' label: { fontSize: 12 },', + ' input: { fontSize: 14 }', + '}' + ].join('\n') + }) + try { + expect( + textInputFontSizeOffenders(root, { local: ['src/ui/Field.tsx', 'src/ui/field-styles.ts'] }) + ).toEqual(['src/ui/field-styles.ts:3']) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + it('follows a spread into the module that really holds the key', () => { + // Both screens this rule exists for reach their input through `{ ...base, ...list }`. A walk + // that stopped at the first module would find no size here and report the offence as absent. + const root = plant({ + 'src/ui/Field.tsx': [ + "import { styles } from './field-styles'", + 'export const Field = () => ' + ].join('\n'), + 'src/ui/field-styles.ts': [ + "import { listStyles } from './list-styles'", + 'export const styles = { ...listStyles }' + ].join('\n'), + 'src/ui/list-styles.ts': 'export const listStyles = { input: { fontSize: 14 } }' + }) + try { + expect( + textInputFontSizeOffenders(root, { + local: ['src/ui/Field.tsx', 'src/ui/field-styles.ts', 'src/ui/list-styles.ts'] + }) + ).toEqual(['src/ui/list-styles.ts:1']) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + it('separates a style it could not follow from one that sets no size', () => { + // An offender list only says every input is on the seam if every input was read. A style the + // walk cannot follow has to surface here rather than pass as a clean input. + const root = plant({ + 'src/ui/Field.tsx': [ + "import { styles } from './field-styles'", + "import { missing } from 'some-package'", + 'export const Bare = () => ', + 'export const Gone = () => ' + ].join('\n'), + 'src/ui/field-styles.ts': 'export const styles = { bare: { padding: 8 } }' + }) + try { + const closure = { local: ['src/ui/Field.tsx', 'src/ui/field-styles.ts'] } + expect(textInputFontSizeOffenders(root, closure)).toEqual([]) + expect(unresolvedTextInputStyles(root, closure)).toEqual(['src/ui/Field.tsx:4 (input)']) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + it('takes the seam as the answer, and an absent size as nothing to answer for', () => { + // A style with no `fontSize` inherits; the floor is about the size an input declares. + const root = plant({ + 'src/ui/Field.tsx': [ + "import { styles } from './field-styles'", + 'export const Field = () => ', + 'export const Other = () => ' + ].join('\n'), + // Imported, not merely spelled: the rule reads the binding now, so a fixture that wrote the + // name without importing it from the seam would be an offender like any other shadow. + 'src/ui/field-styles.ts': [ + SEAM_IMPORT, + 'export const styles = {', + ' input: { fontSize: TEXT_INPUT_FONT_SIZE },', + ' bare: { padding: 8 }', + '}' + ].join('\n'), + ...SEAM_SOURCE + }) + try { + expect( + textInputFontSizeOffenders(root, { local: ['src/ui/Field.tsx', 'src/ui/field-styles.ts'] }) + ).toEqual([]) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + it('reads an inline style literal in place rather than losing it', () => { + // `style={{ fontSize: 14 }}` names no style key, so a walk that only followed `styles.key` + // recorded nothing at all for it: neither an offender nor a hole. + const root = plant({ + 'src/ui/Inline.tsx': [ + 'export const Sized = () => ', + 'export const Bare = () => ' + ].join('\n') + }) + try { + const closure = { local: ['src/ui/Inline.tsx'] } + expect(textInputFontSizeOffenders(root, closure)).toEqual(['src/ui/Inline.tsx:1']) + expect(unresolvedTextInputStyles(root, closure)).toEqual([]) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + it('reads an inline literal beside a style key, and the condition between them', () => { + // `[styles.input, disabled && styles.disabled]` is the shape this tree actually uses, so the + // members of an array — and the right of an `&&` — have to be followed, not walked as a blob. + const root = plant({ + 'src/ui/Mixed.tsx': [ + "import { styles } from './mixed-styles'", + 'export const Mixed = () => (', + ' ', + ')' + ].join('\n'), + 'src/ui/mixed-styles.ts': [ + SEAM_IMPORT, + 'export const styles = {', + ' input: { fontSize: TEXT_INPUT_FONT_SIZE },', + ' disabled: { opacity: 0.5 }', + '}' + ].join('\n'), + ...SEAM_SOURCE + }) + try { + const closure = { + local: ['src/ui/Mixed.tsx', 'src/ui/mixed-styles.ts', ...Object.keys(SEAM_SOURCE)] + } + expect(textInputFontSizeOffenders(root, closure)).toEqual(['src/ui/Mixed.tsx:3']) + expect(unresolvedTextInputStyles(root, closure)).toEqual([]) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + it('names a style shape it cannot follow rather than dropping it', () => { + const root = plant({ + 'src/ui/Called.tsx': 'export const Called = () => ' + }) + try { + expect(unresolvedTextInputStyles(root, { local: ['src/ui/Called.tsx'] })).toEqual([ + 'src/ui/Called.tsx:1 (makeStyle())' + ]) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + it('lets a later spread beat a direct key, as the runtime object does', () => { + // `{ input: safe, ...legacy }` is `legacy.input` at runtime. Answering the direct key first + // read the safe one and called the override clean. + const root = plant({ + 'src/ui/Order.tsx': [ + "import { styles } from './order-styles'", + 'export const Order = () => ' + ].join('\n'), + 'src/ui/order-styles.ts': [ + SEAM_IMPORT, + "import { legacy } from './legacy-styles'", + 'export const styles = { input: { fontSize: TEXT_INPUT_FONT_SIZE }, ...legacy }' + ].join('\n'), + 'src/ui/legacy-styles.ts': 'export const legacy = { input: { fontSize: 14 } }', + ...SEAM_SOURCE + }) + try { + expect( + textInputFontSizeOffenders(root, { + local: [ + 'src/ui/Order.tsx', + 'src/ui/order-styles.ts', + 'src/ui/legacy-styles.ts', + ...Object.keys(SEAM_SOURCE) + ] + }) + ).toEqual(['src/ui/legacy-styles.ts:1']) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + it.each([ + ['a local constant wearing the name', 'const TEXT_INPUT_FONT_SIZE = 14'], + ['an import of the name from elsewhere', "import { TEXT_INPUT_FONT_SIZE } from './elsewhere'"] + ])('reads the seam as a binding, not a spelling: %s', (_label, preamble) => { + const root = plant({ + 'src/ui/Shadow.tsx': [ + "import { styles } from './shadow-styles'", + 'export const Shadow = () => ' + ].join('\n'), + 'src/ui/shadow-styles.ts': [ + preamble, + 'export const styles = { input: { fontSize: TEXT_INPUT_FONT_SIZE } }' + ].join('\n'), + 'src/ui/elsewhere.ts': 'export const TEXT_INPUT_FONT_SIZE = 14', + ...SEAM_SOURCE + }) + try { + expect( + textInputFontSizeOffenders(root, { + local: [ + 'src/ui/Shadow.tsx', + 'src/ui/shadow-styles.ts', + 'src/ui/elsewhere.ts', + ...Object.keys(SEAM_SOURCE) + ] + }) + ).toEqual(['src/ui/shadow-styles.ts:2']) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) +}) + +describeClosure( + 'the text inputs the source-control and review pages reach', + () => { + it.each([HUB, REVIEW])('takes every input size through the seam: %s', async (route) => { + const closure = await mobileWebAppRouteClosure(route) + expect(textInputFontSizeOffenders(mobileDir, closure)).toEqual([]) + }) + + it.each([HUB, REVIEW])( + 'reads every input it found, so the list above is complete: %s', + async (route) => { + const closure = await mobileWebAppRouteClosure(route) + expect(unresolvedTextInputStyles(mobileDir, closure)).toEqual([]) + } + ) + + it.each([HUB, REVIEW])('carries the seam, so the rule is not vacuous: %s', async (route) => { + // Without this an empty offender list would also be what a closure reaching no text input at + // all produces, and the census would pass against a page that has nothing to raise. + const closure = await mobileWebAppRouteClosure(route) + expect(closure.local).toContain(TEXT_INPUT_FONT_SIZE_SEAM) + }) + }, + 240_000 +) diff --git a/config/scripts/mobile-web-app-text-input-font-size-seam.mjs b/config/scripts/mobile-web-app-text-input-font-size-seam.mjs new file mode 100644 index 00000000000..77521f96bb3 --- /dev/null +++ b/config/scripts/mobile-web-app-text-input-font-size-seam.mjs @@ -0,0 +1,377 @@ +/** + * The text-input font-size seam, and how a census finds an input that went around it. + * + * `src/platform/text-input-font-size.web.ts` raises the app's body size to the floor below which + * iOS zooms the page on focus. That zoom is what `keyboard-occlusion.web.ts` reads as "not a + * keyboard", so one 14px input left in a page route's closure is enough to put a typing session at + * a scale other than 1 and stop the commit bar and the note composer lifting for the rest of it. + * A rule per route closure rather than per known site: the first fix covered two inputs and eight + * others in the same closures still carried the old size. + */ +import { existsSync, readFileSync } from 'node:fs' +import { dirname, join, relative, resolve } from 'node:path' +import ts from 'typescript-api' + +export const TEXT_INPUT_FONT_SIZE_SEAM = 'src/platform/text-input-font-size.web.ts' + +/** The seam's own name, and the module it has to come from. */ +const SEAM_EXPORT = 'TEXT_INPUT_FONT_SIZE' +const SEAM_MODULE = 'src/platform/text-input-font-size.ts' + +const parse = (file, source) => ts.createSourceFile(file, source, ts.ScriptTarget.Latest, true) + +function readOrNull(path) { + try { + return readFileSync(path, 'utf8') + } catch { + return null + } +} + +/** A relative specifier as a path under `mobile/`, or null for a package. */ +function resolveLocal(mobileDir, fromFile, specifier) { + if (!specifier.startsWith('.')) { + return null + } + const base = resolve(dirname(join(mobileDir, fromFile)), specifier) + for (const extension of ['.ts', '.tsx', '/index.ts', '/index.tsx']) { + if (existsSync(base + extension)) { + // Relative to the root rather than sliced by its length, which leaves a leading separator + // whenever the root is passed without a trailing one. + return relative(mobileDir, base + extension).replaceAll('\\', '/') + } + } + return null +} + +/** + * The style expressions one `style` prop really applies, flattened. + * + * A prop is rarely one thing: `[styles.input, disabled && styles.disabled]` is the common shape in + * this tree, and both members can reach the element. So arrays, spreads, `&&`, `?:` and parentheses + * are followed to the expressions that can actually land, rather than walked as a blob — a subtree + * walk would also descend into an inline literal's own properties and read them as style keys. + * + * A branch that contributes nothing when taken (`null`, `undefined`, `false`) is dropped: it is not + * a style the walk failed to follow. + */ +function styleExpressions(expression) { + if (ts.isJsxExpression(expression)) { + return expression.expression === undefined ? [] : styleExpressions(expression.expression) + } + if (ts.isParenthesizedExpression(expression) || ts.isAsExpression(expression)) { + return styleExpressions(expression.expression) + } + if (ts.isArrayLiteralExpression(expression)) { + return expression.elements.flatMap((element) => styleExpressions(element)) + } + if (ts.isSpreadElement(expression)) { + return styleExpressions(expression.expression) + } + if (ts.isConditionalExpression(expression)) { + // Either branch can be the one that renders. + return [...styleExpressions(expression.whenTrue), ...styleExpressions(expression.whenFalse)] + } + if (ts.isBinaryExpression(expression)) { + const kind = expression.operatorToken.kind + if (kind === ts.SyntaxKind.AmpersandAmpersandToken) { + // The left side is the condition, not a style. + return styleExpressions(expression.right) + } + if (kind === ts.SyntaxKind.BarBarToken || kind === ts.SyntaxKind.QuestionQuestionToken) { + return [...styleExpressions(expression.left), ...styleExpressions(expression.right)] + } + return [expression] + } + if ( + expression.kind === ts.SyntaxKind.NullKeyword || + expression.kind === ts.SyntaxKind.FalseKeyword || + (ts.isIdentifier(expression) && expression.text === 'undefined') + ) { + return [] + } + return [expression] +} + +/** + * Every style one `TextInput` applies, as something the rule can answer for. + * + * Three shapes, because a shape that is none of them has to be visible rather than dropped: a + * `styles.key` reference to follow, an inline object literal to read in place, and anything else — + * a call, a bare identifier, an expression this walk does not model — which is a hole and belongs + * in the unresolved list. + */ +function textInputStyleRefs(parsed) { + const refs = [] + const visit = (node) => { + const element = ts.isJsxSelfClosingElement(node) + ? node + : ts.isJsxOpeningElement(node) + ? node + : null + if (element !== null && element.tagName.getText() === 'TextInput') { + for (const attribute of element.attributes.properties) { + if ( + !ts.isJsxAttribute(attribute) || + attribute.name.getText() !== 'style' || + attribute.initializer === undefined + ) { + continue + } + // The element's line, so a ref names where its input sits. + const line = parsed.getLineAndCharacterOfPosition(element.getStart(parsed)).line + 1 + for (const expression of styleExpressions(attribute.initializer)) { + if (ts.isPropertyAccessExpression(expression) && ts.isIdentifier(expression.expression)) { + refs.push({ + kind: 'ref', + object: expression.expression.text, + key: expression.name.text, + line + }) + continue + } + if (ts.isObjectLiteralExpression(expression)) { + refs.push({ kind: 'inline', node: expression, key: 'inline style', line }) + continue + } + refs.push({ kind: 'opaque', key: expression.getText().slice(0, 40), line }) + } + } + } + ts.forEachChild(node, visit) + } + ts.forEachChild(parsed, visit) + return refs +} + +/** Where a local name was declared: this file, or the module it came in from. */ +function originOf(mobileDir, parsed, file, name) { + for (const statement of parsed.statements) { + if (!ts.isImportDeclaration(statement) || !ts.isStringLiteral(statement.moduleSpecifier)) { + continue + } + const bindings = statement.importClause?.namedBindings + if (bindings === undefined || !ts.isNamedImports(bindings)) { + continue + } + for (const element of bindings.elements) { + if (element.name.text !== name) { + continue + } + const target = resolveLocal(mobileDir, file, statement.moduleSpecifier.text) + return target === null + ? null + : { file: target, name: (element.propertyName ?? element.name).text } + } + } + return { file, name } +} + +/** + * Whether a `fontSize` initializer is the seam's export, by binding rather than by spelling. + * + * Matching the text accepts `const TEXT_INPUT_FONT_SIZE = 14` two lines up, and an import of the + * same name from any other module — both of which are exactly the regression the seam exists to + * stop, wearing its name. + */ +function isSeamBinding(mobileDir, parsed, file, initializer) { + if (!ts.isIdentifier(initializer)) { + return false + } + for (const statement of parsed.statements) { + if (!ts.isImportDeclaration(statement) || !ts.isStringLiteral(statement.moduleSpecifier)) { + continue + } + const bindings = statement.importClause?.namedBindings + if (bindings === undefined || !ts.isNamedImports(bindings)) { + continue + } + for (const element of bindings.elements) { + if ( + element.name.text === initializer.text && + (element.propertyName ?? element.name).text === SEAM_EXPORT && + resolveLocal(mobileDir, file, statement.moduleSpecifier.text) === SEAM_MODULE + ) { + return true + } + } + } + return false +} + +/** The declaration `name` binds in this file, if it has one. */ +function declarationOf(parsed, name) { + let found = null + const visit = (node) => { + if ( + found === null && + ts.isVariableDeclaration(node) && + ts.isIdentifier(node.name) && + node.name.text === name + ) { + found = node.initializer ?? null + } + ts.forEachChild(node, visit) + } + ts.forEachChild(parsed, visit) + return found +} + +/** The object literal a style declaration ends in, through `StyleSheet.create(...)`. */ +function styleObjectOf(initializer) { + if (initializer === null) { + return null + } + if (ts.isObjectLiteralExpression(initializer)) { + return initializer + } + if (ts.isCallExpression(initializer) && initializer.arguments.length > 0) { + const first = initializer.arguments[0] + return ts.isObjectLiteralExpression(first) ? first : null + } + return null +} + +/** + * What a style key resolves to, following a spread of another module's styles. + * + * Three answers, not two. `null` is "this key is nowhere I could follow", which is a hole in the + * walk rather than a clean input; `{ size: null }` is a key that exists and sets no size, which + * inherits and is nothing to answer for. Collapsing the two would let a resolution failure read as + * a passing input, which is how a census like this goes quietly vacuous. + * + * The spread matters rather than being a nicety: both screens this rule was written for reach their + * input through `{ ...baseStyles, ...listStyles }`, so a walk that stopped at the first module + * would find no `fontSize` and call the offence absent. + */ +function resolveStyleKey(mobileDir, file, exportName, key, seen = new Set()) { + const id = `${file}|${exportName}|${key}` + if (seen.has(id)) { + return null + } + seen.add(id) + const source = readOrNull(join(mobileDir, file)) + if (source === null) { + return null + } + const parsed = parse(file, source) + const object = styleObjectOf(declarationOf(parsed, exportName)) + if (object === null) { + return null + } + // Reverse source order, direct keys and spreads together: the last thing that mentions the key + // is what the object ends up holding, so `{ input: safe, ...legacy }` answers with legacy's. + // Scanning direct keys first answered `safe` and called the override clean. + for (const property of object.properties.toReversed()) { + if (ts.isSpreadAssignment(property) && ts.isIdentifier(property.expression)) { + const origin = originOf(mobileDir, parsed, file, property.expression.text) + if (origin === null) { + continue + } + const hit = resolveStyleKey(mobileDir, origin.file, origin.name, key, seen) + if (hit !== null) { + return hit + } + continue + } + if ( + !ts.isPropertyAssignment(property) || + property.name.getText().replaceAll(/['"]/g, '') !== key || + !ts.isObjectLiteralExpression(property.initializer) + ) { + continue + } + return { size: fontSizeIn(mobileDir, parsed, file, property.initializer) } + } + return null +} + +/** The `fontSize` a style object literal declares, with whether it came through the seam. */ +function fontSizeIn(mobileDir, parsed, file, object) { + for (const entry of object.properties) { + if (ts.isPropertyAssignment(entry) && entry.name.getText() === 'fontSize') { + return { + file, + text: entry.initializer.getText(), + line: parsed.getLineAndCharacterOfPosition(entry.getStart(parsed)).line + 1, + onSeam: isSeamBinding(mobileDir, parsed, file, entry.initializer) + } + } + } + return null +} + +/** Every `TextInput` style reference in a closure, with what the walk made of it. */ +function textInputStyleResolutions(mobileDir, closure) { + const found = [] + for (const file of closure.local) { + const source = readOrNull(join(mobileDir, file)) + if (source === null || !source.includes('TextInput')) { + continue + } + const parsed = parse(file, source) + for (const ref of textInputStyleRefs(parsed)) { + const at = `${file}:${ref.line}` + if (ref.kind === 'opaque') { + found.push({ at, key: ref.key, resolved: null }) + continue + } + if (ref.kind === 'inline') { + // Resolved in place: the literal is its own declaration, so there is nothing to follow. + found.push({ + at, + key: ref.key, + resolved: { size: fontSizeIn(mobileDir, parsed, file, ref.node) } + }) + continue + } + const origin = originOf(mobileDir, parsed, file, ref.object) + found.push({ + at, + key: ref.key, + resolved: + origin === null ? null : resolveStyleKey(mobileDir, origin.file, origin.name, ref.key) + }) + } + } + return found +} + +/** + * Every `TextInput` style this walk could not follow to a declaration, as `path:line`. + * + * The completeness half of the rule below: an offender list is only evidence that every input is on + * the seam if every input was read. A style reached through a package, a helper call or a shape + * this walk does not model lands here instead of passing silently. + */ +export function unresolvedTextInputStyles(mobileDir, closure) { + return textInputStyleResolutions(mobileDir, closure) + .filter((entry) => entry.resolved === null) + .map((entry) => `${entry.at} (${entry.key})`) + .sort() +} + +/** + * Every text input in a closure whose size does not come through the seam, as `path:line`. + * + * A style with no `fontSize` is not an offender: it inherits, and the floor is about the size an + * input declares. The line named is where the size is set, which is the line to change, and it may + * be in a different module from the `TextInput` that uses it. + */ +export function textInputFontSizeOffenders(mobileDir, closure) { + const offenders = new Set() + for (const entry of textInputStyleResolutions(mobileDir, closure)) { + const size = entry.resolved?.size + if (size !== undefined && size !== null && !size.onSeam) { + offenders.add(`${size.file}:${size.line}`) + } + } + return [...offenders].sort((left, right) => { + const [leftFile, leftLine] = left.split(':') + const [rightFile, rightLine] = right.split(':') + if (leftFile !== rightFile) { + return leftFile < rightFile ? -1 : 1 + } + return Number(leftLine) - Number(rightLine) + }) +} diff --git a/mobile/src/components/MobileDiffReviewDrawers.tsx b/mobile/src/components/MobileDiffReviewDrawers.tsx index 1ff763fc9b8..9f979761fe8 100644 --- a/mobile/src/components/MobileDiffReviewDrawers.tsx +++ b/mobile/src/components/MobileDiffReviewDrawers.tsx @@ -2,6 +2,7 @@ import { useMemo } from 'react' import { KeyboardAvoidingView, Platform, Pressable, Text, TextInput, View } from 'react-native' import { Check, Copy, FileText, Plus, Send, Trash2, X } from 'lucide-react-native' import type { DiffComment } from '../../../src/shared/diff-comment-types' +import { useKeyboardAvoidingPadding } from '../platform/keyboard-occlusion' import { colors } from '../theme/mobile-theme' import type { ActionSheetAction } from './ActionSheetModal' import { ActionSheetModal } from './ActionSheetModal' @@ -162,9 +163,16 @@ function sendSheetMessage( function NoteComposerDrawer({ controller }: Props) { const composer = controller.composer + // Zero on a phone, where `KeyboardAvoidingView` above already moved this; the page's own + // keyboard measurement where it cannot, because that view is driven by events RN Web never + // sends. Padding rather than a second avoiding view: the drawer owns the position. + const keyboardPadding = useKeyboardAvoidingPadding() return ( - + 0 ? { paddingBottom: keyboardPadding } : undefined} + > diff --git a/mobile/src/components/MobileSearchField.tsx b/mobile/src/components/MobileSearchField.tsx index cbeab92c208..3b7fc685d23 100644 --- a/mobile/src/components/MobileSearchField.tsx +++ b/mobile/src/components/MobileSearchField.tsx @@ -9,7 +9,8 @@ import { type TextInputProps } from 'react-native' import { Search, X } from 'lucide-react-native' -import { colors, radii, spacing, typography } from '../theme/mobile-theme' +import { colors, radii, spacing } from '../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' // Why: toolbar/list chrome paints and settles after the open tap; native // autoFocus alone often fails to raise the soft keyboard on iOS/Android. @@ -172,7 +173,7 @@ const styles = StyleSheet.create({ padding: 0, margin: 0, color: colors.textPrimary, - fontSize: typography.bodySize, + fontSize: TEXT_INPUT_FONT_SIZE, // Why: Android TextInput draws extra vertical padding that misaligns the // icon/clear chip unless we zero it out. includeFontPadding: false, diff --git a/mobile/src/components/SmartWorkspaceAdvancedFields.tsx b/mobile/src/components/SmartWorkspaceAdvancedFields.tsx index 4af0ad682b5..4c6e0b18f05 100644 --- a/mobile/src/components/SmartWorkspaceAdvancedFields.tsx +++ b/mobile/src/components/SmartWorkspaceAdvancedFields.tsx @@ -1,6 +1,7 @@ import { Platform, StyleSheet, Switch, Text, TextInput, View } from 'react-native' import type { MobileComposerSource } from '../tasks/use-mobile-composer-source' -import { colors, radii, spacing, typography } from '../theme/mobile-theme' +import { colors, radii, spacing } from '../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' type Props = { composer: MobileComposerSource @@ -81,7 +82,7 @@ const styles = StyleSheet.create({ borderRadius: radii.input, paddingHorizontal: spacing.md, paddingVertical: Platform.OS === 'ios' ? spacing.sm + 2 : spacing.sm, - fontSize: typography.bodySize, + fontSize: TEXT_INPUT_FONT_SIZE, borderWidth: 1, borderColor: colors.borderSubtle }, diff --git a/mobile/src/components/SmartWorkspaceSourceField.tsx b/mobile/src/components/SmartWorkspaceSourceField.tsx index 8e14ebd2fb8..ede940ca24c 100644 --- a/mobile/src/components/SmartWorkspaceSourceField.tsx +++ b/mobile/src/components/SmartWorkspaceSourceField.tsx @@ -11,6 +11,7 @@ import type { SmartNameSelection } from '../tasks/mobile-composer-source-types' import type { MobileComposerSource } from '../tasks/use-mobile-composer-source' import { colors, radii, spacing, typography } from '../theme/mobile-theme' import { TaskProviderLogo } from './TaskProviderLogo' +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' type Props = { composer: MobileComposerSource @@ -134,7 +135,7 @@ const styles = StyleSheet.create({ paddingVertical: spacing.sm + 2, borderWidth: 1, borderColor: colors.borderSubtle, - fontSize: typography.bodySize, + fontSize: TEXT_INPUT_FONT_SIZE, color: colors.textPrimary }, disabled: { diff --git a/mobile/src/components/mobile-diff-review-control-styles.ts b/mobile/src/components/mobile-diff-review-control-styles.ts index 9752b282f36..fe37d901250 100644 --- a/mobile/src/components/mobile-diff-review-control-styles.ts +++ b/mobile/src/components/mobile-diff-review-control-styles.ts @@ -1,5 +1,6 @@ import { StyleSheet } from 'react-native' import { colors, radii, spacing, typography } from '../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' export const mobileDiffReviewControlStyles = StyleSheet.create({ footer: { @@ -116,7 +117,8 @@ export const mobileDiffReviewControlStyles = StyleSheet.create({ borderColor: colors.borderSubtle, backgroundColor: colors.bgPanel, color: colors.textPrimary, - fontSize: typography.bodySize, + // Through the seam, as the commit bar is: 14px focuses into a zoomed page on iOS. + fontSize: TEXT_INPUT_FONT_SIZE, lineHeight: 20, padding: spacing.md, textAlignVertical: 'top' diff --git a/mobile/src/components/new-worktree-form-styles.ts b/mobile/src/components/new-worktree-form-styles.ts index e6ccff91751..ed6fc74850e 100644 --- a/mobile/src/components/new-worktree-form-styles.ts +++ b/mobile/src/components/new-worktree-form-styles.ts @@ -1,5 +1,6 @@ import { Platform, StyleSheet } from 'react-native' import { colors, radii, spacing, typography } from '../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' export const newWorktreeFormStyles = StyleSheet.create({ header: { @@ -122,7 +123,7 @@ export const newWorktreeFormStyles = StyleSheet.create({ borderRadius: radii.input, paddingHorizontal: spacing.md, paddingVertical: Platform.OS === 'ios' ? spacing.sm + 2 : spacing.sm, - fontSize: typography.bodySize, + fontSize: TEXT_INPUT_FONT_SIZE, borderWidth: 1, borderColor: colors.borderSubtle }, diff --git a/mobile/src/components/pr-sidebar/MobileLinkPrForm.tsx b/mobile/src/components/pr-sidebar/MobileLinkPrForm.tsx index 9ee63edf4db..0e64f6fe59f 100644 --- a/mobile/src/components/pr-sidebar/MobileLinkPrForm.tsx +++ b/mobile/src/components/pr-sidebar/MobileLinkPrForm.tsx @@ -5,6 +5,7 @@ import type { RpcClient } from '../../transport/rpc-client' import { triggerError, triggerSuccess } from '../../platform/haptics' import { parseGitHubPrReference } from '../../source-control/github-pr-link-parse' import { linkMobilePr } from '../../source-control/mobile-pr-link' +import { TEXT_INPUT_FONT_SIZE } from '../../platform/text-input-font-size' type Props = { client: RpcClient | null @@ -117,7 +118,7 @@ const styles = StyleSheet.create({ paddingHorizontal: spacing.md, paddingVertical: spacing.sm, color: colors.textPrimary, - fontSize: typography.bodySize + fontSize: TEXT_INPUT_FONT_SIZE }, error: { color: colors.statusRed, fontSize: typography.metaSize, marginTop: spacing.md }, submit: { diff --git a/mobile/src/components/pr-sidebar/mobile-pr-sidebar-styles.ts b/mobile/src/components/pr-sidebar/mobile-pr-sidebar-styles.ts index edf308350e4..3cb9c86fd9e 100644 --- a/mobile/src/components/pr-sidebar/mobile-pr-sidebar-styles.ts +++ b/mobile/src/components/pr-sidebar/mobile-pr-sidebar-styles.ts @@ -1,5 +1,6 @@ import { StyleSheet } from 'react-native' import { colors, radii, spacing, typography } from '../../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from '../../platform/text-input-font-size' // Fixed inline-dock width (KTD2/U4): leaves the diff >= ~380px within the 700px // breakpoint where docking engages. @@ -296,7 +297,7 @@ export const mobilePrSidebarStyles = StyleSheet.create({ backgroundColor: colors.bgPanel, color: colors.textPrimary, paddingHorizontal: spacing.md, - fontSize: typography.bodySize, + fontSize: TEXT_INPUT_FONT_SIZE, marginBottom: spacing.sm }, // No maxHeight / FlatList: the parent BottomDrawer scrolls this block so we diff --git a/mobile/src/components/pr-sidebar/pr-comment-composer-styles.ts b/mobile/src/components/pr-sidebar/pr-comment-composer-styles.ts index e6cb4966ca2..8c67bfd3a05 100644 --- a/mobile/src/components/pr-sidebar/pr-comment-composer-styles.ts +++ b/mobile/src/components/pr-sidebar/pr-comment-composer-styles.ts @@ -1,5 +1,6 @@ import { StyleSheet } from 'react-native' import { colors, radii, spacing, typography } from '../../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from '../../platform/text-input-font-size' // Styles for the plain-text reply / root-comment composer. Muted/monochrome to // match the PR comment timeline; split out to keep PRCommentComposer focused. @@ -17,7 +18,7 @@ export const prCommentComposerStyles = StyleSheet.create({ paddingHorizontal: spacing.md, paddingVertical: spacing.sm, color: colors.textPrimary, - fontSize: typography.bodySize, + fontSize: TEXT_INPUT_FONT_SIZE, textAlignVertical: 'top' }, actions: { diff --git a/mobile/src/components/smart-workspace-source-drawer-styles.ts b/mobile/src/components/smart-workspace-source-drawer-styles.ts index 3adc4aab8cb..95f8518cccf 100644 --- a/mobile/src/components/smart-workspace-source-drawer-styles.ts +++ b/mobile/src/components/smart-workspace-source-drawer-styles.ts @@ -1,5 +1,6 @@ import { StyleSheet } from 'react-native' import { colors, radii, spacing, typography } from '../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' export const smartWorkspaceSourceDrawerStyles = StyleSheet.create({ root: { @@ -57,7 +58,7 @@ export const smartWorkspaceSourceDrawerStyles = StyleSheet.create({ borderRadius: radii.input, paddingHorizontal: spacing.md, paddingVertical: spacing.sm + 2, - fontSize: typography.bodySize, + fontSize: TEXT_INPUT_FONT_SIZE, borderWidth: 1, borderColor: colors.borderSubtle }, diff --git a/mobile/src/platform/keyboard-occlusion.test.tsx b/mobile/src/platform/keyboard-occlusion.test.tsx new file mode 100644 index 00000000000..ef9945655b4 --- /dev/null +++ b/mobile/src/platform/keyboard-occlusion.test.tsx @@ -0,0 +1,128 @@ +import { createElement } from 'react' +import { act, create } from 'react-test-renderer' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +type Listener = (event: { endCoordinates: { height: number } }) => void + +type KeyboardHarness = { + listeners: Map + removed: string[] + platform: 'ios' | 'android' + /** Every call, not the surviving subscriptions: a hook that subscribes and unsubscribes still + * costs a phone a render per keyboard event, which `listeners.size` alone would not show. */ + addListenerCalls: number +} + +const keyboard = vi.hoisted((): KeyboardHarness => ({ + listeners: new Map(), + removed: [], + platform: 'ios', + addListenerCalls: 0 +})) + +vi.mock('react-native', () => ({ + Keyboard: { + addListener: (name: string, listener: Listener) => { + keyboard.addListenerCalls += 1 + keyboard.listeners.set(name, listener) + return { + remove: () => { + keyboard.removed.push(name) + keyboard.listeners.delete(name) + } + } + } + }, + Platform: { + get OS() { + return keyboard.platform + } + } +})) + +import { useKeyboardAvoidingPadding, useKeyboardOcclusion } from './keyboard-occlusion' + +let lift = 0 +let padding = 0 + +function Harness(): null { + lift = useKeyboardOcclusion() + return null +} + +/** Separate, so the padding case measures the padding hook's own subscriptions and nothing else. */ +function PaddingHarness(): null { + padding = useKeyboardAvoidingPadding() + return null +} + +async function mountComponent(component: () => null): Promise> { + let tree: ReturnType | null = null + await act(async () => { + tree = create(createElement(component)) + }) + if (tree === null) { + throw new Error('the harness did not mount') + } + return tree +} + +const mount = async (): Promise> => mountComponent(Harness) + +describe('the keyboard the phone reports', () => { + beforeEach(() => { + keyboard.listeners.clear() + keyboard.removed.length = 0 + keyboard.platform = 'ios' + keyboard.addListenerCalls = 0 + lift = 0 + padding = 0 + }) + + it('animates with the keyboard on iOS and after it on Android', async () => { + await mount() + expect([...keyboard.listeners.keys()].sort()).toEqual(['keyboardWillHide', 'keyboardWillShow']) + + keyboard.platform = 'android' + keyboard.listeners.clear() + await mount() + expect([...keyboard.listeners.keys()].sort()).toEqual(['keyboardDidHide', 'keyboardDidShow']) + }) + + it('lifts by the height the event carries and drops back on hide', async () => { + await mount() + await act(async () => { + keyboard.listeners.get('keyboardWillShow')?.({ endCoordinates: { height: 336 } }) + }) + expect(lift).toBe(336) + await act(async () => { + keyboard.listeners.get('keyboardWillHide')?.({ endCoordinates: { height: 0 } }) + }) + expect(lift).toBe(0) + }) + + it('never reports a negative height, whatever the event says', async () => { + await mount() + await act(async () => { + keyboard.listeners.get('keyboardWillShow')?.({ endCoordinates: { height: -10 } }) + }) + expect(lift).toBe(0) + }) + + it('removes both listeners on unmount', async () => { + const tree = await mount() + await act(async () => tree.unmount()) + expect(keyboard.removed.sort()).toEqual(['keyboardWillHide', 'keyboardWillShow']) + }) + + it('asks a phone for no composer padding, because KeyboardAvoidingView already moved it', async () => { + // Rendered, not called: a hook read outside a component measures whatever the module does at + // the top of its body and nothing its effects do, which is where a subscription would live. + // And it subscribes to nothing doing it, so a composer that calls this renders as often as it + // does today — which is what makes adding the call to a shared component safe. + await mountComponent(PaddingHarness) + expect(padding).toBe(0) + expect(keyboard.addListenerCalls).toBe(0) + expect(keyboard.listeners.size).toBe(0) + }) +}) diff --git a/mobile/src/platform/keyboard-occlusion.ts b/mobile/src/platform/keyboard-occlusion.ts new file mode 100644 index 00000000000..23ca8c7bf18 --- /dev/null +++ b/mobile/src/platform/keyboard-occlusion.ts @@ -0,0 +1,51 @@ +import { useEffect, useState } from 'react' +import { Keyboard, Platform } from 'react-native' + +/** + * How much of the bottom of the layout viewport the software keyboard covers. + * + * Native: the keyboard's own reported height, from the events the platform sends. iOS is told + * `will`, Android `did`, which is the difference between animating with the keyboard and after it. + * + * The web sibling is where this earns its place under `platform/`: react-native-web's `Keyboard` is + * a stub — `addListener` returns a subscription that never fires and `isVisible()` is always false + * — so a screen inside the shell's page that waits for a keyboard event waits forever, and the + * software keyboard covers whatever sits at the bottom of the document. The browser reports the + * same geometry a different way, through `visualViewport`. + */ +export function useKeyboardOcclusion(): number { + const [keyboardLift, setKeyboardLift] = useState(0) + + useEffect(() => { + const showEvent = Platform.OS === 'ios' ? 'keyboardWillShow' : 'keyboardDidShow' + const hideEvent = Platform.OS === 'ios' ? 'keyboardWillHide' : 'keyboardDidHide' + + const onShow = Keyboard.addListener(showEvent, (event) => { + // The keyboard's own height already describes the obscured area; the consumer adds whatever + // clearance it wants above it. + setKeyboardLift(Math.max(0, event.endCoordinates.height)) + }) + const onHide = Keyboard.addListener(hideEvent, () => setKeyboardLift(0)) + + return () => { + onShow.remove() + onHide.remove() + } + }, []) + + return keyboardLift +} + +/** + * The bottom padding a composer needs to clear the keyboard, which natively is none. + * + * `KeyboardAvoidingView` already moves the composer on a phone, so adding padding there would move + * it twice. It is inert on the web for the same reason the `Keyboard` stub is — it is driven by + * those events — so there the padding is the whole of the avoidance. + * + * A second name rather than a `Platform.OS` branch at the call site: this one subscribes to nothing + * on a phone, so a composer that asks for it renders exactly as many times as it does today. + */ +export function useKeyboardAvoidingPadding(): number { + return 0 +} diff --git a/mobile/src/platform/keyboard-occlusion.web.test.tsx b/mobile/src/platform/keyboard-occlusion.web.test.tsx new file mode 100644 index 00000000000..8c7cd7dfdcb --- /dev/null +++ b/mobile/src/platform/keyboard-occlusion.web.test.tsx @@ -0,0 +1,197 @@ +// @vitest-environment happy-dom +import { createElement } from 'react' +import { act, create } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { useKeyboardAvoidingPadding, useKeyboardOcclusion } from './keyboard-occlusion.web' + +/** The browser's own object, as much of it as this file reads: a target that resizes and scrolls. */ +class FakeVisualViewport extends EventTarget { + height: number + offsetTop = 0 + /** Optional as the browser's is: older WebViews do not implement it. */ + scale: number | undefined = 1 + readonly counts = { resize: 0, scroll: 0 } + + constructor(height: number) { + super() + this.height = height + } + + override addEventListener(type: string, listener: EventListenerOrEventListenerObject): void { + if (type === 'resize' || type === 'scroll') { + this.counts[type] += 1 + } + super.addEventListener(type, listener) + } + + override removeEventListener(type: string, listener: EventListenerOrEventListenerObject): void { + if (type === 'resize' || type === 'scroll') { + this.counts[type] -= 1 + } + super.removeEventListener(type, listener) + } + + /** The keyboard opening: the layout viewport keeps its size and this one shrinks. */ + resizeTo(height: number, offsetTop = 0): void { + this.height = height + this.offsetTop = offsetTop + this.dispatchEvent(new Event('resize')) + } + + /** Pinch zoom: the visual viewport shrinks by the scale factor with no keyboard anywhere. */ + zoomTo(scale: number): void { + this.scale = scale + this.height = LAYOUT_HEIGHT / scale + this.offsetTop = 0 + this.dispatchEvent(new Event('resize')) + } + + scrollTo(offsetTop: number): void { + this.offsetTop = offsetTop + this.dispatchEvent(new Event('scroll')) + } +} + +const LAYOUT_HEIGHT = 800 +let viewport: FakeVisualViewport | null = null +let lift = 0 +let padding = 0 + +function Harness(): null { + lift = useKeyboardOcclusion() + padding = useKeyboardAvoidingPadding() + return null +} + +async function mount(): Promise> { + let tree: ReturnType | null = null + await act(async () => { + tree = create(createElement(Harness)) + }) + if (tree === null) { + throw new Error('the harness did not mount') + } + return tree +} + +beforeEach(() => { + lift = 0 + padding = 0 + viewport = new FakeVisualViewport(LAYOUT_HEIGHT) + Object.defineProperty(window, 'innerHeight', { value: LAYOUT_HEIGHT, configurable: true }) + Object.defineProperty(window, 'visualViewport', { value: viewport, configurable: true }) +}) + +afterEach(() => { + Object.defineProperty(window, 'visualViewport', { value: undefined, configurable: true }) +}) + +describe('the keyboard the browser reports', () => { + it('reads nothing covered while the visual viewport fills the layout one', async () => { + await mount() + expect(lift).toBe(0) + }) + + it('lifts by the strip the visual viewport stops covering', async () => { + await mount() + await act(async () => viewport?.resizeTo(464)) + expect(lift).toBe(336) + }) + + it('counts an offset visual viewport, which a height alone would read as keyboard', async () => { + // A scrolled or pinched visual viewport sits partway down the layout viewport; the strip below + // it is not the keyboard, and subtracting only the height would call it one. + await mount() + await act(async () => viewport?.resizeTo(464, 100)) + expect(lift).toBe(236) + }) + + it('follows a scroll that moves the offset without resizing anything', async () => { + await mount() + await act(async () => viewport?.resizeTo(464)) + await act(async () => viewport?.scrollTo(50)) + expect(lift).toBe(286) + }) + + it('drops back to nothing when the keyboard closes', async () => { + await mount() + await act(async () => viewport?.resizeTo(464)) + await act(async () => viewport?.resizeTo(LAYOUT_HEIGHT)) + expect(lift).toBe(0) + }) + + it('reads the keyboard already up at mount, which sends no event', async () => { + viewport?.resizeTo(464) + await mount() + expect(lift).toBe(336) + }) + + it('never reports a negative strip, whatever the two viewports disagree about', async () => { + // Mobile Safari reports a visual viewport taller than the layout one mid-scroll, and a bare + // subtraction would push the commit bar down the screen instead of up. + await mount() + await act(async () => viewport?.resizeTo(LAYOUT_HEIGHT + 120)) + expect(lift).toBe(0) + }) + + it('reads a pinch zoom as no keyboard, because geometry alone cannot tell them apart', async () => { + // A 2x zoom halves the visual viewport exactly as a 400px keyboard would, and answering 400 + // here moves the commit bar and the composer on a page nobody is typing into. + await mount() + await act(async () => viewport?.zoomTo(2)) + expect(lift).toBe(0) + }) + + it('goes back to measuring once the zoom is released', async () => { + await mount() + await act(async () => viewport?.zoomTo(2)) + await act(async () => viewport?.zoomTo(1)) + await act(async () => viewport?.resizeTo(464)) + expect(lift).toBe(336) + }) + + it('answers 0 for a keyboard raised while the page is zoomed, which is the accepted loss', async () => { + // The ruling's own case: scale 2 *and* a viewport shrunk well past what the zoom alone + // explains. Nothing in the geometry separates the keyboard's share from the zoom's, so the + // seam declines rather than guessing. What keeps this off the ordinary focus path is the + // input floor — every text input in the two page closures clears 16px on the web, so a focus + // does not zoom and a scale other than 1 means a user pinched. + await mount() + await act(async () => viewport?.zoomTo(2)) + await act(async () => viewport?.resizeTo(232)) + expect(viewport?.scale).toBe(2) + expect(lift).toBe(0) + }) + + it('takes a viewport that reports no scale as unzoomed', async () => { + // `scale` is absent on older WebViews; treating that as zoomed would answer 0 for every + // keyboard on them. + await mount() + await act(async () => { + if (viewport !== null) { + viewport.scale = undefined + viewport.resizeTo(464) + } + }) + expect(lift).toBe(336) + }) + + it('answers 0 when the effect finds no visual viewport to subscribe to', async () => { + Object.defineProperty(window, 'visualViewport', { value: undefined, configurable: true }) + await mount() + expect(lift).toBe(0) + }) + + it('is the whole of the avoidance here, where KeyboardAvoidingView is inert', async () => { + await mount() + await act(async () => viewport?.resizeTo(464)) + expect(padding).toBe(336) + }) + + it('removes both listeners on unmount', async () => { + const tree = await mount() + expect(viewport?.counts).toEqual({ resize: 2, scroll: 2 }) + await act(async () => tree.unmount()) + expect(viewport?.counts).toEqual({ resize: 0, scroll: 0 }) + }) +}) diff --git a/mobile/src/platform/keyboard-occlusion.web.ts b/mobile/src/platform/keyboard-occlusion.web.ts new file mode 100644 index 00000000000..69639f00f88 --- /dev/null +++ b/mobile/src/platform/keyboard-occlusion.web.ts @@ -0,0 +1,75 @@ +import { useEffect, useState } from 'react' + +/** + * Web sibling: the keyboard's height as the browser reports it, which is not as an event. + * + * react-native-web's `Keyboard` is a stub whose `addListener` returns a subscription that never + * fires, so every screen waiting for `keyboardDidShow` inside the shell's page waits forever and + * the software keyboard covers whatever is at the bottom of the document. What the browser does + * publish is `visualViewport`: the layout viewport stays the size it was and the visual viewport + * shrinks to the part still on screen. + * + * So the occluded strip is what the visual viewport leaves uncovered at the bottom — + * `innerHeight - (height + offsetTop)`. `offsetTop` is in it because a pinch-zoomed or scrolled + * visual viewport sits partway down the layout viewport, and without it the strip below would be + * counted as keyboard. + * + * `resize` and `scroll` both, on the visual viewport rather than the window: the keyboard opening + * is a resize, and the browser scrolling the focused input into view is a scroll that moves + * `offsetTop` without resizing anything. + * + * A pinch zoom is not a keyboard, and geometry alone cannot tell them apart: a 2x zoom shrinks the + * visual viewport by exactly as much as a half-screen keyboard. So a `scale` other than 1 answers + * 0, and what makes that affordable is that the ordinary typing path never gets there. iOS zooms + * on focus of any input under 16px and does not zoom back out, so on a 14px input every focus + * would arrive zoomed and this guard would refuse the one flow the seam exists for. The fix is at + * the input rather than here: `text-input-font-size.web.ts` raises both consumers to the floor, so + * a scale other than 1 means a user pinched, and a keyboard raised during one is the rare case + * that costs. `maximum-scale=1` on the viewport meta would have done it too and was rejected — + * Android WebView honours it, so it would have taken pinch zoom from low-vision users to fix a + * problem only iOS has. + * + * `scale` is read defensively because older WebViews do not implement it, and treating its absence + * as zoomed would answer 0 for every keyboard on them. + * + * No `visualViewport` at all is 0 rather than a guess — that guard is in the effect below, which + * is also the only thing that can act on it, and a second copy here was unreachable. + */ +function occlusion(viewport: VisualViewport): number { + if ((viewport.scale ?? 1) !== 1) { + return 0 + } + return Math.max(0, window.innerHeight - (viewport.height + viewport.offsetTop)) +} + +export function useKeyboardOcclusion(): number { + const [keyboardLift, setKeyboardLift] = useState(0) + + useEffect(() => { + // Both shapes: `null` is what the DOM declares, `undefined` is a WebView without the property. + const viewport = window.visualViewport + if (viewport === null || viewport === undefined) { + return + } + const read = (): void => setKeyboardLift(occlusion(viewport)) + // Read once on mount: a composer opened while the keyboard is already up gets no event at all. + read() + viewport.addEventListener('resize', read) + viewport.addEventListener('scroll', read) + + return () => { + viewport.removeEventListener('resize', read) + viewport.removeEventListener('scroll', read) + } + }, []) + + return keyboardLift +} + +/** + * On the web the padding is the whole of the avoidance: `KeyboardAvoidingView` is driven by the + * `Keyboard` events this file exists because the page never receives. + */ +export function useKeyboardAvoidingPadding(): number { + return useKeyboardOcclusion() +} diff --git a/mobile/src/platform/text-input-font-size.test.ts b/mobile/src/platform/text-input-font-size.test.ts new file mode 100644 index 00000000000..89ce1494f28 --- /dev/null +++ b/mobile/src/platform/text-input-font-size.test.ts @@ -0,0 +1,57 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it, vi } from 'vitest' + +// The stylesheets are the subject, so react-native is stubbed down to what they touch rather than +// parsed: its entry point is Flow, which this runner does not read. +vi.mock('react-native', () => ({ + StyleSheet: { + create: (styles: Record) => styles, + hairlineWidth: 1 + } +})) + +import { listStyles } from '../source-control/mobile-source-control-list-styles' +import { mobileDiffReviewControlStyles } from '../components/mobile-diff-review-control-styles' +import { typography } from '../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from './text-input-font-size' +import { TEXT_INPUT_FONT_SIZE as WEB_TEXT_INPUT_FONT_SIZE } from './text-input-font-size.web' + +/** + * The size the two page-served text inputs carry, on each platform. + * + * Both halves are asserted from here because a node test resolves the native sibling, so the web + * value cannot be read off the style object: the bundler is what swaps the module, and that swap + * is the overrides census's subject rather than this file's. What this file can hold is that the + * two styles take their size from the seam at all, which is what makes the swap reach them. + */ +const MOBILE_ROOT = join(import.meta.dirname, '..', '..') +const STYLE_MODULES = [ + 'src/source-control/mobile-source-control-list-styles.ts', + 'src/components/mobile-diff-review-control-styles.ts' +] + +describe('the font size the page-served text inputs carry', () => { + it('clears the size iOS zooms the page for, on the web', () => { + // 16 is the floor; below it a focus zooms the document and the keyboard seam, which reads a + // scale other than 1 as no keyboard, stops lifting for the rest of the session. + expect(WEB_TEXT_INPUT_FONT_SIZE).toBeGreaterThanOrEqual(16) + }) + + it('leaves a phone rendering exactly what it rendered before', () => { + expect(TEXT_INPUT_FONT_SIZE).toBe(typography.bodySize) + expect(listStyles.commitInput.fontSize).toBe(typography.bodySize) + expect(mobileDiffReviewControlStyles.composerInput.fontSize).toBe(typography.bodySize) + }) + + it('takes that size from the seam in both styles, which is what the web build swaps', () => { + // Read as source: both modules resolve to the native constant here, so a style that went back + // to `typography.bodySize` would pass every assertion above and ship 14px to the web. + expect( + STYLE_MODULES.filter( + (module) => + !readFileSync(join(MOBILE_ROOT, module), 'utf8').includes('TEXT_INPUT_FONT_SIZE') + ) + ).toEqual([]) + }) +}) diff --git a/mobile/src/platform/text-input-font-size.ts b/mobile/src/platform/text-input-font-size.ts new file mode 100644 index 00000000000..5bf30aa26ca --- /dev/null +++ b/mobile/src/platform/text-input-font-size.ts @@ -0,0 +1,9 @@ +import { typography } from '../theme/mobile-theme' + +/** + * The font size a text input carries, which is a platform question and not a design one. + * + * A phone has no page to zoom, so this is the app's body size and the rendered input is exactly + * what it was before this module existed. The `.web.ts` sibling is where the difference lives. + */ +export const TEXT_INPUT_FONT_SIZE = typography.bodySize diff --git a/mobile/src/platform/text-input-font-size.web.ts b/mobile/src/platform/text-input-font-size.web.ts new file mode 100644 index 00000000000..dbb4d6d3ef9 --- /dev/null +++ b/mobile/src/platform/text-input-font-size.web.ts @@ -0,0 +1,21 @@ +import { typography } from '../theme/mobile-theme' + +/** Below this, iOS Safari and every iOS WebView zoom the page when an input takes focus. */ +const IOS_FOCUS_ZOOM_FLOOR = 16 + +/** + * Web sibling: the app's body size, raised to the size that stops the page being zoomed. + * + * iOS zooms on focus of any input under 16px and does not zoom back out, so the document the + * keyboard seam is measuring ends up at a scale other than 1 for the whole of a typing session. + * The seam answers 0 there on purpose — geometry cannot separate a zoom from a keyboard — so + * without this the commit bar and the note composer would never lift on the platform they were + * written for. + * + * The font size rather than `maximum-scale=1` on the viewport meta, which was the first fix and + * was wrong: Android WebView honours it and would have taken deliberate pinch zoom away from + * low-vision users to solve a problem only iOS has. + * + * `Math.max` rather than the constant, so a theme that raises the body size past 16 keeps it. + */ +export const TEXT_INPUT_FONT_SIZE = Math.max(typography.bodySize, IOS_FOCUS_ZOOM_FLOOR) diff --git a/mobile/src/source-control/mobile-source-control-list-styles.ts b/mobile/src/source-control/mobile-source-control-list-styles.ts index 8fd304e54d9..0bbb6559d77 100644 --- a/mobile/src/source-control/mobile-source-control-list-styles.ts +++ b/mobile/src/source-control/mobile-source-control-list-styles.ts @@ -1,5 +1,6 @@ import { StyleSheet } from 'react-native' import { colors, radii, spacing, typography } from '../theme/mobile-theme' +import { TEXT_INPUT_FONT_SIZE } from '../platform/text-input-font-size' // Changed-files list, section headers, file rows, and the commit bar. Split // from the main source-control stylesheet to stay under the line limit. @@ -138,7 +139,8 @@ export const listStyles = StyleSheet.create({ backgroundColor: colors.bgBase, color: colors.textPrimary, paddingHorizontal: spacing.md, - fontSize: typography.bodySize + // Through the seam: on the web this must clear the size at which iOS zooms the page on focus. + fontSize: TEXT_INPUT_FONT_SIZE }, commitInputDisabled: { backgroundColor: colors.bgPanel, diff --git a/mobile/src/source-control/use-mobile-source-control-keyboard-lift.ts b/mobile/src/source-control/use-mobile-source-control-keyboard-lift.ts index 7d3b274bcd0..a4606b5ad74 100644 --- a/mobile/src/source-control/use-mobile-source-control-keyboard-lift.ts +++ b/mobile/src/source-control/use-mobile-source-control-keyboard-lift.ts @@ -1,25 +1,12 @@ -import { useEffect, useState } from 'react' -import { Keyboard, Platform } from 'react-native' +import { useKeyboardOcclusion } from '../platform/keyboard-occlusion' +/** + * How far the commit bar sits above the bottom of the screen. + * + * The measurement moved to `platform/keyboard-occlusion`, which the review composer needs too and + * which the page answers from `visualViewport` because react-native-web's `Keyboard` never fires. + * This name stays because it is what the hub's state calls the number. + */ export function useMobileSourceControlKeyboardLift(): number { - const [keyboardLift, setKeyboardLift] = useState(0) - - useEffect(() => { - const showEvent = Platform.OS === 'ios' ? 'keyboardWillShow' : 'keyboardDidShow' - const hideEvent = Platform.OS === 'ios' ? 'keyboardWillHide' : 'keyboardDidHide' - - const onShow = Keyboard.addListener(showEvent, (event) => { - // Why: iOS keyboard height already describes the obscured screen area. - // Subtracting the safe-area inset lets the commit bar tuck under the keyboard. - setKeyboardLift(Math.max(0, event.endCoordinates.height)) - }) - const onHide = Keyboard.addListener(hideEvent, () => setKeyboardLift(0)) - - return () => { - onShow.remove() - onHide.remove() - } - }, []) - - return keyboardLift + return useKeyboardOcclusion() } diff --git a/mobile/web-entry/web-overrides.json b/mobile/web-entry/web-overrides.json index ce10822ea79..b7b3f07d806 100644 --- a/mobile/web-entry/web-overrides.json +++ b/mobile/web-entry/web-overrides.json @@ -25,6 +25,10 @@ "file": "app/h/[hostId]/index.web.tsx", "reason": "The shell renders this page for this route, so the page has no shell to mount inside itself and no flag to read; the switch already happened natively. Its native file reaches OrcaMobileWebShellView, whose requireNativeViewManager call runs at import and throws in a browser." }, + { + "file": "src/platform/text-input-font-size.web.ts", + "reason": "iOS zooms the page on focus of any text input under 16px and does not zoom back out, which leaves the document at a scale other than 1 for a whole typing session. keyboard-occlusion.web.ts reads a scale other than 1 as 'not a keyboard' on purpose, because geometry cannot separate a zoom from a keyboard, so the commit bar and the note composer would never lift on the platform they were written for. The web file raises the app's 14px body size to that floor; the native file is the body size, so a phone renders exactly what it rendered before. maximum-scale=1 on the viewport meta was the other fix and was rejected: Android WebView honours it and it would take deliberate pinch zoom away from low-vision users." + }, { "file": "src/platform/haptics.web.ts", "reason": "expo-haptics has a web build that fakes an iOS haptic by appending a hidden