trevato/deal-clock

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRunsg rEventsFlows

Every page is checked to fit a phone screen without sideways scrolling #316

Merged masoncrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

mason/deal-clock:mason/305-every-page-is-checked-to-fit-a-phone-scr → main 2 commits fork 2 threads · 2 unresolved

  • Merged. mason/305-every-page-is-checked-to-fit-a-phone-scr was merged into main as 76d57b2.

Review

Sign in to review this change.
2 files changed +98 −7
src/phone.test.ts +82 −2
@@ -1,10 +1,18 @@
11 /** Review and a deal page stay readable on a phone: the narrow-screen rules exist and no fixed width is wide. */
2 import { expect, test } from "bun:test";
2 import { afterAll, expect, test } from "bun:test";
3 import { mkdtempSync } from "node:fs";
4 import { tmpdir } from "node:os";
35 import { join } from "node:path";
6 import { forgetRationing } from "./security.ts";
7 import { serve } from "./server.ts";
8 import { sign } from "./who.ts";
49
510 const css = await Bun.file(join(import.meta.dir, "..", "static", "app.css")).text();
611 const layout = await Bun.file(join(import.meta.dir, "pages", "layout.ts")).text();
712
13 /** Every `selector { body }` pair in a stylesheet. */
14 const RULE = /([^{}@]+)\{([^{}]*)\}/g;
15
816 const narrow = (): string => {
917 const at = css.search(/@media \(max-width: 30rem\)/);
1018 expect(at).toBeGreaterThanOrEqual(0);
@@ -31,7 +39,7 @@ test("the tap size is at least 44px", async () => {
3139
3240 test("no rule for Review or deal-page classes sets a fixed width over 360px", () => {
3341 const classes = ["card", "narrow", "move", "draft", "drafts", "draft-text", "draft-body", "draft-who", "dates", "date-row", "parties", "input", "button", "actions", "stack", "pair"];
34 for (const [, selector = "", body = ""] of css.matchAll(/([^{}@]+)\{([^{}]*)\}/g)) {
42 for (const [, selector = "", body = ""] of css.matchAll(RULE)) {
3543 if (!classes.some((c) => new RegExp(`\\.${c}(?![\\w-])`).test(selector))) continue;
3644 for (const [, prop = "", value = ""] of body.matchAll(/(?:^|;|\s)((?:min-)?width)\s*:\s*([^;]+)/g)) {
3745 const px = /^(\d+(?:\.\d+)?)(px|rem)$/.exec(value.trim());
@@ -41,3 +49,75 @@ test("no rule for Review or deal-page classes sets a fixed width over 360px", ()
4149 }
4250 }
4351 });
52
53 // Issue #305: every page a realtor opens on a phone between showings, at 360px.
54
55 const KEY = "a-key-only-the-forge-holds";
56 const server = serve({ port: 0, dataDir: mkdtempSync(join(tmpdir(), "app-phone-")), identityKey: KEY });
57 afterAll(() => server.stop(true));
58 const identity = `u1|alice|user|${Date.now() + 600_000}`;
59 const alice = { "x-forge-identity": identity, "x-forge-signature": sign(identity, KEY) };
60 const adminId = `u2|root|admin|${Date.now() + 600_000}`;
61 const root = { "x-forge-identity": adminId, "x-forge-signature": sign(adminId, KEY) };
62 const get = (path: string, headers: Record<string, string> = alice) => fetch(`http://${server.hostname}:${server.port}${path}`, { headers });
63
64 test("the clock, a deal, People, Review, the week and Activity each carry the viewport tag", async () => {
65 forgetRationing();
66 const sample = await (await get("/")).text();
67 const id = /href="\/deals\/(\d+)"/.exec(sample)?.[1];
68 expect(id).toBeDefined();
69 for (const path of ["/", `/deals/${id}`, "/people", "/reminders/today", "/week", "/activity"]) {
70 const res = await get(path, path === "/activity" ? root : alice);
71 expect({ path, status: res.status }).toEqual({ path, status: 200 });
72 expect({ path, tag: (await res.text()).includes('<meta name="viewport" content="width=device-width, initial-scale=1">') }).toEqual({ path, tag: true });
73 }
74 });
75
76 const rules = [...css.replace(/\/\*[\s\S]*?\*\//g, "").matchAll(RULE)].map(([, selector = "", body = ""]) => ({ selector: selector.trim(), body }));
77
78 test("no rule anywhere fixes a width above 360px unless it scrolls or wraps", () => {
79 for (const { selector, body } of rules) {
80 if (/overflow-x:\s*(auto|scroll)|overflow-wrap|max-width:\s*100%/.test(body)) continue;
81 for (const [, prop = "", value = ""] of body.matchAll(/(?:^|;|\s)((?:min-)?width)\s*:\s*([^;]+)/g)) {
82 const px = /^(\d+(?:\.\d+)?)(px|rem)$/.exec(value.trim());
83 if (px === null) continue;
84 expect({ selector, prop, wide: Number(px[1]) * (px[2] === "rem" ? 16 : 1) > 360 }).toEqual({ selector, prop, wide: false });
85 }
86 }
87 });
88
89 test("every no-wrap component either lives in the clock's scroller or wraps again on a phone", () => {
90 const inScroller = ["tick", "today span", "marker", "visually-hidden", "legend"];
91 const phone = narrow().replace(/\/\*[\s\S]*?\*\//g, "");
92 const unwrapped = new Set<string>();
93 for (const [, selector = "", body = ""] of phone.matchAll(RULE))
94 if (/white-space:\s*normal/.test(body)) for (const one of selector.split(",")) unwrapped.add(one.trim());
95 for (const { selector, body } of rules) {
96 if (!/white-space:\s*nowrap/.test(body)) continue;
97 for (const one of selector.split(",").map((s) => s.trim())) {
98 const name = one.replace(/^\./, "").replace(/:.*$/, "");
99 if (inScroller.some((c) => name === c || name.startsWith(`${c} `))) continue;
100 const resets = unwrapped.has(one);
101 expect({ one, resets }).toEqual({ one, resets: true });
102 }
103 }
sentinelcrew outdated

This test passes even though .side stays nowrap through the cascade. Check source order as well: the reset must come after the base rule. Also add a test for the .pill/.badge rule.
104 });
105
106 test("the client timeline's fixed columns collapse to one below 40rem", () => {
107 const at = css.indexOf("@media (max-width: 40rem) {\n .timeline-row");
108 expect(at).toBeGreaterThan(0);
109 expect(css.slice(at, at + 120)).toContain("grid-template-columns: 1fr");
110 });
111
112 test("the phone block gives pills and badges a cap and a break, and comes after every base rule it resets", () => {
113 const at = css.search(/@media \(max-width: 30rem\)/);
114 const phone = narrow();
115 expect(phone).toMatch(/\.pill,\s*\.badge \{[^}]*max-width: 100%[^}]*overflow-wrap: anywhere/);
116 // Same specificity: the later rule wins, so no base rule for a reset selector may follow the block.
117 for (const m of css.matchAll(RULE)) {
118 if (m.index === undefined || m.index < at) continue;
119 const later = m[1]!.replace(/\/\*[\s\S]*?\*\//g, "").trim();
120 if (later === "") continue;
121 expect({ later, nowrap: /white-space:\s*nowrap/.test(m[2]!) && /^\.(you|side|button|state)$/.test(later) }).toEqual({ later, nowrap: false });
122 }
123 });
static/app.css +16 −5
@@ -989,6 +989,12 @@ a.marker {
989989 display: none;
990990 }
991991
992 /* Which side of a deal the agent is on: a text label beside the address, no box of its own to shift. */
993 .side {
994 font-weight: 600;
995 white-space: nowrap;
996 }
997
992998 /* Phone width (about 480px): Review and a deal's dates stack and wrap, no sideways scroll. */
993999 @media (max-width: 30rem) {
9941000 .move .button,
@@ -1016,6 +1022,16 @@ a.marker {
10161022 .state {
10171023 white-space: normal;
10181024 }
sentinelcrew outdated

This .side reset is overridden by .side { white-space: nowrap } declared later (~line 1055). Move that base rule above this media block, or this does nothing.
1025 .you,
1026 .side,
1027 .button {
1028 white-space: normal;
1029 }
1030 .pill,
1031 .badge {
1032 max-width: 100%;
1033 overflow-wrap: anywhere;
1034 }
10191035 .actions {
10201036 flex-direction: column;
10211037 }
@@ -1051,11 +1067,6 @@ a.marker {
10511067 font-variant-numeric: tabular-nums;
10521068 }
10531069
1054 /* Which side of a deal the agent is on: a text label beside the address, no box of its own to shift. */
1055 .side {
1056 font-weight: 600;
1057 white-space: nowrap;
1058 }
10591070 .side-edit {
10601071 display: flex;
10611072 flex-wrap: wrap;

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs