diff --git a/.changeset/shared-apple-platform-detection.md b/.changeset/shared-apple-platform-detection.md new file mode 100644 index 000000000000..6a5a6d9903ad --- /dev/null +++ b/.changeset/shared-apple-platform-detection.md @@ -0,0 +1,18 @@ +--- +'@astryxdesign/core': patch +--- + +[fix] Platform detection reads the client-hints `Unknown` sentinel as no answer, and lives in one place + +Follow-up to #5325, which taught `useHotkeys` and `Kbd` to fall through to +`navigator.platform` when `userAgentData.platform` is blank. `Unknown` is the +User-Agent Client Hints spec's own value for "cannot say", and it names a +platform no more than `''` does, yet it still committed to the client-hints +branch and answered "not Apple". It now falls through the same way. + +The two detections were independent copies kept aligned by a docstring. They +are now one internal util that both import, so the next change to this logic +cannot land on one surface and miss the other. The util is deliberately not +named in `utils/index.ts`, which would publish it as API. + +@Astro-Han diff --git a/packages/core/src/Kbd/Kbd.test.tsx b/packages/core/src/Kbd/Kbd.test.tsx index c371a35930f8..65fc7e07250b 100644 --- a/packages/core/src/Kbd/Kbd.test.tsx +++ b/packages/core/src/Kbd/Kbd.test.tsx @@ -23,7 +23,6 @@ describe('Kbd', () => { value: originalPlatform, configurable: true, }); - delete (navigator as {userAgentData?: unknown}).userAgentData; }); it('renders a single key', () => { @@ -55,22 +54,6 @@ describe('Kbd', () => { expect(screen.getByText('\u2318')).toBeInTheDocument(); // \u2318 }); - it('reads a blank userAgentData.platform as unknown, not as non-Mac', () => { - // Builds that rewrite their client-hints identity expose the key with an - // empty value; navigator.platform is the only surface left that answers. - Object.defineProperty(navigator, 'userAgentData', { - value: {platform: ''}, - configurable: true, - }); - Object.defineProperty(navigator, 'platform', { - value: 'MacIntel', - configurable: true, - }); - - render(); - expect(screen.getByText('\u2318')).toBeInTheDocument(); - }); - it('maps modifier keys to symbols', () => { render(); expect(screen.getByText('\u2303')).toBeInTheDocument(); // \u2303 diff --git a/packages/core/src/Kbd/Kbd.tsx b/packages/core/src/Kbd/Kbd.tsx index c5b576e82a0e..20fc5ce20a11 100644 --- a/packages/core/src/Kbd/Kbd.tsx +++ b/packages/core/src/Kbd/Kbd.tsx @@ -16,6 +16,7 @@ import React, {useSyncExternalStore} from 'react'; import * as stylex from '@stylexjs/stylex'; import {mergeProps} from '../utils'; +import {isApplePlatform} from '../utils/isApplePlatform'; import type {BaseProps} from '../BaseProps'; import {themeProps} from '../utils/themeProps'; import { @@ -122,31 +123,6 @@ function getServerPlatformSnapshot(): boolean { return false; } -/** - * Detects whether the current platform is macOS/iOS. - * Prefers the User-Agent Client Hints API when it names a platform (modern - * Chrome/Edge), falls back to navigator.platform (deprecated but universally - * supported) when it is absent or blank. - */ -function detectMac(): boolean { - if (typeof navigator === 'undefined') { - return false; - } - // Prefer User-Agent Client Hints API (not deprecated) - const uaData = 'userAgentData' in navigator ? navigator.userAgentData : null; - if (uaData && typeof uaData === 'object' && 'platform' in uaData) { - const uaPlatform = (uaData as {platform?: unknown}).platform; - // A blank platform is no answer, not a negative one. Builds that rewrite - // their client-hints identity ship '', so fall through rather than - // reading it as "not Apple". - if (typeof uaPlatform === 'string' && uaPlatform.trim() !== '') { - return /mac/i.test(uaPlatform); - } - } - // Fallback: navigator.platform (deprecated but still shipped everywhere) - return /Mac|iPhone|iPad|iPod/.test(navigator.platform ?? ''); -} - export interface KbdProps extends BaseProps { ref?: React.Ref; /** @@ -183,7 +159,7 @@ export interface KbdProps extends BaseProps { export function Kbd({keys, ref, xstyle, className, style, ...rest}: KbdProps) { const isMac = useSyncExternalStore( subscribeToPlatformChanges, - detectMac, + isApplePlatform, getServerPlatformSnapshot, ); diff --git a/packages/core/src/hooks/useHotkeys.test.ts b/packages/core/src/hooks/useHotkeys.test.ts index 19e2cf774a06..8bff8087eccf 100644 --- a/packages/core/src/hooks/useHotkeys.test.ts +++ b/packages/core/src/hooks/useHotkeys.test.ts @@ -67,24 +67,6 @@ describe('useHotkeys', () => { expect(onPress).toHaveBeenCalledTimes(1); }); - it('reads a blank userAgentData.platform as unknown, not as non-Apple', () => { - // Builds that rewrite their client-hints identity expose the key with an - // empty value; navigator.platform is the only surface left that answers. - vi.stubGlobal('navigator', { - userAgentData: {platform: ''}, - platform: 'MacIntel', - }); - const onPress = vi.fn(); - renderHook(() => useHotkeys([{keys: 'mod+k', onPress}])); - - press('k', {ctrlKey: true}); - expect(onPress).not.toHaveBeenCalled(); - - const event = press('k', {metaKey: true}); - expect(onPress).toHaveBeenCalledTimes(1); - expect(onPress).toHaveBeenCalledWith(event); - }); - it('does not fire a bare key when modifiers are held', () => { stubApplePlatform(); const onPress = vi.fn(); diff --git a/packages/core/src/hooks/useHotkeys.ts b/packages/core/src/hooks/useHotkeys.ts index a8a9f1b60ae3..e1b5e59c33ee 100644 --- a/packages/core/src/hooks/useHotkeys.ts +++ b/packages/core/src/hooks/useHotkeys.ts @@ -18,7 +18,8 @@ * unless a hotkey opts in via `allowInInputs`. * * Platform-aware: `mod` maps to metaKey (⌘) on Apple platforms and - * ctrlKey elsewhere — mirrors the detection used by Kbd. + * ctrlKey elsewhere, through the same isApplePlatform util Kbd reads, so + * displayed and handled shortcuts cannot disagree. * * SYNC: When modified, update: * - /packages/core/src/hooks/index.ts @@ -26,6 +27,7 @@ */ import {useEffect, useRef} from 'react'; +import {isApplePlatform} from '../utils/isApplePlatform'; /** * A single keyboard shortcut registration. @@ -71,30 +73,6 @@ const KEY_ALIASES: Record = { plus: '+', }; -/** - * Detects whether the current platform is macOS/iOS. - * Prefers the User-Agent Client Hints API when it names a platform (modern - * Chrome/Edge), falls back to navigator.platform (deprecated but universally - * supported) when it is absent or blank. - * Mirrors the detection used by Kbd so displayed and handled shortcuts agree. - */ -function isApplePlatform(): boolean { - if (typeof navigator === 'undefined') { - return false; - } - const uaData = 'userAgentData' in navigator ? navigator.userAgentData : null; - if (uaData && typeof uaData === 'object' && 'platform' in uaData) { - const uaPlatform = (uaData as {platform?: unknown}).platform; - // A blank platform is no answer, not a negative one. Builds that rewrite - // their client-hints identity ship '', so fall through rather than - // reading it as "not Apple". - if (typeof uaPlatform === 'string' && uaPlatform.trim() !== '') { - return /mac/i.test(uaPlatform); - } - } - return /Mac|iPhone|iPad|iPod/.test(navigator.platform ?? ''); -} - /** * Whether the event target is a typing surface where global shortcuts * should stay silent by default. diff --git a/packages/core/src/utils/isApplePlatform.test.ts b/packages/core/src/utils/isApplePlatform.test.ts new file mode 100644 index 000000000000..7dc464aa1f6b --- /dev/null +++ b/packages/core/src/utils/isApplePlatform.test.ts @@ -0,0 +1,61 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file isApplePlatform.test.ts + * @input Uses vitest, isApplePlatform + * @output Unit tests for isApplePlatform + * @position Testing; validates isApplePlatform.ts implementation + */ + +import {describe, it, expect, vi, afterEach} from 'vitest'; +import {isApplePlatform} from './isApplePlatform'; + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('isApplePlatform', () => { + it('trusts a client-hints platform that names one', () => { + vi.stubGlobal('navigator', { + userAgentData: {platform: 'macOS'}, + platform: 'Win32', + }); + expect(isApplePlatform()).toBe(true); + + vi.stubGlobal('navigator', { + userAgentData: {platform: 'Windows'}, + platform: 'MacIntel', + }); + expect(isApplePlatform()).toBe(false); + }); + + it.each([ + ['blank', ''], + ['whitespace', ' '], + ['the spec Unknown sentinel', 'Unknown'], + ['unknown in any case', 'UNKNOWN'], + ['a non-string', null], + ])('falls through to navigator.platform on %s', (_label, platform) => { + vi.stubGlobal('navigator', { + userAgentData: {platform}, + platform: 'MacIntel', + }); + expect(isApplePlatform()).toBe(true); + + vi.stubGlobal('navigator', {userAgentData: {platform}, platform: 'Win32'}); + expect(isApplePlatform()).toBe(false); + }); + + it('falls back to navigator.platform when client hints are absent', () => { + vi.stubGlobal('navigator', {platform: 'iPhone'}); + expect(isApplePlatform()).toBe(true); + + vi.stubGlobal('navigator', {platform: 'Linux x86_64'}); + expect(isApplePlatform()).toBe(false); + }); + + it('answers false when there is no navigator at all', () => { + vi.stubGlobal('navigator', undefined); + expect(isApplePlatform()).toBe(false); + }); +}); diff --git a/packages/core/src/utils/isApplePlatform.ts b/packages/core/src/utils/isApplePlatform.ts new file mode 100644 index 000000000000..b716acda7b71 --- /dev/null +++ b/packages/core/src/utils/isApplePlatform.ts @@ -0,0 +1,37 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file isApplePlatform.ts + * @input Reads navigator.userAgentData.platform and navigator.platform + * @output Exports isApplePlatform + * @position Internal util; the single platform detection behind useHotkeys and Kbd + * + * Deliberately absent from utils/index.ts: packages/core/src/index.ts does + * `export * from './utils'`, so naming it there would publish it as API. + * Import it by path, as interactionModality is imported. + */ + +/** + * Detects whether the current platform is macOS/iOS. + * Prefers the User-Agent Client Hints API when it names a platform (modern + * Chrome/Edge), falls back to navigator.platform (deprecated but universally + * supported) when it names none. + */ +export function isApplePlatform(): boolean { + if (typeof navigator === 'undefined') { + return false; + } + const uaData = 'userAgentData' in navigator ? navigator.userAgentData : null; + if (uaData && typeof uaData === 'object' && 'platform' in uaData) { + const uaPlatform = (uaData as {platform?: unknown}).platform; + const named = typeof uaPlatform === 'string' ? uaPlatform.trim() : ''; + // A value that names nothing is no answer, not a negative one: builds that + // rewrite their client-hints identity ship '', and 'Unknown' is the spec's + // own sentinel for "cannot say". Both fall through rather than reading as + // "not Apple". + if (named !== '' && named.toLowerCase() !== 'unknown') { + return /mac/i.test(named); + } + } + return /Mac|iPhone|iPad|iPod/.test(navigator.platform ?? ''); +}