Skip to content

refactor(web): forceable phone mode for the responsive switch - #1437

Open
mforce wants to merge 3 commits into
mainfrom
refactor/web-phone-mode-seam
Open

mforce wants to merge 3 commits into
mainfrom
refactor/web-phone-mode-seam

Conversation

@mforce

@mforce mforce commented Nov 14, 2026 •

Copy link
Copy Markdown
Owner

This PR prepares the jsdom 31 upgrade (#1426). I chose to refactor first and upgrade second. After this PR, tests can pick the phone or desktop layout.

What

usePhoneMode(): boolean | null lives in lib/breakpoints.ts, next to MD_UP_QUERY, the single boundary constant for the switch.

  • null means nothing can determine the mode, because the host has no matchMedia (jsdom 30 is one such host). Call sites then keep the existing sx={{ display: { xs, md } }} media query, so behavior under jsdom 30 is unchanged.
  • true or false is the result of MUI useMediaQuery(MD_UP_QUERY). In a browser it follows the viewport. In tests, the existing stubMatchMedia helper (src/test/matchMedia.ts) sets it, and the shared vi.unstubAllGlobals() cleanup resets it after each test.

phoneModeDisplay holds the null rule in one place.

Converted switches: the AppLayout sidebar, the BottomNav tab bar and AuthShell's nextStep panels. Dialog is unchanged. Its fullScreenOnPhone decision still reads useMediaQuery(MD_UP_QUERY) directly, and stubMatchMedia already controls that. Layout-only switches (the FieldConsole grid, the CustomersPage column) stay media-based.

Why

jsdom 31 evaluates CSS media queries against a frozen 1024×768 viewport. Its CSS environment is a fixed Object.freezed value, so tests cannot change it. Under jsdom 31 the tab bar is always md: "none", and 98 tests fail with no way to reach the phone layout. A matchMedia stub cannot change how jsdom applies @media rules, so the switch needs a literal display value when the mode is known. The jsdom bump itself is the follow-up PR that closes #1426.

A probe in the jsdom 31 worktree confirmed this. On 31, @media rules apply at 1024px. On 30, jsdom ignores all @media rules, so phone CSS never applied in tests before either.

Verification

  • npm run typecheck: 0 errors
  • npm run test: 184 files, 4,428 tests. Main has 183 files and 4,425 tests. The 3 new cases are in phoneModeSeam.test.tsx: phone, desktop, and no matchMedia (both navs stay rendered).
  • npm run lint
  • npm run build

Mutations, each reverted:

  • usePhoneMode always returns null: the phone and desktop cases fail.
  • BottomNav ignores the hook: the desktop case fails. Under jsdom 30 no phone-side assertion can tell the two versions apart, because jsdom 30 ignores media queries.

Docs-impact: none — behavior-preserving refactor; no user-visible strings or help content change.

@mforce

mforce commented Nov 14, 2026

Copy link
Copy Markdown
Owner Author

I pushed this PR before its local review ran. Round 1 has now run at 60a11740 and found two problems. A fix is in progress, and this PR is not ready to merge until it lands.

  • P2: no test covers the forced-phone override in Dialog.tsx:300. With phoneMode ?? !mdUp changed back to !mdUp, all 28 seam and Dialog tests still pass.
  • P3: setPhoneModeForTests adds mutable test state the repo does not need. The existing src/test/matchMedia.ts helper already controls useMediaQuery, and the shared test cleanup resets it. A probe that removed the setter and the Dialog override, and stubbed matchMedia in the seam tests, passed all 28 tests.

The fix applies the P3 simplification, which also removes the untested override. The nullable hook and phoneModeDisplay stay. A matchMedia stub cannot change how jsdom evaluates CSS media rules, so jsdom 31 still needs the seam.

Remove setPhoneModeForTests and the Dialog override; seam tests now pick phone or desktop with the existing stubMatchMedia helper, reset by the shared vi.unstubAllGlobals cleanup. Drop the window-undefined check: the app has no SSR path.
@mforce

mforce commented Nov 14, 2026

Copy link
Copy Markdown
Owner Author

Commit f4a061d fixes both round-1 findings. I removed setPhoneModeForTests and the Dialog override, and the seam tests now pick phone or desktop through the existing stubMatchMedia helper. Review round 2 on f4a061d found nothing. The full web suite passes: 184 files, 4,428 tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant