[internal] Use @base-ui/utils/platform for platform detection#22710
[internal] Use @base-ui/utils/platform for platform detection#22710romgrk wants to merge 2 commits into
Conversation
| // XXX: discuss this | ||
| // detect if user agent has Android version < 10 or iOS version < 13 | ||
| const mobileVersionMatches = | ||
| typeof navigator !== 'undefined' && navigator.userAgent.match(/android\s(\d+)|OS\s(\d+)/i); | ||
| const androidVersion = | ||
| mobileVersionMatches && mobileVersionMatches[1] ? parseInt(mobileVersionMatches[1], 10) : null; | ||
| const iOSVersion = | ||
| mobileVersionMatches && mobileVersionMatches[2] ? parseInt(mobileVersionMatches[2], 10) : null; | ||
| export const slowAnimationDevices = | ||
| (androidVersion && androidVersion < 10) || (iOSVersion && iOSVersion < 13) || false; |
There was a problem hiding this comment.
Recommendation: drop the OS-version sniff and default to prefers-reduced-motion.
Why dropping it is the right call
- The Android <10 / iOS <13 cutoff is 2018-era; that cohort is negligible in 2026.
- It's already quietly a no-op thanks to UA reduction. Concretely, both branches are dead:
- Android (Chrome): the UA freezes the version to
Android 10and model toK, soandroidVersion < 10is permanently false even for older devices runningAndroid 1-9. - iOS/iPadOS: iPadOS sends a Mac/desktop UA with no
iPhone OS NNtoken, and Safari freezes version reporting — the iOS branch never matches.
- Android (Chrome): the UA freezes the version to
prefers-reduced-motionis the standardized signal for "this user/device wants less motion," and it's increasingly set on lower-end and accessibility-configured devices.- The explicit
reduceAnimationsprop already exists as the escape hatch for anyone who needs to force it.
Why not capability signals
navigator.deviceMemoryis Chromium-only — absent in Safari/Firefox, so you'd get asymmetric behavior across the exact browsers (iOS Safari) the old code most cared about.navigator.hardwareConcurrencyis a weak proxy: cheap-but-fine phones report 8 cores, some capable devices under-report, and any threshold is a guess.- Net, we'd trade a decaying heuristic for a live but noisy one — still "guess the device," which is the anti-pattern. Not an improvement.
export function useReduceAnimations(customReduceAnimations?: boolean) {
const prefersReduced = useMediaQuery(PREFERS_REDUCED_MOTION, { defaultMatches: false });
return customReduceAnimations ?? prefersReduced;
}Caveats: this is a behavior change (not a refactor).
There was a problem hiding this comment.
Do you have source for:
prefers-reduced-motion ... it's increasingly set on lower-end and accessibility-configured devices.
It doesn't seem true to me. I would think the flag is about preference, not capability 🤔
It's not a blocker or anything, just a comment. I think the changes look fine, they don't affect charts though 😆
There was a problem hiding this comment.
Two thoughts:
- a lagging animation is a worse UX than no animation, if the device's hardware is too slow, the expected behavior is no animations.
- "prefers-reduced-motion" is not about no motions, it's about reduced motions, for example, see how macOS behaves when it's enabled. Though, in practice, people tends to side on the edge of caution and aggressively temper animations when this flag is set.
There was a problem hiding this comment.
The Android <10 / iOS <13 cutoff is 2018-era; that cohort is negligible in 2026.
It's already quietly a no-op thanks to UA reduction. Concretely, both branches are dead:
- Android (Chrome): the UA freezes the version to
Android 10and model toK, soandroidVersion < 10is permanently false even for older devices runningAndroid 1-9.- iOS/iPadOS: iPadOS sends a Mac/desktop UA with no
iPhone OS NNtoken, and Safari freezes version reporting — the iOS branch never matches.
If this is actually the case (and based on Claude exploration it mostly is, with some arguments slightly imprecisely formatted), there is no point in keeping the current behavior.
IMHO, we can ship it in a minor if there is a clear need (or issue) with existing behavior.
Otherwise, treat it as a small BC and ship in v10. 👌
There was a problem hiding this comment.
I have verified Claude's claims about Android on two actual devices, the navigator version is indeed set to Android 10 in both cases. Chrome is evergreen on all Android devices, so in theory all Android devices now report Android 10 no matter what their OS version is.
The iOS point isn't worded very clearly, but originally it also contained the fact that iOS < 13 is a shrinking-to-zero cohort.
It doesn't seem true to me. I would think the flag is about preference, not capability
I've done more digging on that, and the wording is confusing: it is indeed increasingly set but by the users themselves.
IMHO, we can ship it in a minor if there is a clear need (or issue) with existing behavior. Otherwise, treat it as a small BC and ship in v10.
I don't have an opinion on that. I prefer removing it now as I'm cleaning up that code, but I'm not going to push for it. I'll only leave a comment unless someone from the pickers team supports the change.
LukasTy
left a comment
There was a problem hiding this comment.
LGTM with a few observations.
Claude review
Blocking (external dependency, not a code defect)
| # | Item | Detail |
|---|---|---|
| 1 | Unreleased ./platform subpath |
The cataloged and latest-published @base-ui/utils is 0.2.9, whose exports has no ./platform entry. Every import { platform } from '@base-ui/utils/platform' therefore fails to resolve, which is what turns typecheck/unit/browser/build/package CI red. Unblock by publishing a base-ui version that exposes ./platform, then bump the catalog (pnpm-workspace.yaml:18, currently ^0.2.9). |
| 2 | Lockfile not regenerated | @base-ui/utils was added to 5 package.json files but pnpm-lock.yaml is untouched, so pnpm install --frozen-lockfile fails on its own. Run pnpm install (+ pnpm dedupe) and commit the lockfile once the new version is cataloged. |
Suggestions
| # | File | Suggestion |
|---|---|---|
| 1 | KeyboardManager.ts:107 (and confirm useGridCellSelection.ts:306) |
Real behavior change worth a conscious decision: base-ui os.mac is !ios && platform.startsWith('mac') and excludes iPadOS (it routes platform === 'macintel' && maxTouchPoints > 1 to ios), whereas the old navigator.platform.includes('Mac') treated iPad as Mac. So in the cross-platform ControlOrMeta check, an iPad with a keyboard now maps to Control instead of Meta/Cmd. If iPad should keep Cmd semantics, use platform.os.apple (= mac |
What Looks Good
- Equivalence verified against base-ui source:
engine.gecko= Firefox,os.android/os.macas expected, andenv.jsdom = /jsdom|happydom/so HappyDOM is still matched (no regression vs the old/jsdom|HappyDOM/). @mui/x-internalsuses a"./*": "./src/*/index.ts"wildcard export, so deleting the platform source needs no export-map cleanup.- Dependency hygiene is right: the 4 newly-importing packages declare
@base-ui/utils, x-tree-view-pro already had it, and root gets it as a devDependency for thetest/utils/skipIfhelper. - Good cleanup: dead
isAndroidexport removed and telemetry/formatNumbertests redirected to the sharedskipIfhelper, cutting duplicated detection logic.
Summary
Replaces MUI X's ad-hoc platform/navigator detection with the new
@base-ui/utils/platformmodule, removing the duplicated in-house detection code.Changes
Removed old detection code:
packages/x-internals/src/platform/index.ts(isFirefox,isJSDOM)packages/x-data-grid/src/utils/isJSDOM.ts(duplicateisJSDOM)isAndroidexport inuseField.utils.tsand the localisAndroidin tree-view-pro'sitemPlugin.tsMigrated to
@base-ui/utils/platformflags:platform.isFirefoxplatform.engine.geckoplatform.isJSDOM/isJSDOMplatform.env.jsdomnavigator.platformMac checksplatform.os.macisAndroid()platform.os.androidTouched:
x-virtualizer,x-data-grid,x-data-grid-premium,x-internal-gestures,x-tree-view-pro.Tests: redirected telemetry +
formatNumbertests to the sharedtest/utils/skipIfhelper; refactoredskipIf.tsitself (isJSDOM→env.jsdom,isOSX→os.mac) and anavigator.platformcheck in the cell-selection test.Dependencies: added
@base-ui/utilsto the consuming packages and to root devDependencies (for the test helper).Notes
useReduceAnimations.tsis intentionally left untouched — it needs OS version numbers, which the boolean platform module doesn't express. Whether to keep/replace that heuristic is a separate behavior decision.platformsubpath only exists in the not-yet-released@base-ui/utils, so wiring it up (linking/rebuilding the local checkout) is still pending — CI typecheck/tests won't pass until then.eslintpasses.🤖 Generated with Claude Code