Skip to content

Commit 01252a4

Browse files
fix(console): the approvals flow at phone width (44 px controls, a responsive long inbox) (#29170)
domain:ui#4 · zhuangjianguo · session_01QsR2jSaU3EpGXaXapmYT4Y <!-- claude-projects-attribution: {"github_login":"zhuangjianguo"} --> _Requested by **Steedos** · [project thread](https://claude.ai/code/project/chan_018ETKaNwMZCNRYD7D8Q9Ubb?thread=cmsg_018ETKaNwMZCNRYD7D8Q9UbbDyYguJHjXSQNjX44fttyQn)_ Fixes #29070 Clause-②: no Metadata-protocol: none ## 维护者速读 - **改了什么**:审批收件箱在手机宽度(低于 768 px)下,页签、搜索框、筛选、卡片上的批准/拒绝按钮、行内确认弹窗按钮,以及记录页审批面板的“发送提醒”,触控高度统一到 44 px;收件箱很长时,打开确认框和输入拒绝理由不再让整张列表重渲染。 - **为什么改**:移动 App 用 WebView 承载 Console,手机审批人只能用 375 px 的 Console。实测这些控件只有 24–40 px,200 条待办在 4 倍 CPU 降速下输入 20 个字的拒绝理由要 8.4 秒。 - **风险与回滚**:桌面尺寸不变(同一组三元式的桌面分支原样保留,单测钉住)。列表改为 useMemo 记忆化并让搜索走 useDeferredValue,若出现列表不刷新可整 PR 回滚,无数据或协议改动。 - **先看**:`ui/apps/console/src/pages/system/ApprovalsInboxPage.tsx`(新组件 InlineDecisionDialogs 与 rowFaces 的依赖数组)、`ui/e2e/approvals-inbox-phone.spec.ts`、`ui/packages/app-shell/src/views/RecordApprovalsPanel.tsx`。 - **两分钟验证**:手机尺寸(375×812)打开 `/apps/<应用>/system/approvals`,卡片上的批准/拒绝按钮与搜索框高度为 44 px;点“拒绝”输入理由,输入跟手,提交后时间线上出现该理由。 - **需要你做的**:无,席位落地。 ## What changes The run record below drove the Approvals flow in Chromium (Playwright, `isMobile` + touch, DPR 2) at 375×812 and 390×844 against the showcase (`pnpm dev`, three seeded pending requests) and the CRM (`dev:crm`, which declares no approval flow, so its inbox is the empty state). Scroll, dialog fit and the emulated keyboard passed; touch targets and the 4× CPU check failed. Both are fixed here. - **Touch targets (inbox).** Below 768 px (`useIsMobile`) the inbox's tabs (`min-h-11 min-w-11`, the list grows to hold them), search field, the three filter triggers (`h-11`), the phone cards' Approve / Reject, the empty state's Browse all requests and Load more (Button `lg`, 44 px) take the touch height. A desktop keeps `h-8` / `sm` exactly as before. - **Touch targets (confirmations).** The inline approve / reject confirmations' Cancel / Approve / Reject take Button's `lg` size on a phone (`buttonVariants({ variant: null, size: 'lg' })`, so each keeps its own variant). They were 40 px. - **Touch target (record page).** The approval panel's Send reminder takes `lg` on a phone; it was a 24 px chip. - **No zoom on focus.** The search field no longer forces `text-sm` on a phone, so it keeps Input's own `text-base md:text-sm` (16 px). A focused field under 16 px makes iOS zoom the page. - **A long inbox at 4× CPU.** Three changes, measured one by one: 1. `InlineDecisionDialogs` owns the confirmation's target row and Reject reason (it was page state), and the page opens it through a ref. Opening a confirmation or typing the reason now re-renders the two dialogs, not every row. 2. Both row faces (desktop table, phone cards) are one `useMemo` (`rowFaces`) over what they read, and the pending tab's client-side filter follows `useDeferredValue(query)`, so a keystroke in the search box repaints the box first and the rows after, interruptibly. 3. Phone cards carry `content-visibility: auto` (`contain-intrinsic-size: auto 9rem`), so a modal's open does not lay out off-screen cards. ## Run record Showcase app `showcase_app`; CRM app `crm_app`. Pages: inbox `/apps/APP/system/approvals` (My Pending and All tabs), the inline approve and reject confirmations opened from a phone card, the request's own page (`sys_approval_request/record/ID`) and its Reject dialog, and two business records with a pending request (`showcase_expense_report`, viewer is an approver; `showcase_invoice`, viewer is the submitter, so Send reminder shows). **Before** (origin/main 0e43df2) | Check | 375×812 | 390×844 | Measured | |:--|:--|:--|:--| | No horizontal page scroll | pass (showcase, CRM) | pass (showcase, CRM) | `documentElement.scrollWidth` equals the viewport on every page above | | Touch targets ≥ 44 px | **fail** (showcase, CRM) | **fail** (showcase, CRM) | tabs 32–34 px tall, All 40 px wide; search 32; filters 32; card Approve / Reject 28; confirmation buttons 40; Send reminder 24; CRM empty-state Browse all requests 36 | | Dialogs fit the viewport | pass (showcase) | pass (showcase) | approve 375×250 at top 281; reject 375×340 at top 236; request-page Reject 375×608 at top 102 (390: 390×608 at 118) | | Keyboard does not cover the field | pass (showcase), see note | pass (showcase), see note | viewport shrunk by 336 px with the reason box focused: box at 189–279 of 476 visible; Reject at 335 | | Long list at 4× CPU | **fail** (showcase) | **fail** (showcase) | 200 rows: tap Approve to dialog 916 ms (longest task 716 ms); 20-char reason 8364 ms, a 350–440 ms task per keystroke; 3-char search 1689 ms (659 ms task); scroll 0 long tasks | **After** (this branch) | Check | 375×812 | 390×844 | Measured | |:--|:--|:--|:--| | No horizontal page scroll | pass (showcase, CRM) | pass (showcase, CRM) | unchanged | | Touch targets ≥ 44 px | pass for the flow (showcase, CRM) | pass for the flow (showcase, CRM) | every inbox, confirmation and panel control ≥ 44 px; the only control under 44 px left on these pages is shared chrome (see Acceptance notes) | | Dialogs fit the viewport | pass (showcase) | pass (showcase) | unchanged geometry, footer buttons 44 px | | Keyboard does not cover the field | pass (showcase) | pass (showcase) | reason box and search field render 16 px text, so iOS does not zoom on focus (the search field was 14 px before) | | Long list at 4× CPU | pass (showcase) | pass (showcase) | 200 rows: tap to dialog 369 / 303 ms (longest task 118 / 119 ms); 20-char reason 655 / 661 ms, no long task; 3-char search 289 / 356 ms, longest task 145 / 105 ms, the filtered list lands (134 cards); scroll 0 long tasks | <details><summary>How the run was taken</summary> - Geometry, dialogs and keyboard: the Vite console on source (`ui/apps/console`, `DEV_PROXY_TARGET` at a `--fresh` showcase / CRM backend), signed in as the seeded admin. Touch targets are every visible `button`, link, input, tab, combobox and checkbox, split into page content and the top bar. - Keyboard: Playwright has no soft keyboard, so the check focuses the field, types, and shrinks the viewport by 336 px (an iPhone keyboard), then reads whether the field and the dialog's submit button stay inside the visible height. The font-size reading is the iOS focus-zoom condition. - CPU: the production console bundle (`pnpm console:build`, served by the backend at `/_console`), CDP `Emulation.setCPUThrottlingRate` 4, the approvals list response inflated to 200 rows in the page's own network layer (the pending tab is unpaged, so 200 pending requests is a reachable inbox), long tasks from a `PerformanceObserver`. The CRM has no request to inflate, so this row is showcase only. A pass is no long task over 200 ms while scrolling, deciding, typing a reason or searching. - Before and after screenshots of every page and dialog above, and the raw JSON of both runs, are in the project folder, not on GitHub. </details> ## Tests - `ui/e2e/approvals-inbox-phone.spec.ts` (new, Playwright, the `record-path-phone-row.spec.ts` shape): reads the phone arm of every size the inbox and panel choose and the components' base classes from source, merges them with the components' own `tailwind-merge`, compiles real Tailwind, and asserts at 375×812 and 390×844 no horizontal scroll, every control ≥ 44×44 px, and the Reject confirmation inside the viewport. A third case builds the desktop arm and asserts it measures under 44 px, so the floor bites. Local run: 3 passed. Ablation: Send reminder's phone arm set back to `sm` turns both width cases red ("send reminder" is 36px tall); restored, `git diff HEAD` empty. - `ApprovalsInboxPage.phoneWidth-29070.test.tsx` (new): phone vs desktop size arms; opening Reject and typing the reason performs zero per-row reads (`isDeletedRecordReference`, which every row face calls) and still sends the reason; the `a` key still opens the approve confirmation. Ablation: with `origin/main`'s page the isolation case reads 150 extra rows ("expected 210 to be 60") and the size case fails. - `RecordApprovalsPanel.phoneReminder-29070.test.tsx` (new): Send reminder takes `h-11` on a phone and keeps `h-6` on a desktop. - Every `ApprovalsInboxPage*.test.tsx` and `RecordApprovalsPanel*.test.tsx`: 25 files, 130 tests passed (locked, `--maxWorkers=2`). - `turbo run type-check --filter=@object-ui/console --filter=@object-ui/app-shell` (72 tasks) and `pnpm --dir ui run type-check:e2e`: exit 0. - eslint on the two source files: the warning set equals `origin/main`'s (no new warning; the new `useMemo` dependency list is complete). ## Acceptance notes Findings, not fixed here; the seat routes each (ruling in the seat thread): - **Shared top bar (Navigation, #29075).** At 375 and 390 px on every page of the flow: sidebar toggle 28×28, home 32×32, app chip 34×30, search 32×32, Open app in Studio 32×32, inbox bell 32×32, account menu 32×32. - **Console page header Refresh (#29075).** `ConsolePageHeader`'s Refresh icon button is 32×32 on the inbox (shared with object lists and dashboards). - **Request page decision bar (Record, #29071).** On `sys_approval_request/record/ID` the record header's `DeclaredActionsBar` buttons (Approve, Reject, Reassign, Send back, Request info, Recall, Reply) are 36 px tall; the back link is 28 px; Details / Timeline tabs 38 px. - **Request page Reject dialog (#29071 and #29078).** The declared-action dialog's Confirm / Cancel are 40 px and Take photo 36 px (#29071); its close X is 16×16 from `components/src/ui/dialog.tsx`; that is card #29107 (domain:ui#2), the Dialog twin of the Sheet close change that #29078 owns. Not touched, per the card's file surface: `sheet.tsx`, `dialog.tsx`, `navigation-overlay.tsx`, `record-list-face.tsx`, `ObjectView.tsx`, `recordOpenPreference.ts`, `ui/packages/mobile/**`, `ui/apps/console/src/{main,App,AppContent}.tsx`. ## Verification - Local derived gates (`dispatch-gates --commands`, 34 commands) on head `708d7368`: 34 run, 34 exit 0; `dispatch-gates --ran` reconciles 34 derived, 0 NOT-MEASURED, 0 UNRUN. One note: `pnpm --dir ui exec vitest run scripts/` was SIGKILLed in the unlocked sweep (default workers on a shared 4-core box); rerun alone under the verify lock with `--maxWorkers=2`: 169 files passed, 2 skipped, 4900 tests, `VERDICT command-exit 0`. - Merged `origin/main` again right before opening (head `d7428a01`, 12 commits, none in this PR's files); the two new unit files re-run there: 2 files, 6 tests passed. - `node scripts/check-sdui-manifest.mjs`: exit 0 (no registry change). - Diff: 6 files, about 1330 changed lines, under 3000. --- _Generated by [Claude Code](https://claude.ai/code/session_01QsR2jSaU3EpGXaXapmYT4Y)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 07eaec2 commit 01252a4

6 files changed

Lines changed: 995 additions & 333 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@object-ui/console': patch
3+
'@object-ui/app-shell': patch
4+
---
5+
6+
fix(console): the approvals flow works at phone width — 44 px controls, and a long inbox stays responsive
7+
8+
Clause-②: no
9+
Metadata-protocol: none
10+
11+
The mobile app hosts the Console in a WebView, so a phone approver gets the Console at 375 px. Measured in Chromium at 375×812 and 390×844 on the showcase and CRM examples:
12+
13+
- **Touch targets.** Below 768 px the approvals inbox's tabs, search field, filters, the phone cards' Approve / Reject and the inline approve / reject confirmations' buttons take a 44 px height (Button's `lg` size, `h-11` where a control has no size); they measured 28–40 px. The record page approval panel's Send reminder does too (it was 24 px). A desktop keeps every size it had.
14+
- **No zoom on focus.** The inbox search field keeps the 16 px text inputs use on a phone, so focusing it no longer makes iOS zoom the page.
15+
- **A long inbox stays responsive.** The inline confirmations now hold their own row and Reject reason, the rows are memoised, and the pending list follows the search box at a deferred priority. At 200 pending requests on a phone at 4× CPU, typing a 20-character reject reason went from 8.4 s to 0.5 s, opening a confirmation from about 1 s to 0.3 s, and the longest task while deciding or searching from about 850 ms to 120 ms.
Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectstack#29070 — the approvals inbox at phone width.
5+
*
6+
* The run record drove the inbox in Chromium at 375×812 and 390×844 against
7+
* the showcase and CRM examples. Two things failed, and this file pins both
8+
* fixes; the geometry itself is pinned in `ui/e2e/approvals-inbox-phone.spec.ts`.
9+
*
10+
* 1. **Touch targets.** The tabs, search, filters and the phone cards'
11+
* Approve / Reject measured 28–34 px and the inline confirmation's buttons
12+
* 40 px. Below 768 px (`useIsMobile`) each takes the 44 px height: Button's
13+
* `lg` size, `h-11` where the control has no size. The search field keeps
14+
* Input's own 16 px phone text, since a smaller focused field makes iOS
15+
* zoom the page. A desktop keeps every size it had.
16+
* 2. **A long list at 4× CPU.** The inline confirmation's target row and its
17+
* Reject reason lived on the page, so opening the dialog and every
18+
* keystroke of the reason re-rendered every row (200 rows: ~1 s from tap to
19+
* dialog, ~400 ms per keystroke). They now live in the dialog component,
20+
* so neither re-renders a row. The probe here is the per-row
21+
* `isDeletedRecordReference` read, which each face of each row calls when
22+
* it renders.
23+
*/
24+
25+
import '@testing-library/jest-dom/vitest';
26+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
27+
import { render, screen, cleanup, fireEvent, waitFor, within } from '@testing-library/react';
28+
import { MemoryRouter, Routes, Route } from 'react-router-dom';
29+
30+
const APP = 'account';
31+
32+
const { approvalsApiStub, ADAPTER, AUTH, I18N, MOBILE, rowReads } = vi.hoisted(() => {
33+
const base = {
34+
process_name: 'invoice_approval',
35+
process_label: 'Invoice Approval',
36+
object_name: 'showcase_invoice',
37+
object_label: 'Invoice',
38+
status: 'pending',
39+
pending_approvers: ['u_1'],
40+
submitted_at: '2026-10-20T00:00:00.000Z',
41+
submitter_id: 'u_2',
42+
submitter_name: 'Sam Submitter',
43+
viewer: { can_act: true, is_submitter: false, can_override: true },
44+
};
45+
const rows = Array.from({ length: 6 }, (_, i) => ({
46+
...base,
47+
id: `req_${i}`,
48+
record_id: `inv_${i}`,
49+
record_title: `Invoice ${i}`,
50+
}));
51+
const approvalsApiStub = {
52+
listRequests: vi.fn(async () => ({ data: rows })),
53+
getRequest: vi.fn(async (id: string) => ({ data: rows.find((r) => r.id === id) })),
54+
listActions: vi.fn(async () => ({ data: [] })),
55+
approve: vi.fn(async () => ({ data: rows[0], finalized: true })),
56+
reject: vi.fn(async () => ({ data: rows[0], finalized: true })),
57+
};
58+
// STABLE singletons: a fresh identity per render re-runs the load effect forever.
59+
const ADAPTER = {
60+
find: vi.fn(async (_object: string, params?: Record<string, unknown>) => {
61+
const ids = (params?.$filter as { id?: { $in?: string[] } } | undefined)?.id?.$in ?? [];
62+
return { data: ids.map((id) => ({ id })) };
63+
}),
64+
};
65+
const AUTH = { user: { id: 'u_1', email: 'ada@example.com' } };
66+
const I18N = {
67+
t: (key: string, options?: Record<string, unknown>) => String(options?.defaultValue ?? key),
68+
language: 'en',
69+
};
70+
return { approvalsApiStub, ADAPTER, AUTH, I18N, MOBILE: { value: false }, rowReads: { count: 0 } };
71+
});
72+
73+
vi.mock('@object-ui/components', async (importOriginal) => ({
74+
...(await importOriginal<Record<string, unknown>>()),
75+
useIsMobile: () => MOBILE.value,
76+
}));
77+
78+
vi.mock('./deadRecordReference', async (importOriginal) => {
79+
const real = await importOriginal<typeof import('./deadRecordReference')>();
80+
return {
81+
...real,
82+
isDeletedRecordReference: (...args: Parameters<typeof real.isDeletedRecordReference>) => {
83+
rowReads.count++;
84+
return real.isDeletedRecordReference(...args);
85+
},
86+
};
87+
});
88+
89+
vi.mock('@object-ui/i18n', async (importOriginal) => ({
90+
...(await importOriginal<Record<string, unknown>>()),
91+
useObjectTranslation: () => I18N,
92+
}));
93+
94+
vi.mock('@object-ui/auth', async (importOriginal) => {
95+
const authFetch = vi.fn(async () => new Response('{}', { status: 200 }));
96+
return {
97+
...(await importOriginal<Record<string, unknown>>()),
98+
useAuth: () => AUTH,
99+
createAuthenticatedFetch: () => authFetch,
100+
TokenStorage: { get: () => null },
101+
};
102+
});
103+
104+
vi.mock('@object-ui/app-shell', async (importOriginal) => ({
105+
...(await importOriginal<Record<string, unknown>>()),
106+
useAdapter: () => ADAPTER,
107+
DeclaredActionsBar: () => null,
108+
isViaOverrideRow: () => false,
109+
}));
110+
111+
vi.mock('sonner', () => ({
112+
toast: Object.assign(vi.fn(), {
113+
success: vi.fn(), error: vi.fn(), info: vi.fn(),
114+
warning: vi.fn(), loading: vi.fn(), dismiss: vi.fn(),
115+
}),
116+
}));
117+
118+
vi.mock('../../services/approvalsApi', async (importOriginal) => ({
119+
...(await importOriginal<Record<string, unknown>>()),
120+
approvalsApi: approvalsApiStub,
121+
}));
122+
123+
import { ApprovalsInboxPage } from './ApprovalsInboxPage';
124+
125+
function renderInbox() {
126+
return render(
127+
<MemoryRouter initialEntries={[`/apps/${APP}/system/approvals`]}>
128+
<Routes>
129+
<Route path="/apps/:appName/system/approvals" element={<ApprovalsInboxPage />} />
130+
</Routes>
131+
</MemoryRouter>,
132+
);
133+
}
134+
135+
/** The phone card holding `title` (a card is not a table row). */
136+
async function cardFor(title: string): Promise<HTMLElement> {
137+
await screen.findAllByText(title, undefined, { timeout: 8000 });
138+
const card = screen.getAllByText(title).map((el) => el.closest('.md\\:hidden > *')).find(Boolean);
139+
if (!card) throw new Error(`no card for ${title}`);
140+
return card as HTMLElement;
141+
}
142+
143+
const classes = (el: Element) => (el.getAttribute('class') ?? '').split(/\s+/);
144+
145+
beforeEach(() => {
146+
for (const fn of Object.values(approvalsApiStub)) fn.mockClear();
147+
rowReads.count = 0;
148+
});
149+
afterEach(() => {
150+
cleanup();
151+
MOBILE.value = false;
152+
});
153+
154+
describe('the inbox controls take 44 px below 768 px (objectstack#29070)', () => {
155+
it('phone: tabs, search, filters, card buttons and the confirmation buttons', async () => {
156+
MOBILE.value = true;
157+
renderInbox();
158+
const card = await cardFor('Invoice 0');
159+
for (const name of ['Approve', 'Reject']) {
160+
expect(classes(within(card).getByRole('button', { name }))).toContain('h-11');
161+
}
162+
for (const tab of screen.getAllByRole('tab')) {
163+
expect(classes(tab)).toEqual(expect.arrayContaining(['min-h-11', 'min-w-11']));
164+
}
165+
const search = screen.getByPlaceholderText('Search record, process, requester…');
166+
expect(classes(search)).toContain('h-11');
167+
// Input's own `text-base md:text-sm`: 16 px on a phone, so iOS does not zoom.
168+
expect(classes(search)).not.toContain('text-sm');
169+
for (const trigger of screen.getAllByRole('combobox')) expect(classes(trigger)).toContain('h-11');
170+
171+
fireEvent.click(within(card).getByRole('button', { name: 'Reject' }));
172+
const dialog = await screen.findByRole('alertdialog');
173+
for (const name of ['Cancel', 'Reject']) {
174+
expect(classes(within(dialog).getByRole('button', { name }))).toContain('h-11');
175+
}
176+
});
177+
178+
it('desktop: every control keeps the size it had', async () => {
179+
renderInbox();
180+
await cardFor('Invoice 0');
181+
const search = screen.getByPlaceholderText('Search record, process, requester…');
182+
expect(classes(search)).toEqual(expect.arrayContaining(['h-8', 'text-sm']));
183+
for (const trigger of screen.getAllByRole('combobox')) expect(classes(trigger)).toContain('h-8');
184+
for (const tab of screen.getAllByRole('tab')) expect(classes(tab)).not.toContain('min-h-11');
185+
const row = screen.getAllByRole('row').find((r) => within(r).queryByText('Invoice 0'))!;
186+
fireEvent.click(within(row).getByRole('button', { name: 'Reject' }));
187+
const dialog = await screen.findByRole('alertdialog');
188+
expect(classes(within(dialog).getByRole('button', { name: 'Reject' }))).toContain('h-10');
189+
});
190+
});
191+
192+
describe('the inline confirmation does not re-render the rows (objectstack#29070)', () => {
193+
it('opening Reject and typing the reason reads no row; the decision still carries the reason', async () => {
194+
MOBILE.value = true;
195+
renderInbox();
196+
const card = await cardFor('Invoice 3');
197+
// Not vacuous: the rows really do read the probe when they render.
198+
await waitFor(() => expect(rowReads.count).toBeGreaterThan(0));
199+
const before = rowReads.count;
200+
201+
fireEvent.click(within(card).getByRole('button', { name: 'Reject' }));
202+
const dialog = await screen.findByRole('alertdialog');
203+
const box = within(dialog).getByRole('textbox', { name: 'Comment (optional)' });
204+
for (const value of ['O', 'Ov', 'Ove', 'Over the cap']) fireEvent.change(box, { target: { value } });
205+
expect(box).toHaveValue('Over the cap');
206+
expect(rowReads.count, 'opening the confirmation or typing the reason re-rendered the rows').toBe(before);
207+
208+
fireEvent.click(within(dialog).getByRole('button', { name: 'Reject' }));
209+
await waitFor(() => expect(approvalsApiStub.reject).toHaveBeenCalledTimes(1));
210+
expect(approvalsApiStub.reject).toHaveBeenCalledWith('req_3', { actor_id: 'u_1', comment: 'Over the cap' });
211+
});
212+
213+
it('the a key still opens the approve confirmation for the focused row', async () => {
214+
renderInbox();
215+
await cardFor('Invoice 0');
216+
fireEvent.keyDown(window, { key: 'j' });
217+
fireEvent.keyDown(window, { key: 'a' });
218+
const dialog = await screen.findByRole('alertdialog');
219+
expect(within(dialog).getByText('Approve "{{title}}"?')).toBeInTheDocument();
220+
fireEvent.click(within(dialog).getByRole('button', { name: 'Approve' }));
221+
await waitFor(() => expect(approvalsApiStub.approve).toHaveBeenCalledWith('req_0', { actor_id: 'u_1' }));
222+
});
223+
});

0 commit comments

Comments
 (0)