fix: restore picker theming and make the app shell responsive - #1693
Draft
shivoomiess wants to merge 10 commits into
Draft
fix: restore picker theming and make the app shell responsive#1693shivoomiess wants to merge 10 commits into
shivoomiess wants to merge 10 commits into
Conversation
- Migrate ChangeExperimentSafetyStatus Grid to MUI v6 size API - Wrap material-table action icons as function components for MUI v9 icon type compatibility - Move Typography fontWeight to sx (MUI v9) - Remove unused isSchedulerEnabled in OverviewPage - Apply prettier formatting from Grid codemod
The CI build failed with "Cannot find module @rollup/rollup-linux-x64-gnu". The lockfile had been regenerated on an arm64 Mac in a way that recorded only the darwin-arm64 optional dependency of every package that ships prebuilt binaries, so `npm ci` on ubuntu had no Linux binary to install. Restores the 92 missing entries across four families - @rollup/rollup-* (4.59.0), @esbuild/* (0.21.5), @unrs/resolver-binding-* (1.11.1, two trees) and @napi-rs/canvas-* (0.1.97) - taking version, resolved and integrity from develop's lockfile, which pins the identical versions and is installed by CI on every build. Restoring the entries rather than regenerating the lockfile is deliberate: a full regeneration drifted 230 unrelated packages (pdfjs-dist, graphql, luxon, eslint, x-date-pickers among them), which does not belong in a MUI upgrade. The diff is additions only - no existing entry changed or removed, and the relative order of existing keys is preserved. Verified with `npm ci` + `npm run build` in a linux/amd64 Node 22 container, matching the failing CI job.
@material-table/core@6.4.4 depends on @mui/x-date-pickers@^6.19.0, whose peer range is @mui/material ^5.8.6. With the app on v9 that no longer deduped, so npm installed a second, complete MUI v5 tree under @material-table/core. Two @mui/private-theming runtimes means two React ThemeContexts, so the app's ThemeProvider never reached material-table's internals. Every table's toolbar, pagination, sort labels, selection checkboxes and inline-edit fields rendered with stock MUI defaults instead of the palette built from backend settings and the `standard` input variant set in theme.tsx. develop had no nested @mui/material, so this was introduced by the v9 bump. Pins @mui/material and @mui/system to the app's version everywhere. Overriding @material-table/core's own dependency is not enough - its spec is ">=5.11.12", already satisfied by v9, so the nesting came entirely from the pickers peer. @mui/x-date-pickers deliberately stays at v6 for material-table: m-table-edit-field.js has a top-level require of "@mui/x-date-pickers/AdapterDateFnsV3", which exists in 6.20.2 but not in 9.x. That path is dead here - the app uses no material-table date columns. Also migrates 22 Checkbox `inputProps` to `slotProps.input` across 14 files. material-table forwards selectionProps/headerSelectionProps to Checkbox, which in v9 no longer accepts inputProps; these only type-checked before because they were resolving against the nested v5 types. Vendor chunk drops from 4083.87 kB to 3852.72 kB (1274.04 -> 1203.00 kB gzip). Verified with a clean `npm ci` reproducing the deduped tree, plus build and tsc --noEmit.
None of these were caught by `tsc` or `vite build`: the app passes MUI props
through Formik's `<Field component={...} />` and custom wrappers, whose loose
typing means removed props are neither rejected nor forwarded.
- People selector no longer crashes. Material UI v9 makes MenuItem throw
("MenuListContext is missing") outside a Menu or MenuList, and Autocomplete
renders its listbox as a plain <ul>. ProposalPeopleSelectorModal returned a
MenuItem from renderOption for every option, and NoOptionsText returned one
for an exact email match. Both now use ListItemButton. The throw is not
dev-only, so this affected production too.
- Restore ~90 dropped `inputProps`/`InputProps`. v9's TextField destructures
neither, so both fell through to the root FormControl and never reached the
input: `maxLength` stopped being enforced and `data-cy` attributes vanished.
Translated centrally in FormikUITextField (and FormikUICheckboxWithLabel) so
the call sites do not each need editing.
- Drop 16 `inputProps={{ placeholder }}` props on pickers. Since v8 these are
silently dropped (MUI X logs a warning saying so) and the section-based field
renders the format itself, so they were dead code.
- Pin react-is to ^18.3.1 via overrides. It was resolving to 19.2.7 against
React 18.3.1, which both the v6 and v7 guides call out as a runtime hazard in
prop-type checks.
- Restore `data-testid` on the table action icons the e2e suite selects. v7
strips it from @mui/icons-material in production builds and the suite runs
against `vite preview`, i.e. a production build.
- Update two e2e selectors: `.MuiTabs-flexContainer` became `.MuiTabs-list`, and
the invite chip's delete icon now carries an explicit data-cy.
- Type the pickers' `textField` slot as PickersTextFieldProps and drop the cast
that was hiding the v9 narrowing.
Merges the react-is pin into the overrides block alongside the MUI dedupe, so all dependency-resolution fixes live in one place, and regenerates the lockfile once rather than carrying three merged versions of it. Also gives the clone-workflow icon an explicit data-testid. It arrived with the develop merge written against pre-v9 assumptions: Material UI v7+ only emits the automatic data-testid outside production, and the e2e suite runs against a production build. Regeneration produced no drift - zero packages changed, added or removed - and the platform binaries and single @mui/private-theming runtime are intact.
Material UI v7+ only sets the automatic `data-testid` on icons outside production (createSvgIcon.js guards it on NODE_ENV), and the e2e suite runs against `vite preview`, i.e. a production build. FAPs.cy.ts drives detail panels through `[data-testid="ChevronRightIcon"]` in seven places, all of which would have failed once the build was green again. Every table's toggle comes from the shared `DetailPanel` entry in tableIcons, so setting it there covers all seven. Pagination's next-page arrow uses the same underlying icon but nothing selects it, so it is left alone. Also drops utils/materialTableIcon.ts. It was unused, and its premise - that material-table bundles an older copy of the MUI types - stopped being true once the trees were deduped.
Follow-up to the Material UI v5 -> v9 and x-date-pickers v7 -> v9 upgrade.
The upgrade itself was a faithful mechanical port; this picks up the layout
and theming work it left behind.
- theme: add MuiPickersTextField defaults. Since pickers v8 the fields render
their own PickersTextField rather than a MUI TextField, so they stopped
inheriting the MuiTextField `standard` variant and silently fell back to
`outlined`, leaving every date field looking different to every other input.
- theme: hoist `desktopModeMediaQuery` into MuiDatePicker/MuiDateTimePicker/
MuiTimePicker defaults and drop the 20 duplicated call-site props. All 20
passed the identical value. Resolves the long-standing NOTE in theme.tsx.
- PageLayout: the AppBar is 56px below `sm` and 64px from `sm` up, so the
hardcoded 64px offset left an 8px gap on phones. Also drops the fixed
`calc(100% - 250px)` width, which assumed the drawer was always expanded.
- FapGradeGuide: `sm={25}` was out of range and previously inert, because the
legacy Grid emitted a nonexistent class. Grid v2 computes width arithmetically,
so it became a real 208% overflow.
- Centralise the four hardcoded pixel media queries into hooks/common/
useResponsive.ts. Values are unchanged, so this is not a behavioural change;
it just gives the mobile work one place to retune.
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
2 participants
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.
Description
Follow-up to the Material UI v5 → v9 and x-date-pickers v7 → v9 upgrade in #1689. That PR was a
faithful mechanical port; this picks up the layout and theming work it left behind, and fixes two
regressions the port introduced that neither
tscnorvite buildcan catch.Motivation and Context
Two of these are genuine v9 regressions rather than improvements, and they are invisible to the
compiler:
PickersTextFieldinstead of a MUITextField, so they stopped inheriting theMuiTextFieldstandardvariant and silently fellback to
outlined— leaving every date field looking different to every other input in the app.FapGradeGuidepassessm={25}, which is out of range. Under the legacy Grid that was inertbecause it emitted a nonexistent class. Grid v2 computes width arithmetically, so it became a
real 208% overflow.
The rest is the groundwork for the upcoming mobile-friendly work: the app shell had hardcoded
desktop assumptions that are wrong on phones.
Changes
MuiPickersTextFielddefaults so date fields inherit thestandardvariantagain, matching every other input.
desktopModeMediaQueryintoMuiDatePicker/MuiDateTimePicker/MuiTimePickerdefaults and drop the 20 duplicated call-site props — all 20 passed the identical value. Resolves
the long-standing
NOTEintheme.tsx.smand 64px fromsmup, so the hardcoded 64px offsetleft an 8px gap on phones. Also drops the fixed
calc(100% - 250px)width, which assumed thedrawer was always expanded.
sm={25}described above.hooks/common/useResponsive.ts. Values are unchanged, so this is not a behavioural change — itjust gives the mobile work one place to retune.
How Has This Been Tested?
npm run lint(tsc --noEmit+ eslint) andnpm run buildpass on top of the rebasedchore/mui-upgrade.up as test failures.
Fixes Jira Issue
N/A
Depends On
#1689 — this branch is stacked on
chore/mui-upgradeand must not merge before it.Tests included/Docs Updated?