Repository navigation
Conversation
Owner
Author
|
I pushed this PR before its local review ran. Round 1 has now run at
The fix applies the P3 simplification, which also removes the untested override. The nullable hook and |
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.
Owner
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 | nulllives inlib/breakpoints.ts, next toMD_UP_QUERY, the single boundary constant for the switch.nullmeans nothing can determine the mode, because the host has nomatchMedia(jsdom 30 is one such host). Call sites then keep the existingsx={{ display: { xs, md } }}media query, so behavior under jsdom 30 is unchanged.trueorfalseis the result of MUIuseMediaQuery(MD_UP_QUERY). In a browser it follows the viewport. In tests, the existingstubMatchMediahelper (src/test/matchMedia.ts) sets it, and the sharedvi.unstubAllGlobals()cleanup resets it after each test.phoneModeDisplayholds the null rule in one place.Converted switches: the AppLayout sidebar, the BottomNav tab bar and AuthShell's nextStep panels. Dialog is unchanged. Its
fullScreenOnPhonedecision still readsuseMediaQuery(MD_UP_QUERY)directly, andstubMatchMediaalready 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 alwaysmd: "none", and 98 tests fail with no way to reach the phone layout. AmatchMediastub cannot change how jsdom applies@mediarules, so the switch needs a literaldisplayvalue 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,
@mediarules apply at 1024px. On 30, jsdom ignores all@mediarules, so phone CSS never applied in tests before either.Verification
npm run typecheck: 0 errorsnpm run test: 184 files, 4,428 tests. Main has 183 files and 4,425 tests. The 3 new cases are inphoneModeSeam.test.tsx: phone, desktop, and nomatchMedia(both navs stay rendered).npm run lintnpm run buildMutations, each reverted:
usePhoneModealways returns null: the phone and desktop cases fail.Docs-impact: none — behavior-preserving refactor; no user-visible strings or help content change.