From 1252fe65f4bcd53bdf0de2ccfb34af0020f553d0 Mon Sep 17 00:00:00 2001 From: atomiks Date: Fri, 28 Feb 2025 13:59:14 +1100 Subject: [PATCH 1/7] [NumberField] Correct virtual cursor rendering --- .../number-field/root/useNumberFieldRoot.ts | 4 +- .../react/src/number-field/root/useScrub.ts | 16 +++- .../NumberFieldScrubAreaCursor.test.tsx | 73 ++++++++++++++++++- .../NumberFieldScrubAreaCursor.tsx | 16 ++-- 4 files changed, 96 insertions(+), 13 deletions(-) diff --git a/packages/react/src/number-field/root/useNumberFieldRoot.ts b/packages/react/src/number-field/root/useNumberFieldRoot.ts index a60829ef07..73ed452410 100644 --- a/packages/react/src/number-field/root/useNumberFieldRoot.ts +++ b/packages/react/src/number-field/root/useNumberFieldRoot.ts @@ -884,7 +884,7 @@ export namespace UseNumberFieldRoot { */ value?: number | null; /** - * The uncontrolled value of the field when it’s initially rendered. + * The uncontrolled value of the field when it's initially rendered. * * To render a controlled number field, use the `value` prop instead. */ @@ -929,6 +929,8 @@ export namespace UseNumberFieldRoot { inputValue: string; value: number | null; isScrubbing: boolean; + isTouchInput: boolean; + isPointerLockDenied: boolean; inputRef: ((instance: HTMLInputElement | null) => void) | null; scrubHandleRef: React.RefObject; scrubAreaRef: React.RefObject; diff --git a/packages/react/src/number-field/root/useScrub.ts b/packages/react/src/number-field/root/useScrub.ts index 91160b31b7..ea2b75fff2 100644 --- a/packages/react/src/number-field/root/useScrub.ts +++ b/packages/react/src/number-field/root/useScrub.ts @@ -31,6 +31,8 @@ export function useScrub(params: ScrubParams) { const [isScrubbing, setIsScrubbing] = React.useState(false); const [cursorTransform, setCursorTransform] = React.useState(''); + const [isTouchInput, setIsTouchInput] = React.useState(false); + const [isPointerLockDenied, setIsPointerLockDenied] = React.useState(false); React.useEffect(() => { return () => { @@ -126,6 +128,9 @@ export function useScrub(params: ScrubParams) { return; } + const isTouch = event.pointerType === 'touch'; + setIsTouchInput(isTouch); + if (event.pointerType === 'mouse') { event.preventDefault(); inputRef.current?.focus(); @@ -135,7 +140,7 @@ export function useScrub(params: ScrubParams) { onScrubbingChange(true, event.nativeEvent); // WebKit causes significant layout shift with the native message, so we can't use it. - if (!isWebKit()) { + if (!isTouch && !isWebKit()) { // There can be some frames where there's no cursor at all when requesting the pointer lock. // This is a workaround to avoid flickering. avoidFlickerTimeoutRef.current = window.setTimeout(async () => { @@ -146,8 +151,9 @@ export function useScrub(params: ScrubParams) { // We need to await it even though it doesn't appear to return a promise in the // types in order for the `catch` to work. await ownerDocument(scrubAreaRef.current).body.requestPointerLock(); - } catch { - // + setIsPointerLockDenied(false); + } catch (e) { + setIsPointerLockDenied(true); } }, 20); } @@ -275,12 +281,14 @@ export function useScrub(params: ScrubParams) { return React.useMemo( () => ({ isScrubbing, + isTouchInput, + isPointerLockDenied, getScrubAreaProps, getScrubAreaCursorProps, scrubAreaCursorRef, scrubAreaRef, scrubHandleRef, }), - [isScrubbing, getScrubAreaProps, getScrubAreaCursorProps], + [isScrubbing, isTouchInput, isPointerLockDenied, getScrubAreaProps, getScrubAreaCursorProps], ); } diff --git a/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx b/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx index 483b6032ca..9f8f62614a 100644 --- a/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx +++ b/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx @@ -1,14 +1,17 @@ import * as React from 'react'; import { expect } from 'chai'; -import { screen } from '@mui/internal-test-utils'; +import { screen, fireEvent, act } from '@mui/internal-test-utils'; import { NumberField } from '@base-ui-components/react/number-field'; import { createRenderer, describeConformance } from '#test-utils'; import { isWebKit } from '../../utils/detectBrowser'; import { NumberFieldRootContext } from '../root/NumberFieldRootContext'; +import sinon from 'sinon'; -const testContext = { +const defaultTestContext: NumberFieldRootContext = { getScrubAreaCursorProps: (externalProps) => externalProps, isScrubbing: true, + isTouchInput: false, + isPointerLockDenied: false, state: { value: null, required: false, @@ -30,7 +33,7 @@ describe('', () => { refInstanceof: window.HTMLSpanElement, render: async (node) => { return render( - + {node} , ); @@ -45,4 +48,68 @@ describe('', () => { ); expect(screen.queryByRole('presentation')).not.to.equal(null); }); + + it('renders when using mouse input', async () => { + const { user } = await render( + + + + + , + ); + + const scrubArea = screen.getByTestId('scrub-area'); + + await user.pointer({ target: scrubArea, keys: '[MouseLeft>]', pointerName: 'mouse' }); + + expect(screen.queryByTestId('scrub-area-cursor')).not.to.equal(null); + }); + + it('does not render when using touch input', async () => { + const { user } = await render( + + + + + , + ); + + const scrubArea = screen.getByRole('presentation'); + + await user.pointer({ target: scrubArea, keys: '[TouchA>]', pointerName: 'touch' }); + + expect(screen.queryByTestId('scrub-area-cursor')).to.equal(null); + }); + + it('handles pointer lock denial through requestPointerLock API', async () => { + const originalRequestPointerLock = Element.prototype.requestPointerLock; + + try { + Element.prototype.requestPointerLock = sinon + .stub() + .throws(new Error('User denied pointer lock')); + + const { user } = await render( + + + + + , + ); + + const scrubArea = screen.getByRole('presentation'); + + await act(async () => { + await user.pointer({ target: scrubArea, keys: '[MouseLeft>]', pointerName: 'mouse' }); + await new Promise((resolve) => setTimeout(resolve, 25)); + }); + + expect(screen.queryByTestId('scrub-area-cursor')).to.equal(null); + + const requestLockStub = Element.prototype.requestPointerLock as sinon.SinonStub; + expect(requestLockStub.called).to.equal(true); + } finally { + Element.prototype.requestPointerLock = originalRequestPointerLock; + } + }); }); diff --git a/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.tsx b/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.tsx index 95918dc81e..c9911f17a4 100644 --- a/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.tsx +++ b/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.tsx @@ -25,8 +25,14 @@ const NumberFieldScrubAreaCursor = React.forwardRef(function NumberFieldScrubAre ) { const { render, className, ...otherProps } = props; - const { isScrubbing, scrubAreaCursorRef, state, getScrubAreaCursorProps } = - useNumberFieldRootContext(); + const { + isScrubbing, + isTouchInput, + isPointerLockDenied, + scrubAreaCursorRef, + state, + getScrubAreaCursorProps, + } = useNumberFieldRootContext(); const [element, setElement] = React.useState(null); @@ -41,7 +47,7 @@ const NumberFieldScrubAreaCursor = React.forwardRef(function NumberFieldScrubAre extraProps: otherProps, }); - if (!isScrubbing || isWebKit()) { + if (!isScrubbing || isWebKit() || isTouchInput || isPointerLockDenied) { return null; } @@ -64,11 +70,11 @@ NumberFieldScrubAreaCursor.propTypes /* remove-proptypes */ = { children: PropTypes.node, /** * CSS class applied to the element, or a function that - * returns a class based on the component’s state. + * returns a class based on the component's state. */ className: PropTypes.oneOfType([PropTypes.func, PropTypes.string]), /** - * Allows you to replace the component’s HTML element + * Allows you to replace the component's HTML element * with a different tag, or compose it with another component. * * Accepts a `ReactElement` or a function that returns the element to render. From 367ec12e61d4d2b3a6c2f39965410563fad55050 Mon Sep 17 00:00:00 2001 From: atomiks Date: Fri, 28 Feb 2025 14:00:09 +1100 Subject: [PATCH 2/7] proptypes --- packages/react/src/number-field/root/NumberFieldRoot.tsx | 2 +- .../scrub-area-cursor/NumberFieldScrubAreaCursor.tsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react/src/number-field/root/NumberFieldRoot.tsx b/packages/react/src/number-field/root/NumberFieldRoot.tsx index c5c1d83964..b1e1d0d472 100644 --- a/packages/react/src/number-field/root/NumberFieldRoot.tsx +++ b/packages/react/src/number-field/root/NumberFieldRoot.tsx @@ -155,7 +155,7 @@ NumberFieldRoot.propTypes /* remove-proptypes */ = { */ className: PropTypes.oneOfType([PropTypes.func, PropTypes.string]), /** - * The uncontrolled value of the field when it’s initially rendered. + * The uncontrolled value of the field when it's initially rendered. * * To render a controlled number field, use the `value` prop instead. */ diff --git a/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.tsx b/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.tsx index c9911f17a4..86a345a829 100644 --- a/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.tsx +++ b/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.tsx @@ -70,11 +70,11 @@ NumberFieldScrubAreaCursor.propTypes /* remove-proptypes */ = { children: PropTypes.node, /** * CSS class applied to the element, or a function that - * returns a class based on the component's state. + * returns a class based on the component’s state. */ className: PropTypes.oneOfType([PropTypes.func, PropTypes.string]), /** - * Allows you to replace the component's HTML element + * Allows you to replace the component’s HTML element * with a different tag, or compose it with another component. * * Accepts a `ReactElement` or a function that returns the element to render. From eaf9bbb292e092a6c4c3ca5f0f3aea1213dda36e Mon Sep 17 00:00:00 2001 From: atomiks Date: Fri, 28 Feb 2025 14:01:04 +1100 Subject: [PATCH 3/7] proptypes --- packages/react/src/number-field/root/NumberFieldRoot.tsx | 2 +- packages/react/src/number-field/root/useNumberFieldRoot.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react/src/number-field/root/NumberFieldRoot.tsx b/packages/react/src/number-field/root/NumberFieldRoot.tsx index b1e1d0d472..c5c1d83964 100644 --- a/packages/react/src/number-field/root/NumberFieldRoot.tsx +++ b/packages/react/src/number-field/root/NumberFieldRoot.tsx @@ -155,7 +155,7 @@ NumberFieldRoot.propTypes /* remove-proptypes */ = { */ className: PropTypes.oneOfType([PropTypes.func, PropTypes.string]), /** - * The uncontrolled value of the field when it's initially rendered. + * The uncontrolled value of the field when it’s initially rendered. * * To render a controlled number field, use the `value` prop instead. */ diff --git a/packages/react/src/number-field/root/useNumberFieldRoot.ts b/packages/react/src/number-field/root/useNumberFieldRoot.ts index 73ed452410..6f53664ab5 100644 --- a/packages/react/src/number-field/root/useNumberFieldRoot.ts +++ b/packages/react/src/number-field/root/useNumberFieldRoot.ts @@ -884,7 +884,7 @@ export namespace UseNumberFieldRoot { */ value?: number | null; /** - * The uncontrolled value of the field when it's initially rendered. + * The uncontrolled value of the field when it’s initially rendered. * * To render a controlled number field, use the `value` prop instead. */ From 4e46e22540643d99c055080b2c50cdb70f8e0d49 Mon Sep 17 00:00:00 2001 From: atomiks Date: Fri, 28 Feb 2025 14:01:42 +1100 Subject: [PATCH 4/7] Lint --- .../scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx b/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx index 9f8f62614a..4dd550bd65 100644 --- a/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx +++ b/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx @@ -1,11 +1,11 @@ import * as React from 'react'; import { expect } from 'chai'; -import { screen, fireEvent, act } from '@mui/internal-test-utils'; +import { screen, act } from '@mui/internal-test-utils'; +import sinon from 'sinon'; import { NumberField } from '@base-ui-components/react/number-field'; import { createRenderer, describeConformance } from '#test-utils'; import { isWebKit } from '../../utils/detectBrowser'; import { NumberFieldRootContext } from '../root/NumberFieldRootContext'; -import sinon from 'sinon'; const defaultTestContext: NumberFieldRootContext = { getScrubAreaCursorProps: (externalProps) => externalProps, From a798d9a4b72957e1ecd353b2d888935eecc74a8c Mon Sep 17 00:00:00 2001 From: atomiks Date: Fri, 28 Feb 2025 14:02:51 +1100 Subject: [PATCH 5/7] Timeouts --- .../NumberFieldScrubAreaCursor.test.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx b/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx index 4dd550bd65..a752129c48 100644 --- a/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx +++ b/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx @@ -60,7 +60,10 @@ describe('', () => { const scrubArea = screen.getByTestId('scrub-area'); - await user.pointer({ target: scrubArea, keys: '[MouseLeft>]', pointerName: 'mouse' }); + await act(async () => { + await user.pointer({ target: scrubArea, keys: '[MouseLeft>]', pointerName: 'mouse' }); + await new Promise((resolve) => setTimeout(resolve, 25)); + }); expect(screen.queryByTestId('scrub-area-cursor')).not.to.equal(null); }); @@ -76,7 +79,10 @@ describe('', () => { const scrubArea = screen.getByRole('presentation'); - await user.pointer({ target: scrubArea, keys: '[TouchA>]', pointerName: 'touch' }); + await act(async () => { + await user.pointer({ target: scrubArea, keys: '[TouchA>]', pointerName: 'touch' }); + await new Promise((resolve) => setTimeout(resolve, 25)); + }); expect(screen.queryByTestId('scrub-area-cursor')).to.equal(null); }); From 0be33a5741afb1b1c58290bb4081652a2d2e93bb Mon Sep 17 00:00:00 2001 From: atomiks Date: Fri, 28 Feb 2025 14:14:25 +1100 Subject: [PATCH 6/7] Lint --- packages/react/src/number-field/root/useScrub.ts | 2 +- .../NumberFieldScrubAreaCursor.test.tsx | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/react/src/number-field/root/useScrub.ts b/packages/react/src/number-field/root/useScrub.ts index ea2b75fff2..b924f5a064 100644 --- a/packages/react/src/number-field/root/useScrub.ts +++ b/packages/react/src/number-field/root/useScrub.ts @@ -152,7 +152,7 @@ export function useScrub(params: ScrubParams) { // types in order for the `catch` to work. await ownerDocument(scrubAreaRef.current).body.requestPointerLock(); setIsPointerLockDenied(false); - } catch (e) { + } catch (error) { setIsPointerLockDenied(true); } }, 20); diff --git a/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx b/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx index a752129c48..1c54430c5e 100644 --- a/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx +++ b/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx @@ -62,7 +62,9 @@ describe('', () => { await act(async () => { await user.pointer({ target: scrubArea, keys: '[MouseLeft>]', pointerName: 'mouse' }); - await new Promise((resolve) => setTimeout(resolve, 25)); + await new Promise((resolve) => { + setTimeout(resolve, 25); + }); }); expect(screen.queryByTestId('scrub-area-cursor')).not.to.equal(null); @@ -81,7 +83,9 @@ describe('', () => { await act(async () => { await user.pointer({ target: scrubArea, keys: '[TouchA>]', pointerName: 'touch' }); - await new Promise((resolve) => setTimeout(resolve, 25)); + await new Promise((resolve) => { + setTimeout(resolve, 25); + }); }); expect(screen.queryByTestId('scrub-area-cursor')).to.equal(null); @@ -107,7 +111,9 @@ describe('', () => { await act(async () => { await user.pointer({ target: scrubArea, keys: '[MouseLeft>]', pointerName: 'mouse' }); - await new Promise((resolve) => setTimeout(resolve, 25)); + await new Promise((resolve) => { + setTimeout(resolve, 25); + }); }); expect(screen.queryByTestId('scrub-area-cursor')).to.equal(null); From cab2202e2f70b27bfea0c0fe1239f3d2ce6b9c58 Mon Sep 17 00:00:00 2001 From: atomiks Date: Fri, 28 Feb 2025 14:19:24 +1100 Subject: [PATCH 7/7] Mock --- .../NumberFieldScrubAreaCursor.test.tsx | 36 +++++++++++-------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx b/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx index 1c54430c5e..399f1ffaf3 100644 --- a/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx +++ b/packages/react/src/number-field/scrub-area-cursor/NumberFieldScrubAreaCursor.test.tsx @@ -50,24 +50,32 @@ describe('', () => { }); it('renders when using mouse input', async () => { - const { user } = await render( - - - - - , - ); + const originalRequestPointerLock = Element.prototype.requestPointerLock; - const scrubArea = screen.getByTestId('scrub-area'); + try { + Element.prototype.requestPointerLock = sinon.stub().resolves(); - await act(async () => { - await user.pointer({ target: scrubArea, keys: '[MouseLeft>]', pointerName: 'mouse' }); - await new Promise((resolve) => { - setTimeout(resolve, 25); + const { user } = await render( + + + + + , + ); + + const scrubArea = screen.getByTestId('scrub-area'); + + await act(async () => { + await user.pointer({ target: scrubArea, keys: '[MouseLeft>]', pointerName: 'mouse' }); + await new Promise((resolve) => { + setTimeout(resolve, 25); + }); }); - }); - expect(screen.queryByTestId('scrub-area-cursor')).not.to.equal(null); + expect(screen.queryByTestId('scrub-area-cursor')).not.to.equal(null); + } finally { + Element.prototype.requestPointerLock = originalRequestPointerLock; + } }); it('does not render when using touch input', async () => {