From d74ddc0d30d85147e20be5e522ceb5c8b94f5467 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 17 Mar 2026 14:07:54 -0500 Subject: [PATCH 01/13] docs: fix stale results in Icons search (#9802) --- packages/dev/s2-docs/src/MobileSearchMenu.tsx | 5 ++--- packages/dev/s2-docs/src/SearchMenu.tsx | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/dev/s2-docs/src/MobileSearchMenu.tsx b/packages/dev/s2-docs/src/MobileSearchMenu.tsx index 89d51b997c7..dd06ed12be6 100644 --- a/packages/dev/s2-docs/src/MobileSearchMenu.tsx +++ b/packages/dev/s2-docs/src/MobileSearchMenu.tsx @@ -315,13 +315,13 @@ function MobileNav({initialTag}: {initialTag?: string}) {
); } - diff --git a/packages/dev/s2-docs/src/SearchMenu.tsx b/packages/dev/s2-docs/src/SearchMenu.tsx index a15c544881a..9457c33d5a5 100644 --- a/packages/dev/s2-docs/src/SearchMenu.tsx +++ b/packages/dev/s2-docs/src/SearchMenu.tsx @@ -163,13 +163,13 @@ export function SearchMenu(props: SearchMenuProps) {
Date: Tue, 17 Mar 2026 14:12:56 -0500 Subject: [PATCH 02/13] fix: usePress and useMove global event attachment timing (#9749) * fix: 9744 compiler lint issue * missed eslint * fix lint * same operation applied to usePress --- .../@react-aria/interactions/src/useMove.ts | 139 +++---- .../@react-aria/interactions/src/usePress.ts | 354 +++++++++--------- 2 files changed, 230 insertions(+), 263 deletions(-) diff --git a/packages/@react-aria/interactions/src/useMove.ts b/packages/@react-aria/interactions/src/useMove.ts index c8158b4f9c3..310c93f223e 100644 --- a/packages/@react-aria/interactions/src/useMove.ts +++ b/packages/@react-aria/interactions/src/useMove.ts @@ -12,8 +12,8 @@ import {disableTextSelection, restoreTextSelection} from './textSelection'; import {DOMAttributes, MoveEvents, PointerType} from '@react-types/shared'; -import React, {useCallback, useMemo, useRef, useState} from 'react'; -import {useEffectEvent, useGlobalListeners, useLayoutEffect} from '@react-aria/utils'; +import React, {useCallback, useMemo, useRef} from 'react'; +import {useEffectEvent, useGlobalListeners} from '@react-aria/utils'; export interface MoveResult { /** Props to spread on the target element. */ @@ -87,66 +87,49 @@ export function useMove(props: MoveEvents): MoveResult { }, [onMoveEnd, state]); let endEvent = useEffectEvent(end); - let [pointerDown, setPointerDown] = useState<'pointer' | 'mouse' | 'touch' | null>(null); - useLayoutEffect(() => { - if (pointerDown === 'pointer') { - let onPointerMove = (e: PointerEvent) => { - if (e.pointerId === state.current.id) { - let pointerType = (e.pointerType || 'mouse') as PointerType; + let moveProps = useMemo(() => { + let moveProps: DOMAttributes = {}; - // Problems with PointerEvent#movementX/movementY: - // 1. it is always 0 on macOS Safari. - // 2. On Chrome Android, it's scaled by devicePixelRatio, but not on Chrome macOS - moveEvent(e, pointerType, e.pageX - (state.current.lastPosition?.pageX ?? 0), e.pageY - (state.current.lastPosition?.pageY ?? 0)); - state.current.lastPosition = {pageX: e.pageX, pageY: e.pageY}; - } - }; + let start = () => { + disableTextSelection(); + state.current.didMove = false; + }; - let onPointerUp = (e: PointerEvent) => { - if (e.pointerId === state.current.id) { - let pointerType = (e.pointerType || 'mouse') as PointerType; - endEvent(e, pointerType); - state.current.id = null; - removeGlobalListener(window, 'pointermove', onPointerMove, false); - removeGlobalListener(window, 'pointerup', onPointerUp, false); - removeGlobalListener(window, 'pointercancel', onPointerUp, false); - setPointerDown(null); - } - }; - addGlobalListener(window, 'pointermove', onPointerMove, false); - addGlobalListener(window, 'pointerup', onPointerUp, false); - addGlobalListener(window, 'pointercancel', onPointerUp, false); - return () => { - removeGlobalListener(window, 'pointermove', onPointerMove, false); - removeGlobalListener(window, 'pointerup', onPointerUp, false); - removeGlobalListener(window, 'pointercancel', onPointerUp, false); - }; - } else if (pointerDown === 'mouse' && process.env.NODE_ENV === 'test') { + if (typeof PointerEvent === 'undefined' && process.env.NODE_ENV === 'test') { let onMouseMove = (e: MouseEvent) => { if (e.button === 0) { + // Should be safe to use the useEffectEvent because these are equivalent https://github.com/reactjs/react.dev/issues/8075#issuecomment-3400179389 + // However, the compiler is not smart enough to know that. As such, this whole file must be manually optimised as the compiler will bail. + // + // eslint-disable-next-line react-hooks/rules-of-hooks moveEvent(e, 'mouse', e.pageX - (state.current.lastPosition?.pageX ?? 0), e.pageY - (state.current.lastPosition?.pageY ?? 0)); state.current.lastPosition = {pageX: e.pageX, pageY: e.pageY}; } }; let onMouseUp = (e: MouseEvent) => { if (e.button === 0) { + // eslint-disable-next-line react-hooks/rules-of-hooks endEvent(e, 'mouse'); removeGlobalListener(window, 'mousemove', onMouseMove, false); removeGlobalListener(window, 'mouseup', onMouseUp, false); - setPointerDown(null); } }; - addGlobalListener(window, 'mousemove', onMouseMove, false); - addGlobalListener(window, 'mouseup', onMouseUp, false); - return () => { - removeGlobalListener(window, 'mousemove', onMouseMove, false); - removeGlobalListener(window, 'mouseup', onMouseUp, false); + moveProps.onMouseDown = (e: React.MouseEvent) => { + if (e.button === 0) { + start(); + e.stopPropagation(); + e.preventDefault(); + state.current.lastPosition = {pageX: e.pageX, pageY: e.pageY}; + addGlobalListener(window, 'mousemove', onMouseMove, false); + addGlobalListener(window, 'mouseup', onMouseUp, false); + } }; - } else if (pointerDown === 'touch' && process.env.NODE_ENV === 'test') { + let onTouchMove = (e: TouchEvent) => { let touch = [...e.changedTouches].findIndex(({identifier}) => identifier === state.current.id); if (touch >= 0) { let {pageX, pageY} = e.changedTouches[touch]; + // eslint-disable-next-line react-hooks/rules-of-hooks moveEvent(e, 'touch', pageX - (state.current.lastPosition?.pageX ?? 0), pageY - (state.current.lastPosition?.pageY ?? 0)); state.current.lastPosition = {pageX, pageY}; } @@ -154,41 +137,12 @@ export function useMove(props: MoveEvents): MoveResult { let onTouchEnd = (e: TouchEvent) => { let touch = [...e.changedTouches].findIndex(({identifier}) => identifier === state.current.id); if (touch >= 0) { + // eslint-disable-next-line react-hooks/rules-of-hooks endEvent(e, 'touch'); state.current.id = null; removeGlobalListener(window, 'touchmove', onTouchMove); removeGlobalListener(window, 'touchend', onTouchEnd); removeGlobalListener(window, 'touchcancel', onTouchEnd); - setPointerDown(null); - } - }; - addGlobalListener(window, 'touchmove', onTouchMove, false); - addGlobalListener(window, 'touchend', onTouchEnd, false); - addGlobalListener(window, 'touchcancel', onTouchEnd, false); - return () => { - removeGlobalListener(window, 'touchmove', onTouchMove, false); - removeGlobalListener(window, 'touchend', onTouchEnd, false); - removeGlobalListener(window, 'touchcancel', onTouchEnd, false); - }; - } - }, [pointerDown, addGlobalListener, removeGlobalListener]); - - let moveProps = useMemo(() => { - let moveProps: DOMAttributes = {}; - - let start = () => { - disableTextSelection(); - state.current.didMove = false; - }; - - if (typeof PointerEvent === 'undefined' && process.env.NODE_ENV === 'test') { - moveProps.onMouseDown = (e: React.MouseEvent) => { - if (e.button === 0) { - start(); - e.stopPropagation(); - e.preventDefault(); - state.current.lastPosition = {pageX: e.pageX, pageY: e.pageY}; - setPointerDown('mouse'); } }; moveProps.onTouchStart = (e: React.TouchEvent) => { @@ -202,9 +156,36 @@ export function useMove(props: MoveEvents): MoveResult { e.preventDefault(); state.current.lastPosition = {pageX, pageY}; state.current.id = identifier; - setPointerDown('touch'); + addGlobalListener(window, 'touchmove', onTouchMove, false); + addGlobalListener(window, 'touchend', onTouchEnd, false); + addGlobalListener(window, 'touchcancel', onTouchEnd, false); }; } else { + let onPointerMove = (e: PointerEvent) => { + if (e.pointerId === state.current.id) { + let pointerType = (e.pointerType || 'mouse') as PointerType; + + // Problems with PointerEvent#movementX/movementY: + // 1. it is always 0 on macOS Safari. + // 2. On Chrome Android, it's scaled by devicePixelRatio, but not on Chrome macOS + // eslint-disable-next-line react-hooks/rules-of-hooks + moveEvent(e, pointerType, e.pageX - (state.current.lastPosition?.pageX ?? 0), e.pageY - (state.current.lastPosition?.pageY ?? 0)); + state.current.lastPosition = {pageX: e.pageX, pageY: e.pageY}; + } + }; + + let onPointerUp = (e: PointerEvent) => { + if (e.pointerId === state.current.id) { + let pointerType = (e.pointerType || 'mouse') as PointerType; + // eslint-disable-next-line react-hooks/rules-of-hooks + endEvent(e, pointerType); + state.current.id = null; + removeGlobalListener(window, 'pointermove', onPointerMove, false); + removeGlobalListener(window, 'pointerup', onPointerUp, false); + removeGlobalListener(window, 'pointercancel', onPointerUp, false); + } + }; + moveProps.onPointerDown = (e: React.PointerEvent) => { if (e.button === 0 && state.current.id == null) { start(); @@ -212,15 +193,19 @@ export function useMove(props: MoveEvents): MoveResult { e.preventDefault(); state.current.lastPosition = {pageX: e.pageX, pageY: e.pageY}; state.current.id = e.pointerId; - setPointerDown('pointer'); + addGlobalListener(window, 'pointermove', onPointerMove, false); + addGlobalListener(window, 'pointerup', onPointerUp, false); + addGlobalListener(window, 'pointercancel', onPointerUp, false); } }; } let triggerKeyboardMove = (e: EventBase, deltaX: number, deltaY: number) => { start(); - move(e, 'keyboard', deltaX, deltaY); - end(e, 'keyboard'); + // eslint-disable-next-line react-hooks/rules-of-hooks + moveEvent(e, 'keyboard', deltaX, deltaY); + // eslint-disable-next-line react-hooks/rules-of-hooks + endEvent(e, 'keyboard'); }; moveProps.onKeyDown = (e) => { @@ -253,7 +238,7 @@ export function useMove(props: MoveEvents): MoveResult { }; return moveProps; - }, [state, move, end]); + }, [addGlobalListener, removeGlobalListener, state]); return {moveProps}; } diff --git a/packages/@react-aria/interactions/src/usePress.ts b/packages/@react-aria/interactions/src/usePress.ts index 46630ea9389..b24a383df12 100644 --- a/packages/@react-aria/interactions/src/usePress.ts +++ b/packages/@react-aria/interactions/src/usePress.ts @@ -30,7 +30,6 @@ import { openLink, useEffectEvent, useGlobalListeners, - useLayoutEffect, useSyncRef } from '@react-aria/utils'; import {createSyntheticEvent, preventFocus, setEventTarget} from './utils'; @@ -202,7 +201,7 @@ export function usePress(props: PressHookProps): PressResult { disposables: [] }); - let {addGlobalListener, removeAllGlobalListeners, removeGlobalListener} = useGlobalListeners(); + let {addGlobalListener, removeAllGlobalListeners} = useGlobalListeners(); let triggerPressStart = useCallback((originalEvent: EventBase, pointerType: PointerType) => { let state = ref.current; @@ -286,7 +285,6 @@ export function usePress(props: PressHookProps): PressResult { triggerPressEnd(createEvent(state.target, e), state.pointerType, false); } state.isPressed = false; - setIsPointerPressed(null); state.isOverTarget = false; state.activePointerId = null; state.pointerType = null; @@ -332,165 +330,6 @@ export function usePress(props: PressHookProps): PressResult { onClick(createSyntheticEvent(event)); } }, [isDisabled, onClick]); - let triggerSyntheticClickEvent = useEffectEvent(triggerSyntheticClick); - - let [isElemKeyPressed, setIsElemKeyPressed] = useState(false); - useLayoutEffect(() => { - let state = ref.current; - if (isElemKeyPressed) { - let onKeyUp = (e: KeyboardEvent) => { - if (state.isPressed && state.target && isValidKeyboardEvent(e, state.target)) { - if (shouldPreventDefaultKeyboard(getEventTarget(e) as Element, e.key)) { - e.preventDefault(); - } - - let target = getEventTarget(e) as Element; - let wasPressed = nodeContains(state.target, target); - triggerPressEndEvent(createEvent(state.target, e), 'keyboard', wasPressed); - if (wasPressed) { - triggerSyntheticClickEvent(e, state.target); - } - removeAllGlobalListeners(); - - // If a link was triggered with a key other than Enter, open the URL ourselves. - // This means the link has a role override, and the default browser behavior - // only applies when using the Enter key. - if (e.key !== 'Enter' && isHTMLAnchorLink(state.target) && nodeContains(state.target, target) && !e[LINK_CLICKED]) { - // Store a hidden property on the event so we only trigger link click once, - // even if there are multiple usePress instances attached to the element. - e[LINK_CLICKED] = true; - openLink(state.target, e, false); - } - - state.isPressed = false; - setIsElemKeyPressed(false); - state.metaKeyEvents?.delete(e.key); - } else if (e.key === 'Meta' && state.metaKeyEvents?.size) { - // If we recorded keydown events that occurred while the Meta key was pressed, - // and those haven't received keyup events already, fire keyup events ourselves. - // See comment above for more info about the macOS bug causing this. - let events = state.metaKeyEvents; - state.metaKeyEvents = undefined; - for (let event of events.values()) { - state.target?.dispatchEvent(new KeyboardEvent('keyup', event)); - } - } - }; - // Focus may move before the key up event, so register the event on the document - // instead of the same element where the key down event occurred. Make it capturing so that it will trigger - // before stopPropagation from useKeyboard on a child element may happen and thus we can still call triggerPress for the parent element. - let originalTarget = state.target; - let pressUp = (e: KeyboardEvent) => { - if (originalTarget && isValidKeyboardEvent(e, originalTarget) && !e.repeat && nodeContains(originalTarget, getEventTarget(e) as Element) && state.target) { - triggerPressUpEvent(createEvent(state.target, e), 'keyboard'); - } - }; - let listener = chain(pressUp, onKeyUp); - addGlobalListener(getOwnerDocument(state.target), 'keyup', listener, true); - return () => { - removeGlobalListener(getOwnerDocument(state.target), 'keyup', listener, true); - }; - } - }, [isElemKeyPressed, addGlobalListener, removeAllGlobalListeners, removeGlobalListener]); - - let [isPointerPressed, setIsPointerPressed] = useState<'pointer' | 'mouse' | 'touch' | null>(null); - useLayoutEffect(() => { - let state = ref.current; - if (isPointerPressed === 'pointer') { - let onPointerUp = (e: PointerEvent) => { - if (e.pointerId === state.activePointerId && state.isPressed && e.button === 0 && state.target) { - if (nodeContains(state.target, getEventTarget(e) as Element) && state.pointerType != null) { - // Wait for onClick to fire onPress. This avoids browser issues when the DOM - // is mutated between onPointerUp and onClick, and is more compatible with third party libraries. - // https://github.com/adobe/react-spectrum/issues/1513 - // https://issues.chromium.org/issues/40732224 - // However, iOS and Android do not focus or fire onClick after a long press. - // We work around this by triggering a click ourselves after a timeout. - // This timeout is canceled during the click event in case the real one fires first. - // The timeout must be at least 32ms, because Safari on iOS delays the click event on - // non-form elements without certain ARIA roles (for hover emulation). - // https://github.com/WebKit/WebKit/blob/dccfae42bb29bd4bdef052e469f604a9387241c0/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm#L875-L892 - let clicked = false; - let timeout = setTimeout(() => { - if (state.isPressed && state.target instanceof HTMLElement) { - if (clicked) { - cancelEvent(e); - } else { - focusWithoutScrolling(state.target); - state.target.click(); - } - } - }, 80); - // Use a capturing listener to track if a click occurred. - // If stopPropagation is called it may never reach our handler. - if (e.currentTarget) { - addGlobalListener(e.currentTarget, 'click', () => clicked = true, true); - } - state.disposables.push(() => clearTimeout(timeout)); - } else { - cancelEvent(e); - } - - // Ignore subsequent onPointerLeave event before onClick on touch devices. - state.isOverTarget = false; - } - }; - - let onPointerCancel = (e: PointerEvent) => { - cancelEvent(e); - }; - - addGlobalListener(getOwnerDocument(state.target), 'pointerup', onPointerUp, false); - addGlobalListener(getOwnerDocument(state.target), 'pointercancel', onPointerCancel, false); - return () => { - removeGlobalListener(getOwnerDocument(state.target), 'pointerup', onPointerUp, false); - removeGlobalListener(getOwnerDocument(state.target), 'pointercancel', onPointerCancel, false); - }; - } else if (isPointerPressed === 'mouse' && process.env.NODE_ENV === 'test') { - let onMouseUp = (e: MouseEvent) => { - // Only handle left clicks - if (e.button !== 0) { - return; - } - - if (state.ignoreEmulatedMouseEvents) { - state.ignoreEmulatedMouseEvents = false; - return; - } - - if (state.target && nodeContains(state.target, e.target as Element) && state.pointerType != null) { - // Wait for onClick to fire onPress. This avoids browser issues when the DOM - // is mutated between onMouseUp and onClick, and is more compatible with third party libraries. - } else { - cancelEvent(e); - } - - state.isOverTarget = false; - }; - - addGlobalListener(getOwnerDocument(state.target), 'mouseup', onMouseUp, false); - return () => { - removeGlobalListener(getOwnerDocument(state.target), 'mouseup', onMouseUp, false); - }; - } else if (isPointerPressed === 'touch' && process.env.NODE_ENV === 'test') { - let onScroll = (e: Event) => { - if (state.isPressed && nodeContains(getEventTarget(e) as Element, state.target)) { - cancelEvent({ - currentTarget: state.target, - shiftKey: false, - ctrlKey: false, - metaKey: false, - altKey: false - }); - } - }; - - addGlobalListener(getOwnerWindow(state.target), 'scroll', onScroll, true); - return () => { - removeGlobalListener(getOwnerWindow(state.target), 'scroll', onScroll, true); - }; - } - }, [isPointerPressed, addGlobalListener, removeGlobalListener]); let pressProps = useMemo(() => { let state = ref.current; @@ -508,11 +347,23 @@ export function usePress(props: PressHookProps): PressResult { if (!state.isPressed && !e.repeat) { state.target = e.currentTarget; state.isPressed = true; - setIsElemKeyPressed(true); state.pointerType = 'keyboard'; shouldStopPropagation = triggerPressStart(e, 'keyboard'); } + // Focus may move before the key up event, so register the event on the document + // instead of the same element where the key down event occurred. Make it capturing so that it will trigger + // before stopPropagation from useKeyboard on a child element may happen and thus we can still call triggerPress for the parent element. + let originalTarget = e.currentTarget; + let pressUp = (e) => { + if (isValidKeyboardEvent(e, originalTarget) && !e.repeat && nodeContains(originalTarget, getEventTarget(e) as Element) && state.target) { + // eslint-disable-next-line react-hooks/rules-of-hooks + triggerPressUpEvent(createEvent(state.target, e), 'keyboard'); + } + }; + + addGlobalListener(getOwnerDocument(e.currentTarget), 'keyup', chain(pressUp, onKeyUp), true); + if (shouldStopPropagation) { e.stopPropagation(); } @@ -546,18 +397,23 @@ export function usePress(props: PressHookProps): PressResult { // trigger as if it were a keyboard click. if (!state.ignoreEmulatedMouseEvents && !state.isPressed && (state.pointerType === 'virtual' || isVirtualClick(e.nativeEvent))) { let stopPressStart = triggerPressStart(e, 'virtual'); - let stopPressUp = triggerPressUp(e, 'virtual'); - let stopPressEnd = triggerPressEnd(e, 'virtual'); + // eslint-disable-next-line react-hooks/rules-of-hooks + let stopPressUp = triggerPressUpEvent(e, 'virtual'); + // eslint-disable-next-line react-hooks/rules-of-hooks + let stopPressEnd = triggerPressEndEvent(e, 'virtual'); triggerClick(e); shouldStopPropagation = stopPressStart && stopPressUp && stopPressEnd; } else if (state.isPressed && state.pointerType !== 'keyboard') { let pointerType = state.pointerType || (e.nativeEvent as PointerEvent).pointerType as PointerType || 'virtual'; - let stopPressUp = triggerPressUp(createEvent(e.currentTarget, e), pointerType); - let stopPressEnd = triggerPressEnd(createEvent(e.currentTarget, e), pointerType, true); + // eslint-disable-next-line react-hooks/rules-of-hooks + let stopPressUp = triggerPressUpEvent(createEvent(e.currentTarget, e), pointerType); + // eslint-disable-next-line react-hooks/rules-of-hooks + let stopPressEnd = triggerPressEndEvent(createEvent(e.currentTarget, e), pointerType, true); shouldStopPropagation = stopPressUp && stopPressEnd; state.isOverTarget = false; triggerClick(e); - cancel(e); + // eslint-disable-next-line react-hooks/rules-of-hooks + cancelEvent(e); } state.ignoreEmulatedMouseEvents = false; @@ -568,6 +424,45 @@ export function usePress(props: PressHookProps): PressResult { } }; + let onKeyUp = (e: KeyboardEvent) => { + if (state.isPressed && state.target && isValidKeyboardEvent(e, state.target)) { + if (shouldPreventDefaultKeyboard(getEventTarget(e) as Element, e.key)) { + e.preventDefault(); + } + + let target = getEventTarget(e); + let wasPressed = nodeContains(state.target, target as Element); + // eslint-disable-next-line react-hooks/rules-of-hooks + triggerPressEndEvent(createEvent(state.target, e), 'keyboard', wasPressed); + if (wasPressed) { + triggerSyntheticClick(e, state.target); + } + removeAllGlobalListeners(); + + // If a link was triggered with a key other than Enter, open the URL ourselves. + // This means the link has a role override, and the default browser behavior + // only applies when using the Enter key. + if (e.key !== 'Enter' && isHTMLAnchorLink(state.target) && nodeContains(state.target, target as Element) && !e[LINK_CLICKED]) { + // Store a hidden property on the event so we only trigger link click once, + // even if there are multiple usePress instances attached to the element. + e[LINK_CLICKED] = true; + openLink(state.target, e, false); + } + + state.isPressed = false; + state.metaKeyEvents?.delete(e.key); + } else if (e.key === 'Meta' && state.metaKeyEvents?.size) { + // If we recorded keydown events that occurred while the Meta key was pressed, + // and those haven't received keyup events already, fire keyup events ourselves. + // See comment above for more info about the macOS bug causing this. + let events = state.metaKeyEvents; + state.metaKeyEvents = undefined; + for (let event of events.values()) { + state.target?.dispatchEvent(new KeyboardEvent('keyup', event)); + } + } + }; + if (typeof PointerEvent !== 'undefined') { pressProps.onPointerDown = (e) => { // Only handle left clicks, and ignore events that bubbled through portals. @@ -589,7 +484,6 @@ export function usePress(props: PressHookProps): PressResult { let shouldStopPropagation = true; if (!state.isPressed) { state.isPressed = true; - setIsPointerPressed('pointer'); state.isOverTarget = true; state.activePointerId = e.pointerId; state.target = e.currentTarget as FocusableElement; @@ -612,6 +506,8 @@ export function usePress(props: PressHookProps): PressResult { (target as Element).releasePointerCapture(e.pointerId); } } + addGlobalListener(getOwnerDocument(e.currentTarget), 'pointerup', onPointerUp, false); + addGlobalListener(getOwnerDocument(e.currentTarget), 'pointercancel', onPointerCancel, false); } if (shouldStopPropagation) { @@ -644,7 +540,8 @@ export function usePress(props: PressHookProps): PressResult { // Only handle left clicks. If isPressed is true, delay until onClick. if (e.button === 0 && !state.isPressed) { - triggerPressUp(e, state.pointerType || e.pointerType); + // eslint-disable-next-line react-hooks/rules-of-hooks + triggerPressUpEvent(e, state.pointerType || e.pointerType); } }; @@ -658,11 +555,55 @@ export function usePress(props: PressHookProps): PressResult { pressProps.onPointerLeave = (e) => { if (e.pointerId === state.activePointerId && state.target && state.isOverTarget && state.pointerType != null) { state.isOverTarget = false; - triggerPressEnd(createEvent(state.target, e), state.pointerType, false); + // eslint-disable-next-line react-hooks/rules-of-hooks + triggerPressEndEvent(createEvent(state.target, e), state.pointerType, false); cancelOnPointerExit(e); } }; + let onPointerUp = (e: PointerEvent) => { + if (e.pointerId === state.activePointerId && state.isPressed && e.button === 0 && state.target) { + if (nodeContains(state.target, getEventTarget(e) as Element) && state.pointerType != null) { + // Wait for onClick to fire onPress. This avoids browser issues when the DOM + // is mutated between onPointerUp and onClick, and is more compatible with third party libraries. + // https://github.com/adobe/react-spectrum/issues/1513 + // https://issues.chromium.org/issues/40732224 + // However, iOS and Android do not focus or fire onClick after a long press. + // We work around this by triggering a click ourselves after a timeout. + // This timeout is canceled during the click event in case the real one fires first. + // The timeout must be at least 32ms, because Safari on iOS delays the click event on + // non-form elements without certain ARIA roles (for hover emulation). + // https://github.com/WebKit/WebKit/blob/dccfae42bb29bd4bdef052e469f604a9387241c0/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm#L875-L892 + let clicked = false; + let timeout = setTimeout(() => { + if (state.isPressed && state.target instanceof HTMLElement) { + if (clicked) { + // eslint-disable-next-line react-hooks/rules-of-hooks + cancelEvent(e); + } else { + focusWithoutScrolling(state.target); + state.target.click(); + } + } + }, 80); + // Use a capturing listener to track if a click occurred. + // If stopPropagation is called it may never reach our handler. + addGlobalListener(e.currentTarget as Document, 'click', () => clicked = true, true); + state.disposables.push(() => clearTimeout(timeout)); + } else { + // eslint-disable-next-line react-hooks/rules-of-hooks + cancelEvent(e); + } + + // Ignore subsequent onPointerLeave event before onClick on touch devices. + state.isOverTarget = false; + } + }; + + let onPointerCancel = (e: PointerEvent) => { + // eslint-disable-next-line react-hooks/rules-of-hooks + cancelEvent(e); + }; pressProps.onDragStart = (e) => { if (!nodeContains(e.currentTarget, getEventTarget(e))) { @@ -670,7 +611,8 @@ export function usePress(props: PressHookProps): PressResult { } // Safari does not call onPointerCancel when a drag starts, whereas Chrome and Firefox do. - cancel(e); + // eslint-disable-next-line react-hooks/rules-of-hooks + cancelEvent(e); }; } else if (process.env.NODE_ENV === 'test') { // NOTE: this fallback branch is entirely used by unit tests. @@ -688,7 +630,6 @@ export function usePress(props: PressHookProps): PressResult { } state.isPressed = true; - setIsPointerPressed('mouse'); state.isOverTarget = true; state.target = e.currentTarget; state.pointerType = isVirtualClick(e.nativeEvent) ? 'virtual' : 'mouse'; @@ -705,6 +646,7 @@ export function usePress(props: PressHookProps): PressResult { state.disposables.push(dispose); } } + addGlobalListener(getOwnerDocument(e.currentTarget), 'mouseup', onMouseUp, false); }; pressProps.onMouseEnter = (e) => { @@ -731,7 +673,8 @@ export function usePress(props: PressHookProps): PressResult { let shouldStopPropagation = true; if (state.isPressed && !state.ignoreEmulatedMouseEvents && state.pointerType != null) { state.isOverTarget = false; - shouldStopPropagation = triggerPressEnd(e, state.pointerType, false); + // eslint-disable-next-line react-hooks/rules-of-hooks + shouldStopPropagation = triggerPressEndEvent(e, state.pointerType, false); cancelOnPointerExit(e); } @@ -746,8 +689,31 @@ export function usePress(props: PressHookProps): PressResult { } if (!state.ignoreEmulatedMouseEvents && e.button === 0 && !state.isPressed) { - triggerPressUp(e, state.pointerType || 'mouse'); + // eslint-disable-next-line react-hooks/rules-of-hooks + triggerPressUpEvent(e, state.pointerType || 'mouse'); + } + }; + + let onMouseUp = (e: MouseEvent) => { + // Only handle left clicks + if (e.button !== 0) { + return; + } + + if (state.ignoreEmulatedMouseEvents) { + state.ignoreEmulatedMouseEvents = false; + return; + } + + if (state.target && nodeContains(state.target, getEventTarget(e) as Element) && state.pointerType != null) { + // Wait for onClick to fire onPress. This avoids browser issues when the DOM + // is mutated between onMouseUp and onClick, and is more compatible with third party libraries. + } else { + // eslint-disable-next-line react-hooks/rules-of-hooks + cancelEvent(e); } + + state.isOverTarget = false; }; pressProps.onTouchStart = (e) => { @@ -763,7 +729,6 @@ export function usePress(props: PressHookProps): PressResult { state.ignoreEmulatedMouseEvents = true; state.isOverTarget = true; state.isPressed = true; - setIsPointerPressed('touch'); state.target = e.currentTarget; state.pointerType = 'touch'; @@ -775,6 +740,7 @@ export function usePress(props: PressHookProps): PressResult { if (shouldStopPropagation) { e.stopPropagation(); } + addGlobalListener(getOwnerWindow(e.currentTarget), 'scroll', onScroll, true); }; pressProps.onTouchMove = (e) => { @@ -796,7 +762,8 @@ export function usePress(props: PressHookProps): PressResult { } } else if (state.isOverTarget && state.pointerType != null) { state.isOverTarget = false; - shouldStopPropagation = triggerPressEnd(createTouchEvent(state.target!, e), state.pointerType, false); + // eslint-disable-next-line react-hooks/rules-of-hooks + shouldStopPropagation = triggerPressEndEvent(createTouchEvent(state.target!, e), state.pointerType, false); cancelOnPointerExit(createTouchEvent(state.target!, e)); } @@ -818,11 +785,14 @@ export function usePress(props: PressHookProps): PressResult { let touch = getTouchById(e.nativeEvent, state.activePointerId); let shouldStopPropagation = true; if (touch && isOverTarget(touch, e.currentTarget) && state.pointerType != null) { - triggerPressUp(createTouchEvent(state.target!, e), state.pointerType); - shouldStopPropagation = triggerPressEnd(createTouchEvent(state.target!, e), state.pointerType); + // eslint-disable-next-line react-hooks/rules-of-hooks + triggerPressUpEvent(createTouchEvent(state.target!, e), state.pointerType); + // eslint-disable-next-line react-hooks/rules-of-hooks + shouldStopPropagation = triggerPressEndEvent(createTouchEvent(state.target!, e), state.pointerType); triggerSyntheticClick(e.nativeEvent, state.target!); } else if (state.isOverTarget && state.pointerType != null) { - shouldStopPropagation = triggerPressEnd(createTouchEvent(state.target!, e), state.pointerType, false); + // eslint-disable-next-line react-hooks/rules-of-hooks + shouldStopPropagation = triggerPressEndEvent(createTouchEvent(state.target!, e), state.pointerType, false); } if (shouldStopPropagation) { @@ -830,7 +800,6 @@ export function usePress(props: PressHookProps): PressResult { } state.isPressed = false; - setIsPointerPressed(null); state.activePointerId = null; state.isOverTarget = false; state.ignoreEmulatedMouseEvents = true; @@ -847,7 +816,21 @@ export function usePress(props: PressHookProps): PressResult { e.stopPropagation(); if (state.isPressed) { - cancel(createTouchEvent(state.target!, e)); + // eslint-disable-next-line react-hooks/rules-of-hooks + cancelEvent(createTouchEvent(state.target!, e)); + } + }; + + let onScroll = (e: Event) => { + if (state.isPressed && nodeContains(getEventTarget(e) as Element, state.target)) { + // eslint-disable-next-line react-hooks/rules-of-hooks + cancelEvent({ + currentTarget: state.target, + shiftKey: false, + ctrlKey: false, + metaKey: false, + altKey: false + }); } }; @@ -856,21 +839,20 @@ export function usePress(props: PressHookProps): PressResult { return; } - cancel(e); + // eslint-disable-next-line react-hooks/rules-of-hooks + cancelEvent(e); }; } return pressProps; }, [ + addGlobalListener, isDisabled, preventFocusOnPress, removeAllGlobalListeners, allowTextSelectionOnPress, - cancel, cancelOnPointerExit, - triggerPressEnd, triggerPressStart, - triggerPressUp, triggerClick, triggerSyntheticClick ]); From 07214be1cd37d789df4ece7675c67f26bb6cc08c Mon Sep 17 00:00:00 2001 From: "nanto_vi, TOYAMA Nao" Date: Wed, 18 Mar 2026 04:15:14 +0900 Subject: [PATCH 03/13] fix: Don't reset form fields if reset event is cancelled. (#7603) * fix: Don't reset form fields if reset event is cancelled. * tests, handle bubble preventdefault and stoppropagation * fix: Allow multiple form elements with useFormReset * simplify case again --------- Co-authored-by: Robert Snow Co-authored-by: Robert Snow --- .../@react-aria/utils/src/useFormReset.ts | 5 +- .../utils/test/useFormReset.test.tsx | 137 ++++++++++++++++++ 2 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 packages/@react-aria/utils/test/useFormReset.test.tsx diff --git a/packages/@react-aria/utils/src/useFormReset.ts b/packages/@react-aria/utils/src/useFormReset.ts index c37eb67cc50..5051312974a 100644 --- a/packages/@react-aria/utils/src/useFormReset.ts +++ b/packages/@react-aria/utils/src/useFormReset.ts @@ -19,8 +19,9 @@ export function useFormReset( initialValue: T, onReset: (value: T) => void ): void { - let handleReset = useEffectEvent(() => { - if (onReset) { + + let handleReset = useEffectEvent((e: Event) => { + if (onReset && !e.defaultPrevented) { onReset(initialValue); } }); diff --git a/packages/@react-aria/utils/test/useFormReset.test.tsx b/packages/@react-aria/utils/test/useFormReset.test.tsx new file mode 100644 index 00000000000..b0be933a3f3 --- /dev/null +++ b/packages/@react-aria/utils/test/useFormReset.test.tsx @@ -0,0 +1,137 @@ +/* + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {fireEvent, render} from '@react-spectrum/test-utils-internal'; +import React, {useRef} from 'react'; +import {useFormReset} from '../'; + +describe('useFormReset', () => { + it('should call onReset on reset', () => { + const onReset = jest.fn(); + const Form = () => { + const ref = useRef(null); + useFormReset(ref, '', onReset); + return ( +
+ + +
+ ); + }; + const {getByRole} = render(
); + const button = getByRole('button'); + fireEvent.click(button); + expect(onReset).toHaveBeenCalled(); + }); + + it('should call onReset on reset even if event is stopped', () => { + const onReset = jest.fn(); + const Form = () => { + const ref = useRef(null); + useFormReset(ref, '', onReset); + return ( + e.stopPropagation()}> + + + + ); + }; + const {getByRole} = render(
); + const button = getByRole('button'); + fireEvent.click(button); + expect(onReset).toHaveBeenCalled(); + }); + + it('should call every onReset on reset', () => { + const onReset1 = jest.fn(); + const onReset2 = jest.fn(); + const Form = () => { + const ref1 = useRef(null); + useFormReset(ref1, '', onReset1); + const ref2 = useRef(null); + useFormReset(ref2, '', onReset2); + return ( + + + + + + ); + }; + const {getByRole} = render(
); + const button = getByRole('button'); + fireEvent.click(button); + expect(onReset1).toHaveBeenCalled(); + expect(onReset2).toHaveBeenCalled(); + }); + + it.skip('should not call onReset if reset is cancelled', async () => { + // Simpler case at the moment, but you have to setup a capture listener to prevent the default behavior. + // Matching native behavior is too much of a change until someone asks for it. + const onReset = jest.fn(); + const Form = () => { + const ref = useRef(null); + useFormReset(ref, '', onReset); + return ( + e.preventDefault()}> + + + + ); + }; + const {getByRole} = render(
); + const button = getByRole('button'); + fireEvent.click(button); + expect(onReset).not.toHaveBeenCalled(); + }); + + it('should not call onReset if reset is cancelled in capture phase', async () => { + const onReset = jest.fn(); + const Form = () => { + const ref = useRef(null); + useFormReset(ref, '', onReset); + return ( + e.preventDefault()}> + + + + ); + }; + const {getByRole} = render(
); + const button = getByRole('button'); + fireEvent.click(button); + expect(onReset).not.toHaveBeenCalled(); + }); + + it('should not call any onReset if reset is cancelled', () => { + const onReset1 = jest.fn(); + const onReset2 = jest.fn(); + const Form = () => { + const ref1 = useRef(null); + useFormReset(ref1, '', onReset1); + const ref2 = useRef(null); + useFormReset(ref2, '', onReset2); + return ( + e.preventDefault()}> + + + + + ); + }; + const {getByRole} = render(
); + const button = getByRole('button'); + fireEvent.click(button); + expect(onReset1).not.toHaveBeenCalled(); + expect(onReset2).not.toHaveBeenCalled(); + }); +}); From 4e6456bd7e749ab9f75f3f327e0f9b112b006104 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 17 Mar 2026 14:36:47 -0500 Subject: [PATCH 04/13] chore: calendar new today indicator (#9591) * chore: calendar new today indicator * fix lint * simplify css and put selection hover between background and border * fix unavailable dates * fix to match expectations in chromatic * adjust padding * update remaining tokens and setup error message * Apply suggestions from code review Co-authored-by: Robert Snow --- packages/@react-spectrum/s2/src/Calendar.tsx | 159 +++++++++++++----- .../@react-spectrum/s2/src/DatePicker.tsx | 11 +- .../s2/src/DateRangePicker.tsx | 11 +- 3 files changed, 130 insertions(+), 51 deletions(-) diff --git a/packages/@react-spectrum/s2/src/Calendar.tsx b/packages/@react-spectrum/s2/src/Calendar.tsx index 57736226afd..7da325534d5 100644 --- a/packages/@react-spectrum/s2/src/Calendar.tsx +++ b/packages/@react-spectrum/s2/src/Calendar.tsx @@ -36,7 +36,6 @@ import { useSlottedContext } from 'react-aria-components'; import {AriaCalendarGridProps} from '@react-aria/calendar'; -import {baseColor, focusRing, lightDark, style} from '../style' with {type: 'macro'}; import { CalendarDate, getDayOfWeek, @@ -44,12 +43,13 @@ import { } from '@internationalized/date'; import ChevronLeftIcon from '../s2wf-icons/S2_Icon_ChevronLeft_20_N.svg'; import ChevronRightIcon from '../s2wf-icons/S2_Icon_ChevronRight_20_N.svg'; +import {focusRing, lightDark, style} from '../style' with {type: 'macro'}; import {forwardRefType, GlobalDOMAttributes} from '@react-types/shared'; import {getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; import {helpTextStyles} from './Field'; // @ts-ignore import intlMessages from '../intl/*.json'; -import React, {createContext, CSSProperties, ForwardedRef, forwardRef, Fragment, PropsWithChildren, ReactElement, ReactNode, useContext, useMemo, useRef} from 'react'; +import React, {createContext, ForwardedRef, forwardRef, Fragment, PropsWithChildren, ReactElement, ReactNode, useContext, useMemo, useRef} from 'react'; import {useDateFormatter, useLocale, useLocalizedStringFormatter} from '@react-aria/i18n'; import {useSpectrumContextProps} from './useSpectrumContextProps'; @@ -135,7 +135,10 @@ const cellStyles = style({ default: 2, isFirstWeek: 0 }, - paddingBottom: 2, + paddingBottom: { + default: 2, + isLastWeek: 0 + }, position: 'relative', width: 32, height: 32, @@ -156,7 +159,6 @@ const cellInnerStyles = style({ +const selectionBackgroundStyles = style<{isInvalid?: boolean, isFirstDayInWeek?: boolean, isLastDayInWeek?: boolean, isSelectionStart?: boolean, isSelectionEnd?: boolean, isPreviousDayNotSelected?: boolean, isNextDayNotSelected?: boolean}>({ position: 'absolute', zIndex: -1, top: 0, - insetStart: 'calc(-1 * var(--selection-span) * (var(--cell-width) + var(--cell-gap) + var(--cell-gap)))', - insetEnd: 0, + insetStart: { + default: -4, + isFirstDayInWeek: 0, + isSelectionStart: 0, + isPreviousDayNotSelected: 0 + }, + insetEnd: { + default: -4, + isLastDayInWeek: 0, + isSelectionEnd: 0, + isNextDayNotSelected: 0 + }, bottom: 0, - borderWidth: 2, - borderStyle: 'dashed', - borderColor: { - default: 'blue-800', // focus-indicator-color - isInvalid: 'negative-900', - forcedColors: { - default: 'ButtonText' - } + borderStartRadius: { + default: 'none', + isFirstDayInWeek: 'full', + isSelectionStart: 'full', + isPreviousDayNotSelected: 'full' + }, + borderEndRadius: { + default: 'none', + isLastDayInWeek: 'full', + isSelectionEnd: 'full', + isNextDayNotSelected: 'full' }, - borderStartRadius: 'full', - borderEndRadius: 'full', backgroundColor: { default: 'blue-subtle', isInvalid: 'negative-100', @@ -330,6 +352,58 @@ const selectionSpanStyles = style<{isInvalid?: boolean}>({ forcedColorAdjust: 'none' }); +const selectionBorderStyles = style<{isInvalid?: boolean, isFirstDayInWeek?: boolean, isLastDayInWeek?: boolean, isSelectionStart?: boolean, isSelectionEnd?: boolean, isPreviousDayNotSelected?: boolean, isNextDayNotSelected?: boolean}>({ + position: 'absolute', + zIndex: 1, + top: 0, + insetStart: { + default: -4, + isFirstDayInWeek: 0, + isSelectionStart: 0, + isPreviousDayNotSelected: 0 + }, + insetEnd: { + default: -4, + isLastDayInWeek: 0, + isSelectionEnd: 0, + isNextDayNotSelected: 0 + }, + bottom: 0, + borderStartWidth: { + default: 0, + isFirstDayInWeek: 1, + isSelectionStart: 1, + isPreviousDayNotSelected: 1 + }, + borderTopWidth: 1, + borderEndWidth: { + default: 0, + isLastDayInWeek: 1, + isSelectionEnd: 1, + isNextDayNotSelected: 1 + }, + borderBottomWidth: 1, + borderStyle: 'solid', + borderColor: { + default: 'blue-800', // focus-indicator-color + isInvalid: 'negative-900', + forcedColors: { + default: 'ButtonText' + } + }, + borderStartRadius: { + default: 'none', + isFirstDayInWeek: 'full', + isSelectionStart: 'full', + isPreviousDayNotSelected: 'full' + }, + borderEndRadius: { + default: 'none', + isLastDayInWeek: 'full', + isSelectionEnd: 'full', + isNextDayNotSelected: 'full' + } +}); /** * Calendars display a grid of days in one or more months and allow users to select a single date. */ @@ -508,28 +582,29 @@ const CalendarCell = (props: Omit & {firstDayOfWe let {locale} = useLocale(); let firstDayOfWeek = props.firstDayOfWeek; // Calculate the day and week index based on the date. - let {dayIndex, weekIndex} = useWeekAndDayIndices(props.date, locale, firstDayOfWeek); + let {dayIndex, weekIndex, lastWeekIndex} = useWeekAndDayIndices(props.date, locale, firstDayOfWeek); let calendarStateContext = useContext(CalendarStateContext); let rangeCalendarStateContext = useContext(RangeCalendarStateContext); let state = (calendarStateContext ?? rangeCalendarStateContext)!; + let isFirstWeek = weekIndex === 0; + let isLastWeek = weekIndex === lastWeekIndex; let isFirstChild = dayIndex === 0; let isLastChild = dayIndex === 6; return ( cellStyles({...renderProps, isFirstChild, isLastChild, isFirstWeek})}> + className={(renderProps) => cellStyles({...renderProps, isFirstChild, isLastChild, isFirstWeek, isLastWeek})}> {(renderProps) => } ); }; const CalendarCellInner = (props: Omit & {isRangeSelection: boolean, state: CalendarState | RangeCalendarState, weekIndex: number, dayIndex: number, renderProps?: CalendarCellRenderProps, date: DateValue}): ReactElement => { - let {weekIndex, dayIndex, date, renderProps, state, isRangeSelection} = props; - let {getDatesInWeek} = state; + let {dayIndex, date, renderProps, state, isRangeSelection} = props; let ref = useRef(null); let {isUnavailable, formattedDate, isSelected, isSelectionStart, isSelectionEnd, isInvalid} = renderProps!; // only apply the selection start/end styles if the start/end date is actually selectable (aka not unavailable) @@ -537,9 +612,6 @@ const CalendarCellInner = (props: Omit & {isRange isSelectionStart = isSelectionStart && (!isUnavailable || isInvalid); isSelectionEnd = isSelectionEnd && (!isUnavailable || isInvalid); - let startDate = startOfMonth(date); - let datesInWeek = getDatesInWeek(weekIndex, startDate); - let isDateInRange = (checkDate: CalendarDate) => { if (!('highlightedRange' in state) || !state.highlightedRange) { return state.isSelected(checkDate); @@ -553,20 +625,12 @@ const CalendarCellInner = (props: Omit & {isRange return state.isSelected(checkDate); }; - // Starting from the current day, find the first day before it in the current week that is not selected. - // Then, the span of selected days is the current day minus the first unselected day. - let firstUnselectedInRangeInWeek = datesInWeek.slice(0, dayIndex + 1).reverse().findIndex((date, i) => { - return date && i > 0 && (!isDateInRange(date) || date.month !== props.date.month); - }); - - let selectionSpan = -1; - if (firstUnselectedInRangeInWeek > -1 && isSelected) { - selectionSpan = firstUnselectedInRangeInWeek - 1; - } else if (isSelected) { - selectionSpan = dayIndex; - } let prevDay = date.subtract({days: 1}); let nextDay = date.add({days: 1}); + let isFirstDayInWeek = dayIndex === 0; + let isLastDayInWeek = dayIndex === 6; + let isPreviousDayNotSelected = !prevDay || (!isDateInRange(prevDay) || prevDay.month !== props.date.month); + let isNextDayNotSelected = !nextDay || (!isDateInRange(nextDay) || nextDay.month !== props.date.month); // when invalid, show background for all selected dates (including unavailable) to make continuous range appearance // when valid, only show background for available selected dates @@ -592,12 +656,14 @@ const CalendarCellInner = (props: Omit & {isRange ref={ref} style={pressScale(ref, {})(renderProps!)} className={cellInnerStyles({...renderProps!, isSelectionStart, isSelectionEnd, selectionMode: isRangeSelection ? 'range' : 'single'})}> +
{formattedDate}
{isUnavailable &&
}
- {isBackgroundStyleApplied &&
} + {isBackgroundStyleApplied &&
} + {isBackgroundStyleApplied &&
}
); }; @@ -616,7 +682,7 @@ function useWeekAndDayIndices( locale: string, firstDayOfWeek?: DayOfWeek ) { - let {dayIndex, weekIndex} = useMemo(() => { + let result = useMemo(() => { // Get the day index within the week (0-6) const dayIndex = getDayOfWeek(date, locale, firstDayOfWeek); @@ -628,12 +694,15 @@ function useWeekAndDayIndices( const dayOfMonth = date.day; const weekIndex = Math.floor((dayOfMonth + monthStartDayOfWeek - 1) / 7); + const lastDayOfMonth = startOfMonth(date).add({months: 1}).subtract({days: 1}); + const lastWeekIndex = Math.floor((lastDayOfMonth.day + monthStartDayOfWeek - 1) / 7); return { weekIndex, + lastWeekIndex, dayIndex }; }, [date, locale, firstDayOfWeek]); - return {dayIndex, weekIndex}; + return result; } diff --git a/packages/@react-spectrum/s2/src/DatePicker.tsx b/packages/@react-spectrum/s2/src/DatePicker.tsx index c5e1ee8ca40..260473b1b4d 100644 --- a/packages/@react-spectrum/s2/src/DatePicker.tsx +++ b/packages/@react-spectrum/s2/src/DatePicker.tsx @@ -57,7 +57,11 @@ export interface DatePickerProps extends * The maximum number of months to display at once in the calendar popover, if screen space permits. * @default 1 */ - maxVisibleMonths?: number + maxVisibleMonths?: number, + /** + * The error message to display when the calendar is invalid. + */ + errorMessage?: ReactNode } export const DatePickerContext = createContext>, HTMLDivElement>>(null); @@ -208,7 +212,8 @@ export const DatePicker = /*#__PURE__*/ (forwardRef as forwardRefType)(function + createCalendar={createCalendar} + errorMessage={errorMessage} /> {showTimeField && (
& {childre
extends * The maximum number of months to display at once in the calendar popover, if screen space permits. * @default 1 */ - maxVisibleMonths?: number + maxVisibleMonths?: number, + /** + * The error message to display when the calendar is invalid. + */ + errorMessage?: ReactNode } export const DateRangePickerContext = createContext>, HTMLDivElement>>(null); @@ -148,7 +152,8 @@ export const DateRangePicker = /*#__PURE__*/ (forwardRef as forwardRefType)(func + createCalendar={createCalendar} + errorMessage={errorMessage} /> {showTimeField && (
Date: Tue, 17 Mar 2026 15:15:45 -0500 Subject: [PATCH 05/13] feat: NumberParser assorted bug fixes (#8592) * fix: NumberParsing * fix test usage * fuzzier matching for numbers * fix ambiguous case with leading zeroes * fix last breaking tests * fix case of formatted numbers with no numerals * fix cases of numbers with no numerals * remove ambiguous case but allow for numbers to start with group characters * explanation * handle ambiguous group vs decimal case * Revert "handle ambiguous group vs decimal case" This reverts commit f459439dc10064f422c63abfa421d60d715d3a84. * Reapply "handle ambiguous group vs decimal case" This reverts commit 64959502c7ed112ec7192d52ef9adde6234b5c7e. * simplify * add test for 5927 * remove extra handling --- .../number/src/NumberParser.ts | 59 ++++++--- .../number/test/NumberParser.test.js | 95 +++++++++++++- .../numberfield/test/NumberField.test.js | 13 ++ .../stories/NumberField.stories.tsx | 22 +++- .../test/NumberField.test.js | 122 +++++++++++++++++- 5 files changed, 285 insertions(+), 26 deletions(-) diff --git a/packages/@internationalized/number/src/NumberParser.ts b/packages/@internationalized/number/src/NumberParser.ts index ae6448f027d..dfeb7c6c24f 100644 --- a/packages/@internationalized/number/src/NumberParser.ts +++ b/packages/@internationalized/number/src/NumberParser.ts @@ -19,7 +19,9 @@ interface Symbols { group?: string, literals: RegExp, numeral: RegExp, - index: (v: string) => string + numerals: string[], + index: (v: string) => string, + noNumeralUnits: Array<{unit: string, value: number}> } const CURRENCY_SIGN_REGEX = new RegExp('^.*\\(.*\\).*$'); @@ -130,13 +132,17 @@ class NumberParserImpl { } parse(value: string) { + let isGroupSymbolAllowed = this.formatter.resolvedOptions().useGrouping; // to parse the number, we need to remove anything that isn't actually part of the number, for example we want '-10.40' not '-10.40 USD' let fullySanitizedValue = this.sanitize(value); - if (this.symbols.group) { - // Remove group characters, and replace decimal points and numerals with ASCII values. - fullySanitizedValue = replaceAll(fullySanitizedValue, this.symbols.group, ''); + // Return NaN if there is a group symbol but useGrouping is false + if (!isGroupSymbolAllowed && this.symbols.group && fullySanitizedValue.includes(this.symbols.group)) { + return NaN; + } else if (this.symbols.group) { + fullySanitizedValue = fullySanitizedValue.replaceAll(this.symbols.group!, ''); } + if (this.symbols.decimal) { fullySanitizedValue = fullySanitizedValue.replace(this.symbols.decimal!, '.'); } @@ -189,12 +195,17 @@ class NumberParserImpl { if (this.options.currencySign === 'accounting' && CURRENCY_SIGN_REGEX.test(value)) { newValue = -1 * newValue; } - return newValue; } sanitize(value: string) { - // Remove literals and whitespace, which are allowed anywhere in the string + let isGroupSymbolAllowed = this.formatter.resolvedOptions().useGrouping; + // If the value is only a unit and it matches one of the formatted numbers where the value is part of the unit and doesn't have any numerals, then + // return the known value for that case. + if (this.symbols.noNumeralUnits.length > 0 && this.symbols.noNumeralUnits.find(obj => obj.unit === value)) { + return this.symbols.noNumeralUnits.find(obj => obj.unit === value)!.value.toString(); + } + value = value.replace(this.symbols.literals, ''); // Replace the ASCII minus sign with the minus sign used in the current locale @@ -207,23 +218,23 @@ class NumberParserImpl { // instead they use the , (44) character or apparently the (1548) character. if (this.options.numberingSystem === 'arab') { if (this.symbols.decimal) { - value = value.replace(',', this.symbols.decimal); - value = value.replace(String.fromCharCode(1548), this.symbols.decimal); + value = replaceAll(value, ',', this.symbols.decimal); + value = replaceAll(value, String.fromCharCode(1548), this.symbols.decimal); } - if (this.symbols.group) { + if (this.symbols.group && isGroupSymbolAllowed) { value = replaceAll(value, '.', this.symbols.group); } } // In some locale styles, such as swiss currency, the group character can be a special single quote // that keyboards don't typically have. This expands the character to include the easier to type single quote. - if (this.symbols.group === '’' && value.includes("'")) { + if (this.symbols.group === '’' && value.includes("'") && isGroupSymbolAllowed) { value = replaceAll(value, "'", this.symbols.group); } // fr-FR group character is narrow non-breaking space, char code 8239 (U+202F), but that's not a key on the french keyboard, // so allow space and non-breaking space as a group char as well - if (this.options.locale === 'fr-FR' && this.symbols.group) { + if (this.options.locale === 'fr-FR' && this.symbols.group && isGroupSymbolAllowed) { value = replaceAll(value, ' ', this.symbols.group); value = replaceAll(value, /\u00A0/g, this.symbols.group); } @@ -232,6 +243,7 @@ class NumberParserImpl { } isValidPartialNumber(value: string, minValue: number = -Infinity, maxValue: number = Infinity): boolean { + let isGroupSymbolAllowed = this.formatter.resolvedOptions().useGrouping; value = this.sanitize(value); // Remove minus or plus sign, which must be at the start of the string. @@ -241,18 +253,13 @@ class NumberParserImpl { value = value.slice(this.symbols.plusSign.length); } - // Numbers cannot start with a group separator - if (this.symbols.group && value.startsWith(this.symbols.group)) { - return false; - } - // Numbers that can't have any decimal values fail if a decimal character is typed if (this.symbols.decimal && value.indexOf(this.symbols.decimal) > -1 && this.options.maximumFractionDigits === 0) { return false; } // Remove numerals, groups, and decimals - if (this.symbols.group) { + if (this.symbols.group && isGroupSymbolAllowed) { value = replaceAll(value, this.symbols.group, ''); } value = value.replace(this.symbols.numeral, ''); @@ -282,12 +289,21 @@ function getSymbols(locale: string, formatter: Intl.NumberFormat, intlOptions: I maximumSignificantDigits: 21, roundingIncrement: 1, roundingPriority: 'auto', - roundingMode: 'halfExpand' + roundingMode: 'halfExpand', + useGrouping: true }); // Note: some locale's don't add a group symbol until there is a ten thousands place let allParts = symbolFormatter.formatToParts(-10000.111); let posAllParts = symbolFormatter.formatToParts(10000.111); let pluralParts = pluralNumbers.map(n => symbolFormatter.formatToParts(n)); + // if the plural parts include a unit but no integer or fraction, then we need to add the unit to the special set + let noNumeralUnits = pluralParts.map((p, i) => { + let unit = p.find(p => p.type === 'unit'); + if (unit && !p.some(p => p.type === 'integer' || p.type === 'fraction')) { + return {unit: unit.value, value: pluralNumbers[i]}; + } + return null; + }).filter(p => !!p); let minusSign = allParts.find(p => p.type === 'minusSign')?.value ?? '-'; let plusSign = posAllParts.find(p => p.type === 'plusSign')?.value; @@ -311,9 +327,10 @@ function getSymbols(locale: string, formatter: Intl.NumberFormat, intlOptions: I let pluralPartsLiterals = pluralParts.flatMap(p => p.filter(p => !nonLiteralParts.has(p.type)).map(p => escapeRegex(p.value))); let sortedLiterals = [...new Set([...allPartsLiterals, ...pluralPartsLiterals])].sort((a, b) => b.length - a.length); + // Match both whitespace and formatting characters let literals = sortedLiterals.length === 0 ? - new RegExp('[\\p{White_Space}]', 'gu') : - new RegExp(`${sortedLiterals.join('|')}|[\\p{White_Space}]`, 'gu'); + new RegExp('\\p{White_Space}|\\p{Cf}', 'gu') : + new RegExp(`${sortedLiterals.join('|')}|\\p{White_Space}|\\p{Cf}`, 'gu'); // These are for replacing non-latn characters with the latn equivalent let numerals = [...new Intl.NumberFormat(intlOptions.locale, {useGrouping: false}).format(9876543210)].reverse(); @@ -321,7 +338,7 @@ function getSymbols(locale: string, formatter: Intl.NumberFormat, intlOptions: I let numeral = new RegExp(`[${numerals.join('')}]`, 'g'); let index = d => String(indexes.get(d)); - return {minusSign, plusSign, decimal, group, literals, numeral, index}; + return {minusSign, plusSign, decimal, group, literals, numeral, numerals, index, noNumeralUnits}; } function replaceAll(str: string, find: string | RegExp, replace: string) { diff --git a/packages/@internationalized/number/test/NumberParser.test.js b/packages/@internationalized/number/test/NumberParser.test.js index a9266d997cf..19222fe946a 100644 --- a/packages/@internationalized/number/test/NumberParser.test.js +++ b/packages/@internationalized/number/test/NumberParser.test.js @@ -56,6 +56,11 @@ describe('NumberParser', function () { expect(new NumberParser('en-US', {style: 'decimal'}).parse('1abc')).toBe(NaN); }); + it('should return NaN for invalid grouping', function () { + expect(new NumberParser('en-US', {useGrouping: false}).parse('1234,7')).toBeNaN(); + expect(new NumberParser('de-DE', {useGrouping: false}).parse('1234.7')).toBeNaN(); + }); + describe('currency', function () { it('should parse without the currency symbol', function () { expect(new NumberParser('en-US', {currency: 'USD', style: 'currency'}).parse('10.50')).toBe(10.5); @@ -194,8 +199,13 @@ describe('NumberParser', function () { expect(new NumberParser('de-CH', {style: 'currency', currency: 'CHF'}).parse("CHF 1'000.00")).toBe(1000); }); + it('should parse arabic singular and dual counts', () => { + expect(new NumberParser('ar-AE', {style: 'unit', unit: 'day', unitDisplay: 'long'}).parse('يومان')).toBe(2); + expect(new NumberParser('ar-AE', {style: 'unit', unit: 'day', unitDisplay: 'long'}).parse('يوم')).toBe(1); + }); + describe('round trips', function () { - fc.configureGlobal({numRuns: 200}); + fc.configureGlobal({numRuns: 2000}); // Locales have to include: 'de-DE', 'ar-EG', 'fr-FR' and possibly others // But for the moment they are not properly supported const localesArb = fc.constantFrom(...locales); @@ -301,6 +311,78 @@ describe('NumberParser', function () { const formattedOnce = formatter.format(1); expect(formatter.format(parser.parse(formattedOnce))).toBe(formattedOnce); }); + it('should handle small numbers', () => { + let locale = 'ar-AE'; + let options = { + style: 'decimal', + minimumIntegerDigits: 4, + maximumSignificantDigits: 1 + }; + const formatter = new Intl.NumberFormat(locale, options); + const parser = new NumberParser(locale, options); + const formattedOnce = formatter.format(2.220446049250313e-16); + expect(formatter.format(parser.parse(formattedOnce))).toBe(formattedOnce); + }); + it('should handle currency small numbers', () => { + let locale = 'ar-AE-u-nu-latn'; + let options = { + style: 'currency', + currency: 'USD' + }; + const formatter = new Intl.NumberFormat(locale, options); + const parser = new NumberParser(locale, options); + const formattedOnce = formatter.format(2.220446049250313e-16); + expect(formatter.format(parser.parse(formattedOnce))).toBe(formattedOnce); + }); + it('should handle hanidec small numbers', () => { + let locale = 'ar-AE-u-nu-hanidec'; + let options = { + style: 'decimal' + }; + const formatter = new Intl.NumberFormat(locale, options); + const parser = new NumberParser(locale, options); + const formattedOnce = formatter.format(2.220446049250313e-16); + expect(formatter.format(parser.parse(formattedOnce))).toBe(formattedOnce); + }); + it('should handle beng with minimum integer digits', () => { + let locale = 'ar-AE-u-nu-beng'; + let options = { + style: 'decimal', + minimumIntegerDigits: 4, + maximumFractionDigits: 0 + }; + const formatter = new Intl.NumberFormat(locale, options); + const parser = new NumberParser(locale, options); + const formattedOnce = formatter.format(2.220446049250313e-16); + expect(formatter.format(parser.parse(formattedOnce))).toBe(formattedOnce); + }); + it('should handle percent with minimum integer digits', () => { + let locale = 'ar-AE-u-nu-latn'; + let options = { + style: 'percent', + minimumIntegerDigits: 4, + minimumFractionDigits: 9, + maximumSignificantDigits: 1, + maximumFractionDigits: undefined + }; + const formatter = new Intl.NumberFormat(locale, options); + const parser = new NumberParser(locale, options); + const formattedOnce = formatter.format(0.0095); + expect(formatter.format(parser.parse(formattedOnce))).toBe(formattedOnce); + }); + it('should handle non-grouping in russian locale', () => { + let locale = 'ru-RU'; + let options = { + style: 'percent', + useGrouping: false, + minimumFractionDigits: undefined, + maximumFractionDigits: undefined + }; + const formatter = new Intl.NumberFormat(locale, options); + const parser = new NumberParser(locale, options); + const formattedOnce = formatter.format(2.220446049250313e-16); + expect(formatter.format(parser.parse(formattedOnce))).toBe(formattedOnce); + }); }); }); @@ -327,14 +409,21 @@ describe('NumberParser', function () { }); it('should support group characters', function () { - expect(new NumberParser('en-US', {style: 'decimal'}).isValidPartialNumber(',')).toBe(true); // en-US-u-nu-arab uses commas as the decimal point character - expect(new NumberParser('en-US', {style: 'decimal'}).isValidPartialNumber(',000')).toBe(false); // latin numerals cannot follow arab decimal point + // starting with arabic decimal point + expect(new NumberParser('en-US', {style: 'decimal'}).isValidPartialNumber(',')).toBe(true); + expect(new NumberParser('en-US', {style: 'decimal'}).isValidPartialNumber(',000')).toBe(true); + expect(new NumberParser('en-US', {style: 'decimal'}).isValidPartialNumber('000,000')).toBe(true); expect(new NumberParser('en-US', {style: 'decimal'}).isValidPartialNumber('1,000')).toBe(true); expect(new NumberParser('en-US', {style: 'decimal'}).isValidPartialNumber('-1,000')).toBe(true); expect(new NumberParser('en-US', {style: 'decimal'}).isValidPartialNumber('1,000,000')).toBe(true); expect(new NumberParser('en-US', {style: 'decimal'}).isValidPartialNumber('-1,000,000')).toBe(true); }); + it('should return false for invalid grouping', function () { + expect(new NumberParser('en-US', {useGrouping: false}).isValidPartialNumber('1234,7')).toBe(false); + expect(new NumberParser('de-DE', {useGrouping: false}).isValidPartialNumber('1234.7')).toBe(false); + }); + it('should reject random characters', function () { expect(new NumberParser('en-US', {style: 'decimal'}).isValidPartialNumber('g')).toBe(false); expect(new NumberParser('en-US', {style: 'decimal'}).isValidPartialNumber('1abc')).toBe(false); diff --git a/packages/@react-spectrum/numberfield/test/NumberField.test.js b/packages/@react-spectrum/numberfield/test/NumberField.test.js index 17395b00e83..c3941a229a1 100644 --- a/packages/@react-spectrum/numberfield/test/NumberField.test.js +++ b/packages/@react-spectrum/numberfield/test/NumberField.test.js @@ -2046,6 +2046,19 @@ describe('NumberField', function () { expect(textField).toHaveAttribute('value', formatter.format(21)); }); + it('should maintain original parser and formatting when restoring a previous value', async () => { + let {textField} = renderNumberField({onChange: onChangeSpy, defaultValue: 10}); + expect(textField).toHaveAttribute('value', '10'); + + await user.tab(); + await user.clear(textField); + await user.keyboard(',123'); + act(() => {textField.blur();}); + expect(textField).toHaveAttribute('value', '123'); + expect(onChangeSpy).toHaveBeenCalledTimes(1); + expect(onChangeSpy).toHaveBeenCalledWith(123); + }); + describe('beforeinput', () => { let getTargetRanges = InputEvent.prototype.getTargetRanges; beforeEach(() => { diff --git a/packages/react-aria-components/stories/NumberField.stories.tsx b/packages/react-aria-components/stories/NumberField.stories.tsx index 93d0a94ce62..9f7cf521fc5 100644 --- a/packages/react-aria-components/stories/NumberField.stories.tsx +++ b/packages/react-aria-components/stories/NumberField.stories.tsx @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {Button, FieldError, Group, Input, Label, NumberField, NumberFieldProps} from 'react-aria-components'; +import {Button, FieldError, Group, I18nProvider, Input, Label, NumberField, NumberFieldProps} from 'react-aria-components'; import {Meta, StoryObj} from '@storybook/react'; import React, {useState} from 'react'; import './styles.css'; @@ -72,3 +72,23 @@ export const NumberFieldControlledExample = { ) }; + +export const ArabicNumberFieldExample = { + args: { + defaultValue: 0, + formatOptions: {style: 'unit', unit: 'day', unitDisplay: 'long'} + }, + render: (args) => ( + + (v & 1 ? 'Invalid value' : null)}> + + + + + + + + + + ) +}; diff --git a/packages/react-aria-components/test/NumberField.test.js b/packages/react-aria-components/test/NumberField.test.js index cd3516072e1..f72d76bacbd 100644 --- a/packages/react-aria-components/test/NumberField.test.js +++ b/packages/react-aria-components/test/NumberField.test.js @@ -13,7 +13,7 @@ jest.mock('@react-aria/live-announcer'); import {act, pointerMap, render} from '@react-spectrum/test-utils-internal'; import {announce} from '@react-aria/live-announcer'; -import {Button, FieldError, Form, Group, Input, Label, NumberField, NumberFieldContext, Text} from '../'; +import {Button, FieldError, Form, Group, I18nProvider, Input, Label, NumberField, NumberFieldContext, Text} from '../'; import React from 'react'; import userEvent from '@testing-library/user-event'; @@ -198,6 +198,114 @@ describe('NumberField', () => { expect(numberfield).not.toHaveAttribute('data-invalid'); }); + it('supports pasting value in another numbering system', async () => { + let {getByRole, rerender} = render(); + let input = getByRole('textbox'); + await user.tab(); + act(() => { + input.setSelectionRange(0, input.value.length); + }); + await user.paste('3.000.000,25'); + await user.keyboard('{Enter}'); + expect(input).toHaveValue('1,024'); + + act(() => { + input.setSelectionRange(0, input.value.length); + }); + await user.paste('3 000 000,25'); + await user.keyboard('{Enter}'); + expect(input).toHaveValue('300,000,025'); + + rerender(); + + act(() => { + input.setSelectionRange(0, input.value.length); + }); + await user.paste('3 000 000,256789'); + await user.keyboard('{Enter}'); + expect(input).toHaveValue('$3,000,000,256,789.00'); + + act(() => { + input.setSelectionRange(0, input.value.length); + }); + await user.paste('1,000'); + await user.keyboard('{Enter}'); + expect(input).toHaveValue('$1,000.00', 'Ambiguous value should be parsed using the current locale'); + + act(() => { + input.setSelectionRange(0, input.value.length); + }); + + await user.paste('1.000'); + await user.keyboard('{Enter}'); + expect(input).toHaveValue('$1.00', 'Ambiguous value should be parsed using the current locale'); + }); + + it('should support arabic singular and dual counts', async () => { + let onChange = jest.fn(); + let {getByRole} = render( + + + + + + + + + + + + ); + let input = getByRole('textbox'); + await user.tab(); + await user.keyboard('{ArrowUp}'); + expect(onChange).toHaveBeenLastCalledWith(1); + expect(input).toHaveValue('يوم'); + + await user.keyboard('{ArrowUp}'); + expect(input).toHaveValue('يومان'); + expect(onChange).toHaveBeenLastCalledWith(2); + }); + + it('should not type the grouping characters when useGrouping is false', async () => { + let {getByRole} = render(); + let input = getByRole('textbox'); + + await user.keyboard('102,4'); + expect(input).toHaveAttribute('value', '1024'); + + await user.clear(input); + expect(input).toHaveAttribute('value', ''); + + await user.paste('102,4'); + await user.tab(); + expect(input).toHaveAttribute('value', '1024'); + + await user.paste('1,024'); + await user.tab(); + expect(input).toHaveAttribute('value', '1024'); + + }); + + it('should not type the grouping characters when useGrouping is false and in German locale', async () => { + let {getByRole} = render(); + let input = getByRole('textbox'); + + await user.keyboard('102.4'); + expect(input).toHaveAttribute('value', '1024'); + + await user.clear(input); + expect(input).toHaveAttribute('value', ''); + + await user.paste('102.4'); + await user.tab(); + expect(input).toHaveAttribute('value', '1024'); + + await user.paste('1.024'); + await user.tab(); + expect(input).toHaveAttribute('value', '1024'); + }); + it('should trigger onChange via programmatic click() on stepper buttons', () => { const onChange = jest.fn(); const {container} = render( @@ -215,6 +323,18 @@ describe('NumberField', () => { expect(onChange).toHaveBeenCalledWith(1024); }); + it('should allow you to delete the first digit in a number if it is followed by a group separator', async () => { + let {getByRole} = render(); + let input = getByRole('textbox'); + await user.tab(); + await user.keyboard('{ArrowLeft}'); + await user.keyboard('{ArrowRight}'); + await user.keyboard('{Backspace}'); + expect(input).toHaveValue(',024'); + await user.keyboard('{Enter}'); + expect(input).toHaveValue('24'); + }); + it('supports onChange', async () => { let onChange = jest.fn(); let {getByRole} = render(); From ca9e92b14a51edfe040f46a11c88b5e06306f4c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikolas=20Schr=C3=B6ter?= <25958801+nwidynski@users.noreply.github.com> Date: Tue, 17 Mar 2026 21:20:35 +0100 Subject: [PATCH 06/13] Feat: Add support for horizontal `orientation` to `GridList` & `ListBox` (#8533) * feat: add orientation to layout * feat: forward orientation to gridlist keyboard delegate * fix: remove duplicate keyboard delegate in gridlist * Feat: Add support for `orientation` to `KeyboardDelegate` interface * fix: implement layout delegate interface * fix: enumerable keys * fix: bidirectional overscan for table layout * chore: remove orientation fallback from domlayoutdelegate * fix: listbox stories * chore: review * fix: gridlist orientation * chore: naming * fix: content nodes * fix page up/down navigation for horizontal lists --------- Co-authored-by: Daniel Lu --- .../@react-aria/listbox/src/useListBox.ts | 12 +- .../selection/src/ListKeyboardDelegate.ts | 2 +- .../selection/src/useSelectableList.ts | 17 +- .../@react-spectrum/table/test/TableTests.js | 2 +- .../@react-stately/layout/src/ListLayout.ts | 173 +++++++++--------- .../virtualizer/src/OverscanManager.ts | 10 +- .../react-aria-components/src/GridList.tsx | 21 +-- .../react-aria-components/src/ListBox.tsx | 8 +- .../stories/ListBox.stories.tsx | 58 ++++-- .../react-aria-components/stories/utils.tsx | 12 +- 10 files changed, 183 insertions(+), 132 deletions(-) diff --git a/packages/@react-aria/listbox/src/useListBox.ts b/packages/@react-aria/listbox/src/useListBox.ts index c5ac0984bbf..4fdbdffbceb 100644 --- a/packages/@react-aria/listbox/src/useListBox.ts +++ b/packages/@react-aria/listbox/src/useListBox.ts @@ -11,7 +11,7 @@ */ import {AriaListBoxProps} from '@react-types/listbox'; -import {DOMAttributes, KeyboardDelegate, LayoutDelegate, RefObject} from '@react-types/shared'; +import {DOMAttributes, KeyboardDelegate, LayoutDelegate, Orientation, RefObject} from '@react-types/shared'; import {filterDOMProps, mergeProps, useId} from '@react-aria/utils'; import {listData} from './utils'; import {ListState} from '@react-stately/list'; @@ -55,7 +55,13 @@ export interface AriaListBoxOptions extends Omit, 'childr * - 'override': links override all other interactions (link items are not selectable). * @default 'override' */ - linkBehavior?: 'action' | 'selection' | 'override' + linkBehavior?: 'action' | 'selection' | 'override', + + /** + * The primary orientation of the items. Usually this is the direction that the collection scrolls. + * @default 'vertical' + */ + orientation?: Orientation } /** @@ -68,6 +74,7 @@ export function useListBox(props: AriaListBoxOptions, state: ListState, let domProps = filterDOMProps(props, {labelable: true}); // Use props instead of state here. We don't want this to change due to long press. let selectionBehavior = props.selectionBehavior || 'toggle'; + let orientation = props.orientation || 'vertical'; let linkBehavior = props.linkBehavior || (selectionBehavior === 'replace' ? 'action' : 'override'); if (selectionBehavior === 'toggle' && linkBehavior === 'action') { // linkBehavior="action" does not work with selectionBehavior="toggle" because there is no way @@ -119,6 +126,7 @@ export function useListBox(props: AriaListBoxOptions, state: ListState, 'aria-multiselectable': 'true' } : {}, { role: 'listbox', + 'aria-orientation': orientation, ...mergeProps(fieldProps, listProps) }) }; diff --git a/packages/@react-aria/selection/src/ListKeyboardDelegate.ts b/packages/@react-aria/selection/src/ListKeyboardDelegate.ts index 21239d23dad..eb223deae50 100644 --- a/packages/@react-aria/selection/src/ListKeyboardDelegate.ts +++ b/packages/@react-aria/selection/src/ListKeyboardDelegate.ts @@ -248,7 +248,7 @@ export class ListKeyboardDelegate implements KeyboardDelegate { let nextKey: Key | null = key; if (this.orientation === 'horizontal') { - let pageX = Math.min(this.layoutDelegate.getContentSize().width, itemRect.y - itemRect.width + this.layoutDelegate.getVisibleRect().width); + let pageX = Math.min(this.layoutDelegate.getContentSize().width, itemRect.x - itemRect.width + this.layoutDelegate.getVisibleRect().width); while (itemRect && itemRect.x < pageX && nextKey != null) { nextKey = this.getKeyBelow(nextKey); diff --git a/packages/@react-aria/selection/src/useSelectableList.ts b/packages/@react-aria/selection/src/useSelectableList.ts index 98072b7c3ee..0e319623423 100644 --- a/packages/@react-aria/selection/src/useSelectableList.ts +++ b/packages/@react-aria/selection/src/useSelectableList.ts @@ -11,7 +11,7 @@ */ import {AriaSelectableCollectionOptions, useSelectableCollection} from './useSelectableCollection'; -import {Collection, DOMAttributes, Key, KeyboardDelegate, LayoutDelegate, Node} from '@react-types/shared'; +import {Collection, DOMAttributes, Key, KeyboardDelegate, LayoutDelegate, Node, Orientation} from '@react-types/shared'; import {ListKeyboardDelegate} from './ListKeyboardDelegate'; import {useCollator} from '@react-aria/i18n'; import {useMemo} from 'react'; @@ -34,7 +34,12 @@ export interface AriaSelectableListOptions extends Omit + disabledKeys: Set, + /** + * The primary orientation of the items. Usually this is the direction that the collection scrolls. + * @default 'vertical' + */ + orientation?: Orientation } export interface SelectableListAria { @@ -54,7 +59,8 @@ export function useSelectableList(props: AriaSelectableListOptions): SelectableL disabledKeys, ref, keyboardDelegate, - layoutDelegate + layoutDelegate, + orientation } = props; // By default, a KeyboardDelegate is provided which uses the DOM to query layout information (e.g. for page up/page down). @@ -68,9 +74,10 @@ export function useSelectableList(props: AriaSelectableListOptions): SelectableL disabledBehavior, ref, collator, - layoutDelegate + layoutDelegate, + orientation }) - ), [keyboardDelegate, layoutDelegate, collection, disabledKeys, ref, collator, disabledBehavior]); + ), [keyboardDelegate, layoutDelegate, collection, disabledKeys, ref, collator, disabledBehavior, orientation]); let {collectionProps} = useSelectableCollection({ ...props, diff --git a/packages/@react-spectrum/table/test/TableTests.js b/packages/@react-spectrum/table/test/TableTests.js index c2f21cc6fd5..cef92442a0c 100644 --- a/packages/@react-spectrum/table/test/TableTests.js +++ b/packages/@react-spectrum/table/test/TableTests.js @@ -1990,7 +1990,7 @@ export let tableTests = () => { let row = cell.closest('[role=row]'); let cells = within(row).getAllByRole('gridcell'); let rowHeaders = within(row).getAllByRole('rowheader'); - expect(cells).toHaveLength(9); + expect(cells).toHaveLength(10); expect(rowHeaders).toHaveLength(1); expect(cells[0]).toHaveAttribute('aria-colindex', '1'); // checkbox expect(rowHeaders[0]).toHaveAttribute('aria-colindex', '2'); // rowheader diff --git a/packages/@react-stately/layout/src/ListLayout.ts b/packages/@react-stately/layout/src/ListLayout.ts index 7060ad08edc..7014381e9ec 100644 --- a/packages/@react-stately/layout/src/ListLayout.ts +++ b/packages/@react-stately/layout/src/ListLayout.ts @@ -10,11 +10,16 @@ * governing permissions and limitations under the License. */ -import {Collection, DropTarget, DropTargetDelegate, ItemDropTarget, Key, Node} from '@react-types/shared'; +import {Collection, DropTarget, DropTargetDelegate, ItemDropTarget, Key, Node, Orientation} from '@react-types/shared'; import {getChildNodes} from '@react-stately/collections'; import {InvalidationContext, Layout, LayoutInfo, Rect, Size} from '@react-stately/virtualizer'; export interface ListLayoutOptions { + /** + * The primary orientation of the items. Usually this is the direction that the collection scrolls. + * @default 'vertical' + */ + orientation?: Orientation, /** * The fixed height of a row in px. * @default 48 @@ -70,6 +75,7 @@ const DEFAULT_HEIGHT = 48; */ export class ListLayout extends Layout, O> implements DropTargetDelegate { protected rowHeight: number | null; + protected orientation: Orientation; protected estimatedRowHeight: number | null; protected headingHeight: number | null; protected estimatedHeadingHeight: number | null; @@ -94,6 +100,7 @@ export class ListLayout exte constructor(options: ListLayoutOptions = {}) { super(); this.rowHeight = options.rowHeight ?? null; + this.orientation = options.orientation ?? 'vertical'; this.estimatedRowHeight = options.estimatedRowHeight ?? null; this.headingHeight = options.headingHeight ?? null; this.estimatedHeadingHeight = options.estimatedHeadingHeight ?? null; @@ -121,23 +128,27 @@ export class ListLayout exte } getVisibleLayoutInfos(rect: Rect): LayoutInfo[] { + let visibleRect = rect.copy(); + let offsetProperty = this.orientation === 'horizontal' ? 'x' : 'y'; + let heightProperty = this.orientation === 'horizontal' ? 'width' : 'height'; + // Adjust rect to keep number of visible rows consistent. - // (only if height > 1 for getDropTargetFromPoint) - if (rect.height > 1) { + // (only if height > 1 or width > 1 for getDropTargetFromPoint) + if (visibleRect[heightProperty] > 1) { let rowHeight = (this.rowHeight ?? this.estimatedRowHeight ?? DEFAULT_HEIGHT) + this.gap; - rect.y = Math.floor(rect.y / rowHeight) * rowHeight; - rect.height = Math.ceil(rect.height / rowHeight) * rowHeight; + visibleRect[offsetProperty] = Math.floor(visibleRect[offsetProperty] / rowHeight) * rowHeight; + visibleRect[heightProperty] = Math.ceil(visibleRect[heightProperty] / rowHeight) * rowHeight; } // If layout hasn't yet been done for the requested rect, union the // new rect with the existing valid rect, and recompute. - this.layoutIfNeeded(rect); + this.layoutIfNeeded(visibleRect); let res: LayoutInfo[] = []; let addNodes = (nodes: LayoutNode[]) => { for (let node of nodes) { - if (this.isVisible(node, rect)) { + if (this.isVisible(node, visibleRect)) { res.push(node.layoutInfo); if (node.children) { @@ -194,6 +205,7 @@ export class ListLayout exte let options = invalidationContext.layoutOptions; return invalidationContext.sizeChanged || this.rowHeight !== (options?.rowHeight ?? this.rowHeight) + || this.orientation !== (options?.orientation ?? this.orientation) || this.headingHeight !== (options?.headingHeight ?? this.headingHeight) || this.loaderHeight !== (options?.loaderHeight ?? this.loaderHeight) || this.gap !== (options?.gap ?? this.gap) @@ -202,6 +214,7 @@ export class ListLayout exte shouldInvalidateLayoutOptions(newOptions: O, oldOptions: O): boolean { return newOptions.rowHeight !== oldOptions.rowHeight + || newOptions.orientation !== oldOptions.orientation || newOptions.estimatedRowHeight !== oldOptions.estimatedRowHeight || newOptions.headingHeight !== oldOptions.headingHeight || newOptions.estimatedHeadingHeight !== oldOptions.estimatedHeadingHeight @@ -224,6 +237,7 @@ export class ListLayout exte let options = invalidationContext.layoutOptions; this.rowHeight = options?.rowHeight ?? this.rowHeight; + this.orientation = options?.orientation ?? this.orientation; this.estimatedRowHeight = options?.estimatedRowHeight ?? this.estimatedRowHeight; this.headingHeight = options?.headingHeight ?? this.headingHeight; this.estimatedHeadingHeight = options?.estimatedHeadingHeight ?? this.estimatedHeadingHeight; @@ -251,7 +265,7 @@ export class ListLayout exte this.validRect = this.requestedRect.copy(); } - protected buildCollection(y: number = this.padding): LayoutNode[] { + protected buildCollection(offset: number = this.padding): LayoutNode[] { let collection = this.virtualizer!.collection; // filter out content nodes since we don't want them to affect the height // Tree specific for now, if we add content nodes to other collection items, we might need to reconsider this @@ -260,19 +274,21 @@ export class ListLayout exte let nodes: LayoutNode[] = []; let isEmptyOrLoading = collection?.size === 0; if (isEmptyOrLoading) { - y = 0; + offset = 0; } for (let node of collectionNodes) { + let offsetProperty = this.orientation === 'horizontal' ? 'x' : 'y'; + let maxOffsetProperty = this.orientation === 'horizontal' ? 'maxX' : 'maxY'; let rowHeight = (this.rowHeight ?? this.estimatedRowHeight ?? DEFAULT_HEIGHT) + this.gap; // Skip rows before the valid rectangle unless they are already cached. - if (node.type === 'item' && y + rowHeight < this.requestedRect.y && !this.isValid(node, y)) { - y += rowHeight; + if (node.type === 'item' && offset + rowHeight < this.requestedRect[offsetProperty] && !this.isValid(node, offset)) { + offset += rowHeight; continue; } - let layoutNode = this.buildChild(node, this.padding, y, null); - y = layoutNode.layoutInfo.rect.maxY + this.gap; + let layoutNode = this.orientation === 'horizontal' ? this.buildChild(node, offset, this.padding, null) : this.buildChild(node, this.padding, offset, null); + offset = layoutNode.layoutInfo.rect[maxOffsetProperty] + this.gap; nodes.push(layoutNode); if (node.type === 'loader') { let index = loaderNodes.indexOf(node); @@ -282,44 +298,45 @@ export class ListLayout exte // Build each loader that exists in the collection that is outside the visible rect so that they are persisted // at the proper estimated location. If the node.type is "section" then we don't do this shortcut since we have to // build the sections to see how tall they are. - if ((node.type === 'item' || node.type === 'loader') && y > this.requestedRect.maxY) { + if ((node.type === 'item' || node.type === 'loader') && offset > this.requestedRect[maxOffsetProperty]) { let lastProcessedIndex = collectionNodes.indexOf(node); for (let loaderNode of loaderNodes) { let loaderNodeIndex = collectionNodes.indexOf(loaderNode); // Subtract by an additional 1 since we've already added the current item's height to y - y += (loaderNodeIndex - lastProcessedIndex - 1) * rowHeight; - let loader = this.buildChild(loaderNode, this.padding, y, null); + offset += (loaderNodeIndex - lastProcessedIndex - 1) * rowHeight; + let loader = this.orientation === 'horizontal' ? this.buildChild(loaderNode, offset, this.padding, null) : this.buildChild(loaderNode, this.padding, offset, null); nodes.push(loader); - y = loader.layoutInfo.rect.maxY; + offset = loader.layoutInfo.rect[maxOffsetProperty]; lastProcessedIndex = loaderNodeIndex; } // Account for the rest of the items after the last loader spinner, subtract by 1 since we've processed the current node's height already - y += (collectionNodes.length - lastProcessedIndex - 1) * rowHeight; + offset += (collectionNodes.length - lastProcessedIndex - 1) * rowHeight; break; } } - y -= this.gap; - y += isEmptyOrLoading ? 0 : this.padding; - this.contentSize = new Size(this.virtualizer!.visibleRect.width, y); + offset = Math.max(offset - this.gap, 0); + offset += isEmptyOrLoading ? 0 : this.padding; + this.contentSize = this.orientation === 'horizontal' ? new Size(offset, this.virtualizer!.visibleRect.height) : new Size(this.virtualizer!.visibleRect.width, offset); return nodes; } - protected isValid(node: Node, y: number): boolean { + protected isValid(node: Node, offset: number): boolean { let cached = this.layoutNodes.get(node.key); + let offsetProperty = this.orientation === 'horizontal' ? 'x' : 'y'; return ( !this.invalidateEverything && !!cached && cached.node === node && - y === cached.layoutInfo.rect.y && + offset === cached.layoutInfo.rect[offsetProperty] && cached.layoutInfo.rect.intersects(this.validRect) && cached.validRect.containsRect(cached.layoutInfo.rect.intersection(this.requestedRect)) ); } protected buildChild(node: Node, x: number, y: number, parentKey: Key | null): LayoutNode { - if (this.isValid(node, y)) { + if (this.isValid(node, this.orientation === 'horizontal' ? x : y)) { return this.layoutNodes.get(node.key)!; } @@ -350,11 +367,17 @@ export class ListLayout exte protected buildLoader(node: Node, x: number, y: number): LayoutNode { let rect = new Rect(x, y, this.padding, 0); - let layoutInfo = new LayoutInfo('loader', node.key, rect); - rect.width = this.virtualizer!.contentSize.width - this.padding - x; + let layoutInfo = new LayoutInfo(node.type, node.key, rect); + // Note that if the user provides isLoading to their sentinel during a case where they only want to render the emptyState, this will reserve // room for the loader alongside rendering the emptyState - rect.height = node.props.isLoading ? this.loaderHeight ?? this.rowHeight ?? this.estimatedRowHeight ?? DEFAULT_HEIGHT : 0; + if (this.orientation === 'horizontal') { + rect.height = this.virtualizer!.contentSize.height - this.padding - y; + rect.width = node.props.isLoading ? this.loaderHeight ?? this.rowHeight ?? this.estimatedRowHeight ?? DEFAULT_HEIGHT : 0; + } else { + rect.width = this.virtualizer!.contentSize.width - this.padding - x; + rect.height = node.props.isLoading ? this.loaderHeight ?? this.rowHeight ?? this.estimatedRowHeight ?? DEFAULT_HEIGHT : 0; + } return { layoutInfo, @@ -364,11 +387,16 @@ export class ListLayout exte protected buildSection(node: Node, x: number, y: number): LayoutNode { let collection = this.virtualizer!.collection; - let width = this.virtualizer!.visibleRect.width - this.padding; - let rect = new Rect(x, y, width - x, 0); + let width = this.virtualizer!.visibleRect.width - this.padding - x; + let height = this.virtualizer!.visibleRect.height - this.padding - y; + let rect = this.orientation === 'horizontal' ? new Rect(x, y, 0, height) : new Rect(x, y, width, 0); let layoutInfo = new LayoutInfo(node.type, node.key, rect); - let startY = y; + let offset = this.orientation === 'horizontal' ? x : y; + let offsetProperty = this.orientation === 'horizontal' ? 'x' : 'y'; + let maxOffsetProperty = this.orientation === 'horizontal' ? 'maxX' : 'maxY'; + let heightProperty = this.orientation === 'horizontal' ? 'width' : 'height'; + let skipped = 0; let children: LayoutNode[] = []; for (let child of getChildNodes(node, collection)) { @@ -380,25 +408,25 @@ export class ListLayout exte let rowHeight = (this.rowHeight ?? this.estimatedRowHeight ?? DEFAULT_HEIGHT) + this.gap; // Skip rows before the valid rectangle unless they are already cached. - if (y + rowHeight < this.requestedRect.y && !this.isValid(node, y)) { - y += rowHeight; + if (offset + rowHeight < this.requestedRect[offsetProperty] && !this.isValid(node, offset)) { + offset += rowHeight; skipped++; continue; } - let layoutNode = this.buildChild(child, x, y, layoutInfo.key); - y = layoutNode.layoutInfo.rect.maxY + this.gap; + let layoutNode = this.orientation === 'horizontal' ? this.buildChild(child, offset, y, layoutInfo.key) : this.buildChild(child, x, offset, layoutInfo.key); + offset = layoutNode.layoutInfo.rect[maxOffsetProperty] + this.gap; children.push(layoutNode); - if (y > this.requestedRect.maxY) { + if (offset > this.requestedRect[maxOffsetProperty]) { // Estimate the remaining height for rows that we don't need to layout right now. - y += ([...getChildNodes(node, collection)].length - (children.length + skipped)) * rowHeight; + offset += ([...getChildNodes(node, collection)].length - (children.length + skipped)) * rowHeight; break; } } - y -= this.gap; - rect.height = y - startY; + offset -= this.gap; + rect[heightProperty] = offset - (this.orientation === 'horizontal' ? x : y); return { layoutInfo, @@ -409,45 +437,14 @@ export class ListLayout exte } protected buildSectionHeader(node: Node, x: number, y: number): LayoutNode { - let width = this.virtualizer!.visibleRect.width - this.padding; - let rectHeight = this.headingHeight; - let isEstimated = false; - - // If no explicit height is available, use an estimated height. - if (rectHeight == null) { - // If a previous version of this layout info exists, reuse its height. - // Mark as estimated if the size of the overall virtualizer changed, - // or the content of the item changed. - let previousLayoutNode = this.layoutNodes.get(node.key); - let previousLayoutInfo = previousLayoutNode?.layoutInfo; - if (previousLayoutInfo) { - let curNode = this.virtualizer!.collection.getItem(node.key); - let lastNode = this.lastCollection ? this.lastCollection.getItem(node.key) : null; - rectHeight = previousLayoutInfo.rect.height; - isEstimated = width !== previousLayoutInfo.rect.width || curNode !== lastNode || previousLayoutInfo.estimatedSize; - } else { - rectHeight = (node.rendered ? this.estimatedHeadingHeight : 0); - isEstimated = true; - } - } - - if (rectHeight == null) { - rectHeight = DEFAULT_HEIGHT; - } - - let headerRect = new Rect(x, y, width - x, rectHeight); - let header = new LayoutInfo('header', node.key, headerRect); - header.estimatedSize = isEstimated; - return { - layoutInfo: header, - children: [], - validRect: header.rect.intersection(this.requestedRect), - node - }; + return this.buildItem(node, x, y); } protected buildItem(node: Node, x: number, y: number): LayoutNode { - let width = this.virtualizer!.visibleRect.width - this.padding - x; + let widthProperty = this.orientation === 'horizontal' ? 'height' : 'width'; + let heightProperty = this.orientation === 'horizontal' ? 'width' : 'height'; + + let width = this.virtualizer!.visibleRect[widthProperty] - this.padding - (this.orientation === 'horizontal' ? y : x); let rectHeight = this.rowHeight; let isEstimated = false; @@ -458,10 +455,10 @@ export class ListLayout exte // or the content of the item changed. let previousLayoutNode = this.layoutNodes.get(node.key); if (previousLayoutNode) { - rectHeight = previousLayoutNode.layoutInfo.rect.height; - isEstimated = width !== previousLayoutNode.layoutInfo.rect.width || node !== previousLayoutNode.node || previousLayoutNode.layoutInfo.estimatedSize; + rectHeight = previousLayoutNode.layoutInfo.rect[heightProperty]; + isEstimated = width !== previousLayoutNode.layoutInfo.rect[widthProperty] || node !== previousLayoutNode.node || previousLayoutNode.layoutInfo.estimatedSize; } else { - rectHeight = this.estimatedRowHeight; + rectHeight = node.type === 'item' || node.rendered ? this.estimatedRowHeight : 0; isEstimated = true; } } @@ -470,13 +467,13 @@ export class ListLayout exte rectHeight = DEFAULT_HEIGHT; } - let rect = new Rect(x, y, width, rectHeight); + let rect = this.orientation === 'horizontal' ? new Rect(x, y, rectHeight, width) : new Rect(x, y, width, rectHeight); let layoutInfo = new LayoutInfo(node.type, node.key, rect); layoutInfo.estimatedSize = isEstimated; return { layoutInfo, children: [], - validRect: layoutInfo.rect, + validRect: layoutInfo.rect.intersection(this.requestedRect), node }; } @@ -490,19 +487,21 @@ export class ListLayout exte let collection = this.virtualizer!.collection; let layoutInfo = layoutNode.layoutInfo; + let offsetProperty = this.orientation === 'horizontal' ? 'x' : 'y'; + let heightProperty = this.orientation === 'horizontal' ? 'width' : 'height'; layoutInfo.estimatedSize = false; - if (layoutInfo.rect.height !== size.height) { + if (layoutInfo.rect[heightProperty] !== size[heightProperty]) { // Copy layout info rather than mutating so that later caches are invalidated. let newLayoutInfo = layoutInfo.copy(); - newLayoutInfo.rect.height = size.height; + newLayoutInfo.rect[heightProperty] = size[heightProperty]; layoutNode.layoutInfo = newLayoutInfo; // Items after this layoutInfo will need to be repositioned to account for the new height. // Adjust the validRect so that only items above remain valid. - this.validRect.height = Math.min(this.validRect.height, layoutInfo.rect.y - this.validRect.y); + this.validRect[heightProperty] = Math.min(this.validRect[heightProperty], layoutInfo.rect[offsetProperty] - this.validRect[offsetProperty]); // The requestedRect also needs to be adjusted to account for the height difference. - this.requestedRect.height += newLayoutInfo.rect.height - layoutInfo.rect.height; + this.requestedRect[heightProperty] += newLayoutInfo.rect[heightProperty] - layoutInfo.rect[heightProperty]; // Invalidate layout for this layout node and all parents this.updateLayoutNode(key, layoutInfo, newLayoutInfo); @@ -598,7 +597,9 @@ export class ListLayout exte let layoutInfo = this.getLayoutInfo(target.key)!; let rect: Rect; if (target.dropPosition === 'before') { - rect = new Rect(layoutInfo.rect.x, Math.max(0, layoutInfo.rect.y - this.dropIndicatorThickness / 2), layoutInfo.rect.width, this.dropIndicatorThickness); + rect = this.orientation === 'horizontal' ? + new Rect(Math.max(0, layoutInfo.rect.x - this.dropIndicatorThickness / 2), layoutInfo.rect.y, this.dropIndicatorThickness, layoutInfo.rect.height) + : new Rect(layoutInfo.rect.x, Math.max(0, layoutInfo.rect.y - this.dropIndicatorThickness / 2), layoutInfo.rect.width, this.dropIndicatorThickness); } else if (target.dropPosition === 'after') { // Render after last visible descendant of the drop target. let targetNode = this.collection.getItem(target.key); @@ -616,7 +617,9 @@ export class ListLayout exte currentKey = this.collection.getKeyAfter(currentKey); } } - rect = new Rect(layoutInfo.rect.x, layoutInfo.rect.maxY - this.dropIndicatorThickness / 2, layoutInfo.rect.width, this.dropIndicatorThickness); + rect = this.orientation === 'horizontal' ? + new Rect(layoutInfo.rect.maxX - this.dropIndicatorThickness / 2, layoutInfo.rect.y, this.dropIndicatorThickness, layoutInfo.rect.height) + : new Rect(layoutInfo.rect.x, layoutInfo.rect.maxY - this.dropIndicatorThickness / 2, layoutInfo.rect.width, this.dropIndicatorThickness); } else { rect = layoutInfo.rect; } diff --git a/packages/@react-stately/virtualizer/src/OverscanManager.ts b/packages/@react-stately/virtualizer/src/OverscanManager.ts index 9794da1592d..cc25053ba25 100644 --- a/packages/@react-stately/virtualizer/src/OverscanManager.ts +++ b/packages/@react-stately/virtualizer/src/OverscanManager.ts @@ -43,12 +43,10 @@ export class OverscanManager { overscanned.y -= overscanY; } - if (this.velocity.x !== 0) { - let overscanX = this.visibleRect.width / 3; - overscanned.width += overscanX; - if (this.velocity.x < 0) { - overscanned.x -= overscanX; - } + let overscanX = this.visibleRect.width / 3; + overscanned.width += overscanX; + if (this.velocity.x < 0) { + overscanned.x -= overscanX; } return overscanned; diff --git a/packages/react-aria-components/src/GridList.tsx b/packages/react-aria-components/src/GridList.tsx index 15af9e858c7..6e0bc341d96 100644 --- a/packages/react-aria-components/src/GridList.tsx +++ b/packages/react-aria-components/src/GridList.tsx @@ -33,7 +33,7 @@ import {DragAndDropContext, DropIndicatorContext, DropIndicatorProps, useDndPers import {DragAndDropHooks} from './useDragAndDrop'; import {DraggableCollectionState, DroppableCollectionState, Collection as ICollection, ListState, Node, SelectionBehavior, UNSTABLE_useFilteredListState, useListState} from 'react-stately'; import {filterDOMProps, inertValue, LoadMoreSentinelProps, useLoadMoreSentinel, useObjectRef} from '@react-aria/utils'; -import {forwardRefType, GlobalDOMAttributes, HoverEvents, Key, LinkDOMProps, PressEvents, RefObject} from '@react-types/shared'; +import {forwardRefType, GlobalDOMAttributes, HoverEvents, Key, LinkDOMProps, Orientation, PressEvents, RefObject} from '@react-types/shared'; import {ListStateContext} from './ListBox'; import React, {createContext, ForwardedRef, forwardRef, HTMLAttributes, JSX, ReactNode, useContext, useEffect, useMemo, useRef} from 'react'; import {SelectionIndicatorContext} from './SelectionIndicator'; @@ -66,6 +66,11 @@ export interface GridListRenderProps { * @selector [data-layout="stack | grid"] */ layout: 'stack' | 'grid', + /** + * The primary orientation of the items. + * @selector [data-orientation="vertical | horizontal"] + */ + orientation: Orientation, /** * State of the grid list. */ @@ -101,7 +106,7 @@ export interface GridListProps extends Omit, 'children'> * The primary orientation of the items. Usually this is the direction that the collection scrolls. * @default 'vertical' */ - orientation?: 'horizontal' | 'vertical' + orientation?: Orientation } @@ -213,14 +218,6 @@ function GridListInner({props, collection, gridListRef: ref}: selectionManager }); - let keyboardDelegate = new ListKeyboardDelegate({ - collection: filteredState.collection, - disabledKeys: selectionManager.disabledKeys, - disabledBehavior: selectionManager.disabledBehavior, - ref, - orientation, - direction - }); let dropTargetDelegate = dragAndDropHooks.dropTargetDelegate || ctxDropTargetDelegate || new dragAndDropHooks.ListDropTargetDelegate(collection, ref, {layout, direction, orientation}); droppableCollection = dragAndDropHooks.useDroppableCollection!({ keyboardDelegate, @@ -234,6 +231,7 @@ function GridListInner({props, collection, gridListRef: ref}: let isEmpty = filteredState.collection.size === 0; let renderValues = { isDropTarget: isRootDropTarget, + orientation, isEmpty, isFocused, isFocusVisible, @@ -274,7 +272,8 @@ function GridListInner({props, collection, gridListRef: ref}: data-empty={isEmpty || undefined} data-focused={isFocused || undefined} data-focus-visible={isFocusVisible || undefined} - data-layout={layout}> + data-layout={layout} + data-orientation={orientation}> ({state: inputState, props, listBoxRef}: isFocused, isFocusVisible, layout: props.layout || 'stack', + orientation, state }; let renderProps = useRenderProps({ @@ -266,7 +272,7 @@ function ListBoxInner({state: inputState, props, listBoxRef}: data-focused={isFocused || undefined} data-focus-visible={isFocusVisible || undefined} data-layout={props.layout || 'stack'} - data-orientation={props.orientation || 'vertical'}> + data-orientation={orientation}> - + {section => ( -
{section.name}
+ {section.name} {item => {item.name}} @@ -384,6 +388,12 @@ export const VirtualizedListBox: StoryObj = { args: { variableHeight: false, isLoading: false + }, + argTypes: { + orientation: { + control: 'radio', + options: ['vertical', 'horizontal'] + } } }; @@ -402,7 +412,8 @@ export let VirtualizedListBoxEmpty: ListBoxStoryObj = { ) }; -export let VirtualizedListBoxDnd: ListBoxStory = () => { +function VirtualizedListBoxDndRender(args): JSX.Element { + let {orientation} = args; let items: {id: number, name: string}[] = []; for (let i = 0; i < 10000; i++) { items.push({id: i, name: `Item ${i}`}); @@ -433,10 +444,12 @@ export let VirtualizedListBoxDnd: ListBoxStory = () => { { ); }; +export const VirtualizedListBoxDnd: StoryObj = { + render: (args) => , + args: { + orientation: 'vertical' + }, + argTypes: { + orientation: { + control: 'radio', + options: ['vertical', 'horizontal'] + } + } +}; + function VirtualizedListBoxGridExample({minSize = 80, maxSize = 100, preserveAspectRatio = false}: {minSize: number, maxSize: number, preserveAspectRatio: boolean}): JSX.Element { let items: {id: number, name: string}[] = []; for (let i = 0; i < 10000; i++) { @@ -749,11 +775,11 @@ export const ListBoxScrollMargin: ListBoxStory = (args) => { items.push({id: i, name: `Item ${i}`, description: `Description ${i}`}); } return ( - {item => ( @@ -771,12 +797,12 @@ export const ListBoxSmoothScroll: ListBoxStory = (args) => { items.push({id: i, name: `Item ${i}`}); } return ( - {item => {item.name}} diff --git a/packages/react-aria-components/stories/utils.tsx b/packages/react-aria-components/stories/utils.tsx index d5a7439afa9..86dfc3db21a 100644 --- a/packages/react-aria-components/stories/utils.tsx +++ b/packages/react-aria-components/stories/utils.tsx @@ -1,13 +1,17 @@ import {classNames} from '@react-spectrum/utils'; -import {ListBoxItem, ListBoxItemProps, MenuItem, MenuItemProps, ProgressBar} from 'react-aria-components'; -import React, {JSX} from 'react'; +import {Header, ListBoxItem, ListBoxItemProps, MenuItem, MenuItemProps, ProgressBar} from 'react-aria-components'; +import React, {HTMLAttributes, JSX} from 'react'; import styles from '../example/index.css'; -export const MyListBoxItem = (props: ListBoxItemProps): JSX.Element => { +export const MyHeader = (props: HTMLAttributes) => { + return
; +}; + +export const MyListBoxItem = (props: ListBoxItemProps) => { return ( classNames(styles, 'item', { focused: isFocused, selected: isSelected, From f187e3b33c45d347871a252b546355d69f1368a9 Mon Sep 17 00:00:00 2001 From: Will Stone <654103+will-stone@users.noreply.github.com> Date: Tue, 17 Mar 2026 20:40:01 +0000 Subject: [PATCH 07/13] feat: add commitBehavior to NumberField (#9679) * feat: add isValueSnappingDisabled to NumberField * Add more info to the test and change prop name * Implement native validation * rename story --------- Co-authored-by: Robert Snow Co-authored-by: Robert Snow Co-authored-by: Devon Govett Co-authored-by: Daniel Lu --- .../numberfield/src/useNumberField.ts | 69 +++++++++++++++- .../stories/NumberField.stories.tsx | 6 ++ .../numberfield/test/NumberField.test.js | 70 ++++++++++++++++ .../numberfield/src/useNumberFieldState.ts | 33 +++----- .../@react-types/numberfield/src/index.d.ts | 9 +- .../s2-docs/pages/react-aria/NumberField.mdx | 2 +- packages/dev/s2-docs/pages/s2/NumberField.mdx | 2 +- .../stories/NumberField.stories.tsx | 6 +- .../test/NumberField.test.js | 82 +++++++++++++++++++ 9 files changed, 253 insertions(+), 26 deletions(-) diff --git a/packages/@react-aria/numberfield/src/useNumberField.ts b/packages/@react-aria/numberfield/src/useNumberField.ts index 06e5d42e526..427017983c2 100644 --- a/packages/@react-aria/numberfield/src/useNumberField.ts +++ b/packages/@react-aria/numberfield/src/useNumberField.ts @@ -13,7 +13,7 @@ import {announce} from '@react-aria/live-announcer'; import {AriaButtonProps} from '@react-types/button'; import {AriaNumberFieldProps} from '@react-types/numberfield'; -import {chain, filterDOMProps, getActiveElement, getEventTarget, isAndroid, isIOS, isIPhone, mergeProps, useFormReset, useId} from '@react-aria/utils'; +import {chain, filterDOMProps, getActiveElement, getEventTarget, isAndroid, isIOS, isIPhone, mergeProps, useFormReset, useId, useLayoutEffect} from '@react-aria/utils'; import { type ClipboardEvent, type ClipboardEventHandler, @@ -262,6 +262,7 @@ export function useNumberField(props: AriaNumberFieldProps, state: NumberFieldSt }, state, inputRef); useFormReset(inputRef, state.defaultNumberValue, state.setNumberValue); + useNativeValidation(state, props.validationBehavior, props.commitBehavior, inputRef, state.minValue, state.maxValue, props.step, state.numberValue); let inputProps: InputHTMLAttributes = mergeProps( spinButtonProps, @@ -363,3 +364,69 @@ export function useNumberField(props: AriaNumberFieldProps, state: NumberFieldSt validationDetails }; } + +let numberInput: HTMLInputElement | null = null; + +function useNativeValidation( + state: NumberFieldState, + validationBehavior: 'native' | 'aria' | undefined, + commitBehavior: 'snap' | 'validate' | undefined, + inputRef: RefObject, + min: number | undefined, + max: number | undefined, + step: number | undefined, + value: number | undefined +) { + useLayoutEffect(() => { + let input = inputRef.current; + if (commitBehavior !== 'validate' || state.realtimeValidation.isInvalid || !input || input.disabled) { + return; + } + + // Create a native number input and use it to implement validation of min/max/step. + // This lets us get the native validation message provided by the browser instead of needing our own translations. + if (!numberInput && typeof document !== 'undefined') { + numberInput = document.createElement('input'); + numberInput.type = 'number'; + } + + if (!numberInput) { + // For TypeScript. + return; + } + + numberInput.min = min != null && !isNaN(min) ? String(min) : ''; + numberInput.max = max != null && !isNaN(max) ? String(max) : ''; + numberInput.step = step != null && !isNaN(step) ? String(step) : ''; + numberInput.value = value != null && !isNaN(value) ? String(value) : ''; + + // Merge validity with the visible text input (for other validations like required). + let valid = input.validity.valid && numberInput.validity.valid; + let validationMessage = input.validationMessage || numberInput.validationMessage; + let validity = { + isInvalid: !valid, + validationErrors: validationMessage ? [validationMessage] : [], + validationDetails: { + badInput: input.validity.badInput, + customError: input.validity.customError, + patternMismatch: input.validity.patternMismatch, + rangeOverflow: numberInput.validity.rangeOverflow, + rangeUnderflow: numberInput.validity.rangeUnderflow, + stepMismatch: numberInput.validity.stepMismatch, + tooLong: input.validity.tooLong, + tooShort: input.validity.tooShort, + typeMismatch: input.validity.typeMismatch, + valueMissing: input.validity.valueMissing, + valid + } + }; + + state.updateValidation(validity); + + // Block form submission if validation behavior is native. + // This won't overwrite any user-defined validation message because we checked realtimeValidation above. + if (validationBehavior === 'native' && !numberInput.validity.valid) { + input.setCustomValidity(numberInput.validationMessage); + } + }); +} diff --git a/packages/@react-spectrum/numberfield/stories/NumberField.stories.tsx b/packages/@react-spectrum/numberfield/stories/NumberField.stories.tsx index 0703e37be51..3531c828c51 100644 --- a/packages/@react-spectrum/numberfield/stories/NumberField.stories.tsx +++ b/packages/@react-spectrum/numberfield/stories/NumberField.stories.tsx @@ -207,6 +207,12 @@ Step3WithMin2Max21.story = { name: 'step = 3 with min = 2, max = 21' }; +export const InteractOutsideBehaviorNone: NumberFieldStory = () => render({step: 3, minValue: 2, maxValue: 21, commitBehavior: 'validate'}); + +InteractOutsideBehaviorNone.story = { + name: 'commitBehavior = validate' +}; + export const AutoFocus: NumberFieldStory = () => render({autoFocus: true}); AutoFocus.story = { diff --git a/packages/@react-spectrum/numberfield/test/NumberField.test.js b/packages/@react-spectrum/numberfield/test/NumberField.test.js index c3941a229a1..707e75ead51 100644 --- a/packages/@react-spectrum/numberfield/test/NumberField.test.js +++ b/packages/@react-spectrum/numberfield/test/NumberField.test.js @@ -335,6 +335,29 @@ describe('NumberField', function () { expect(container).not.toHaveAttribute('aria-invalid'); }); + it.each` + Name + ${'NumberField'} + `('$Name will allow typing of a number less than the min when value snapping is disabled', async () => { + let { + container, + textField + } = renderNumberField({onChange: onChangeSpy, minValue: 10, commitBehavior: 'validate'}); + + expect(container).not.toHaveAttribute('aria-invalid'); + + act(() => {textField.focus();}); + await user.clear(textField); + await user.keyboard('5'); + expect(onChangeSpy).toHaveBeenCalledTimes(0); + expect(textField).toHaveAttribute('value', '5'); + act(() => {textField.blur();}); + expect(onChangeSpy).toHaveBeenCalledTimes(1); + expect(onChangeSpy).toHaveBeenCalledWith(5); + expect(textField).toHaveAttribute('value', '5'); + expect(container).toHaveAttribute('aria-invalid', 'true'); + }); + it.each` Name ${'NumberField'} @@ -383,6 +406,38 @@ describe('NumberField', function () { expect(textField).toHaveAttribute('value', '1'); }); + it.each` + Name + ${'NumberField'} + `('$Name will allow typing of a number greater than the max when value snapping is disabled', async () => { + let { + container, + textField + } = renderNumberField({onChange: onChangeSpy, maxValue: 1, defaultValue: 0, commitBehavior: 'validate'}); + + expect(container).not.toHaveAttribute('aria-invalid'); + + act(() => {textField.focus();}); + await user.keyboard('2'); + expect(onChangeSpy).not.toHaveBeenCalled(); + act(() => {textField.blur();}); + expect(onChangeSpy).toHaveBeenCalled(); + expect(onChangeSpy).toHaveBeenCalledWith(2); + expect(textField).toHaveAttribute('value', '2'); + + expect(container).toHaveAttribute('aria-invalid', 'true'); + + onChangeSpy.mockReset(); + act(() => {textField.focus();}); + await user.keyboard('2'); + expect(onChangeSpy).not.toHaveBeenCalled(); + act(() => {textField.blur();}); + expect(onChangeSpy).toHaveBeenCalled(); + expect(onChangeSpy).toHaveBeenCalledWith(22); + expect(textField).toHaveAttribute('value', '22'); + expect(container).toHaveAttribute('aria-invalid', 'true'); + }); + it.each` Name ${'NumberField'} @@ -772,6 +827,21 @@ describe('NumberField', function () { expect(textField).toHaveAttribute('value', result); }); + it.each` + Name | value + ${'NumberField down positive'} | ${'6'} + ${'NumberField up positive'} | ${'8'} + ${'NumberField down negative'} | ${'-8'} + ${'NumberField up negative'} | ${'-6'} + `('$Name does not round to step on commit when value snapping is disabled', async ({value}) => { + let {textField} = renderNumberField({onChange: onChangeSpy, step: 5, commitBehavior: 'validate'}); + act(() => {textField.focus();}); + await user.keyboard(value); + act(() => {textField.blur();}); + expect(textField).toHaveAttribute('value', value); + expect(textField).toHaveAttribute('aria-invalid', 'true'); + }); + it.each` Name | value | result ${'NumberField down positive'} | ${'6'} | ${'5'} diff --git a/packages/@react-stately/numberfield/src/useNumberFieldState.ts b/packages/@react-stately/numberfield/src/useNumberFieldState.ts index ad71558b3c9..de227c9162b 100644 --- a/packages/@react-stately/numberfield/src/useNumberFieldState.ts +++ b/packages/@react-stately/numberfield/src/useNumberFieldState.ts @@ -90,27 +90,26 @@ export function useNumberFieldState( onChange, locale, isDisabled, - isReadOnly + isReadOnly, + commitBehavior = 'snap' } = props; if (value === null) { value = NaN; } - if (value !== undefined && !isNaN(value)) { - if (step !== undefined && !isNaN(step)) { - value = snapValueToStep(value, minValue, maxValue, step); - } else { - value = clamp(value, minValue, maxValue); - } + let snapValue = useCallback(value => { + return step === undefined || isNaN(step) + ? clamp(value, minValue, maxValue) + : snapValueToStep(value, minValue, maxValue, step); + }, [step, minValue, maxValue]); + + if (value !== undefined && !isNaN(value) && commitBehavior === 'snap') { + value = snapValue(value); } - if (!isNaN(defaultValue)) { - if (step !== undefined && !isNaN(step)) { - defaultValue = snapValueToStep(defaultValue, minValue, maxValue, step); - } else { - defaultValue = clamp(defaultValue, minValue, maxValue); - } + if (!isNaN(defaultValue) && commitBehavior === 'snap') { + defaultValue = snapValue(defaultValue); } let [numberValue, setNumberValue] = useControlledState(value, isNaN(defaultValue) ? NaN : defaultValue, onChange); @@ -167,13 +166,7 @@ export function useNumberFieldState( } // Clamp to min and max, round to the nearest step, and round to specified number of digits - let clampedValue: number; - if (step === undefined || isNaN(step)) { - clampedValue = clamp(newParsedValue, minValue, maxValue); - } else { - clampedValue = snapValueToStep(newParsedValue, minValue, maxValue, step); - } - + let clampedValue = commitBehavior === 'snap' ? snapValue(newParsedValue) : newParsedValue; clampedValue = numberParser.parse(format(clampedValue)); let shouldValidate = clampedValue !== numberValue; setNumberValue(clampedValue); diff --git a/packages/@react-types/numberfield/src/index.d.ts b/packages/@react-types/numberfield/src/index.d.ts index ab27493aa60..384faa5cbc1 100644 --- a/packages/@react-types/numberfield/src/index.d.ts +++ b/packages/@react-types/numberfield/src/index.d.ts @@ -29,7 +29,14 @@ export interface NumberFieldProps extends InputBase, Validation, Focusab * Formatting options for the value displayed in the number field. * This also affects what characters are allowed to be typed by the user. */ - formatOptions?: Intl.NumberFormatOptions + formatOptions?: Intl.NumberFormatOptions, + /** + * Controls the behavior of the number field when the user blurs the field after editing. + * 'snap' will clamp the value to the min/max values, and snap to the nearest step value. + * 'validate' will not clamp the value, and will validate that the value is within the min/max range and on a valid step. + * @default 'snap' + */ + commitBehavior?: 'snap' | 'validate' } export interface AriaNumberFieldProps extends NumberFieldProps, DOMProps, AriaLabelingProps, TextInputDOMEvents { diff --git a/packages/dev/s2-docs/pages/react-aria/NumberField.mdx b/packages/dev/s2-docs/pages/react-aria/NumberField.mdx index 2c5bc3ce0a6..d156a5664ac 100644 --- a/packages/dev/s2-docs/pages/react-aria/NumberField.mdx +++ b/packages/dev/s2-docs/pages/react-aria/NumberField.mdx @@ -87,7 +87,7 @@ Use the `minValue`, `maxValue`, and `step` props to set the allowed values. Step component={VanillaNumberField} docs={docs.exports.NumberField} links={docs.links} - props={['minValue', 'maxValue', 'step']} + props={['minValue', 'maxValue', 'step', 'commitBehavior']} initialProps={{ label: 'Amount', minValue: 0, diff --git a/packages/dev/s2-docs/pages/s2/NumberField.mdx b/packages/dev/s2-docs/pages/s2/NumberField.mdx index 50d5824dc5e..fe268091d77 100644 --- a/packages/dev/s2-docs/pages/s2/NumberField.mdx +++ b/packages/dev/s2-docs/pages/s2/NumberField.mdx @@ -80,7 +80,7 @@ Use the `minValue`, `maxValue`, and `step` props to set the allowed values. Step component={NumberField} docs={docs.exports.NumberField} links={docs.links} - props={['minValue', 'maxValue', 'step']} + props={['minValue', 'maxValue', 'step', 'commitBehavior']} initialProps={{ label: 'Amount', placeholder: 'Enter a number', diff --git a/packages/react-aria-components/stories/NumberField.stories.tsx b/packages/react-aria-components/stories/NumberField.stories.tsx index 9f7cf521fc5..1139db3641b 100644 --- a/packages/react-aria-components/stories/NumberField.stories.tsx +++ b/packages/react-aria-components/stories/NumberField.stories.tsx @@ -29,10 +29,12 @@ export const NumberFieldExample: NumberFieldStory = { maxValue: 100, step: 1, formatOptions: {style: 'currency', currency: 'USD'}, - isWheelDisabled: false + isWheelDisabled: false, + isRequired: false, + commitBehavior: 'snap' }, render: (args) => ( - (v & 1 ? 'Invalid value' : null)}> + diff --git a/packages/react-aria-components/test/NumberField.test.js b/packages/react-aria-components/test/NumberField.test.js index f72d76bacbd..1da1aafcc0a 100644 --- a/packages/react-aria-components/test/NumberField.test.js +++ b/packages/react-aria-components/test/NumberField.test.js @@ -406,4 +406,86 @@ describe('NumberField', () => { expect(input).toHaveAttribute('aria-describedby'); expect(document.getElementById(input.getAttribute('aria-describedby').split(' ')[0])).toHaveTextContent('This field has an error.'); }); + + it('should not change the edited input value when value snapping is disabled', async () => { + let {getByRole, getByTestId} = render( + + + + + + + + + + + + ); + let input = getByRole('textbox'); + expect(input.validity.valid).toBe(true); + + // Over max + await user.tab(); + await user.clear(input); + await user.keyboard('1024'); + await user.tab(); + expect(input).toHaveValue('1,024'); + expect(announce).toHaveBeenLastCalledWith('1,024', 'assertive'); + expect(input.closest('.react-aria-NumberField')).toHaveAttribute('data-invalid', 'true'); + expect(input).toHaveAttribute('aria-invalid', 'true'); + expect(input.validity.valid).toBe(false); + expect(input).toHaveAttribute('aria-describedby'); + expect(document.getElementById(input.getAttribute('aria-describedby'))).toHaveTextContent('Constraints not satisfied'); + + act(() => {getByTestId('form').checkValidity();}); + expect(document.activeElement).toBe(input); + + // Valid + await user.clear(input); + await user.keyboard('30'); + await user.tab(); + expect(input).toHaveValue('30'); + expect(announce).toHaveBeenLastCalledWith('30', 'assertive'); + expect(input.validity.valid).toBe(true); + expect(input).not.toHaveAttribute('aria-describedby'); + + // Under min + await user.clear(input); + await user.keyboard('2'); + await user.tab(); + expect(input).toHaveValue('2'); + expect(announce).toHaveBeenLastCalledWith('2', 'assertive'); + expect(input.validity.valid).toBe(false); + expect(input).toHaveAttribute('aria-describedby'); + + act(() => {getByTestId('form').checkValidity();}); + expect(document.activeElement).toBe(input); + + // Not on step + await user.clear(input); + await user.keyboard('31'); + await user.tab(); + expect(input).toHaveValue('31'); + expect(announce).toHaveBeenLastCalledWith('31', 'assertive'); + expect(input.validity.valid).toBe(false); + expect(input).toHaveAttribute('aria-describedby'); + + act(() => {getByTestId('form').checkValidity();}); + expect(document.activeElement).toBe(input); + + // Required + await user.clear(input); + await user.tab(); + expect(input).toHaveValue(''); + expect(input.validity.valid).toBe(false); + expect(input).toHaveAttribute('aria-describedby'); + + // Valid + await user.clear(input); + await user.keyboard('30'); + await user.tab(); + expect(input).toHaveValue('30'); + expect(input.validity.valid).toBe(true); + expect(input).not.toHaveAttribute('aria-describedby'); + }); }); From 2d3b48dd2df1e7ca4a8f278fc9190513a76260be Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 17 Mar 2026 15:44:08 -0500 Subject: [PATCH 08/13] chore(S2): audit style macro utilities (#9544) * export all utilities * export remaining utils * removed unsafe exports for now * add JSDoc descriptions * rename colorScheme() to setColorScheme() * remove raw/keyframes from s2 export * export WidthProperties and HeightProperties as types * cleanup JSDocs * add docs * lint * address review comments * extract docs from JSDoc * add imports to all examples * update styles->className in example * fix size in md output * add linearGradient * remove getAllowedOverrides from exports/docs (could have breaking changes) * remove WidthProperties/HeightProperties since we removed getAllowedOverrides export * address review comments * rename raw -> css --- .../@react-spectrum/s2/src/CenterBaseline.tsx | 4 +- packages/@react-spectrum/s2/src/CoachMark.tsx | 10 +- packages/@react-spectrum/s2/src/ComboBox.tsx | 4 +- .../@react-spectrum/s2/src/Disclosure.tsx | 4 +- packages/@react-spectrum/s2/src/Menu.tsx | 4 +- packages/@react-spectrum/s2/src/Modal.tsx | 5 +- packages/@react-spectrum/s2/src/Picker.tsx | 4 +- packages/@react-spectrum/s2/src/Popover.tsx | 6 +- packages/@react-spectrum/s2/src/Provider.tsx | 6 +- .../@react-spectrum/s2/src/SearchField.tsx | 4 +- packages/@react-spectrum/s2/src/Skeleton.tsx | 4 +- packages/@react-spectrum/s2/src/TableView.tsx | 10 +- .../@react-spectrum/s2/src/TabsPicker.tsx | 4 +- packages/@react-spectrum/s2/src/TextField.tsx | 4 +- packages/@react-spectrum/s2/src/Tooltip.tsx | 6 +- packages/@react-spectrum/s2/src/bar-utils.ts | 3 +- packages/@react-spectrum/s2/src/index.ts | 11 + .../@react-spectrum/s2/src/style-utils.ts | 42 ++- packages/@react-spectrum/s2/style/index.ts | 65 +++- packages/@react-spectrum/s2/style/runtime.ts | 16 + .../s2/style/spectrum-theme.ts | 86 +++++ .../@react-spectrum/s2/style/style-macro.ts | 34 +- .../dev/parcel-packager-docs/DocsPackager.js | 1 + .../DocsTransformer.js | 67 ++++ packages/dev/s2-docs/pages/s2/style-macro.mdx | 60 ++++ packages/dev/s2-docs/pages/s2/styling.mdx | 8 +- .../s2-docs/scripts/generateMarkdownDocs.mjs | 333 +++++++++++++++++- packages/dev/s2-docs/src/FunctionJSDoc.tsx | 69 ++++ packages/dev/s2-docs/src/Step.tsx | 4 +- 29 files changed, 803 insertions(+), 75 deletions(-) create mode 100644 packages/dev/s2-docs/src/FunctionJSDoc.tsx diff --git a/packages/@react-spectrum/s2/src/CenterBaseline.tsx b/packages/@react-spectrum/s2/src/CenterBaseline.tsx index 788083bc0ad..58e5fecc998 100644 --- a/packages/@react-spectrum/s2/src/CenterBaseline.tsx +++ b/packages/@react-spectrum/s2/src/CenterBaseline.tsx @@ -10,9 +10,9 @@ * governing permissions and limitations under the License. */ +import {css} from '../style/style-macro' with {type: 'macro'}; import {CSSProperties, ReactNode} from 'react'; import {mergeStyles} from '../style/runtime'; -import {raw} from '../style/style-macro' with {type: 'macro'}; import {style} from '../style' with {type: 'macro'}; import {StyleString} from '../style/types'; @@ -39,7 +39,7 @@ export function CenterBaseline(props: CenterBaselineProps): ReactNode { ); } -export const centerBaselineBefore = raw('&::before { content: "\u00a0"; width: 0; visibility: hidden }'); +export const centerBaselineBefore = css('&::before { content: "\u00a0"; width: 0; visibility: hidden }'); export function centerBaseline(props: Omit = {}): (icon: ReactNode) => ReactNode { return (icon: ReactNode) => {icon}; diff --git a/packages/@react-spectrum/s2/src/CoachMark.tsx b/packages/@react-spectrum/s2/src/CoachMark.tsx index d84dde75282..caed4fcaac6 100644 --- a/packages/@react-spectrum/s2/src/CoachMark.tsx +++ b/packages/@react-spectrum/s2/src/CoachMark.tsx @@ -27,7 +27,6 @@ import { import {ButtonContext} from './Button'; import {Card} from './Card'; import {CheckboxContext} from './Checkbox'; -import {colorScheme, getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; import {ColorSchemeContext} from './Provider'; import {ContentContext, FooterContext, KeyboardContext, TextContext} from './Content'; import { @@ -38,16 +37,17 @@ import { useContext, useRef } from 'react'; +import {css, keyframes} from '../style/style-macro' with {type: 'macro'}; import {DividerContext} from './Divider'; import {forwardRefType} from './types'; +import {getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; import {GlobalDOMAttributes} from '@react-types/shared'; import {ImageContext} from './Image'; import {ImageCoordinator} from './ImageCoordinator'; -import {keyframes, raw} from '../style/style-macro' with {type: 'macro'}; import {mergeStyles} from '../style/runtime'; import {PressResponder} from '@react-aria/interactions'; +import {setColorScheme, space, style} from '../style' with {type: 'macro'}; import {SliderContext} from './Slider'; -import {space, style} from '../style' with {type: 'macro'}; import {useId, useObjectRef, useOverlayTrigger} from 'react-aria'; import {useLayoutEffect} from '@react-aria/utils'; import {useMenuTriggerState} from 'react-stately'; @@ -106,7 +106,7 @@ const slideLeftKeyframes = keyframes(` `); let popover = style({ - ...colorScheme(), + ...setColorScheme(), '--s2-container-bg': { type: 'backgroundColor', value: 'layer-2' @@ -474,7 +474,7 @@ const indicator = style({ } }); -const pulse = raw(`&:before { content: ""; display: inline-block; position: absolute; top: var(--borderOffset); bottom: var(--borderOffset); left: var(--borderOffset); right: var(--borderOffset); border-radius: var(--ringRadius); outline-style: solid; outline-color: var(--activeElement); outline-width: 4px; animation-duration: 2s; animation-iteration-count: infinite; animation-timing-function: ease-in-out; animation-fill-mode: forwards; animation-name: ${pulseAnimation}}`); +const pulse = css(`&:before { content: ""; display: inline-block; position: absolute; top: var(--borderOffset); bottom: var(--borderOffset); left: var(--borderOffset); right: var(--borderOffset); border-radius: var(--ringRadius); outline-style: solid; outline-color: var(--activeElement); outline-width: 4px; animation-duration: 2s; animation-iteration-count: infinite; animation-timing-function: ease-in-out; animation-fill-mode: forwards; animation-name: ${pulseAnimation}}`); interface CoachMarkIndicatorProps { children: ReactNode, diff --git a/packages/@react-spectrum/s2/src/ComboBox.tsx b/packages/@react-spectrum/s2/src/ComboBox.tsx index 87509f9c5d9..f51a77d5cb4 100644 --- a/packages/@react-spectrum/s2/src/ComboBox.tsx +++ b/packages/@react-spectrum/s2/src/ComboBox.tsx @@ -36,9 +36,8 @@ import { import {AsyncLoadable, GlobalDOMAttributes, HelpTextProps, LoadingState, SingleSelection, SpectrumLabelableProps} from '@react-types/shared'; import {AvatarContext} from './Avatar'; import {BaseCollection, CollectionNode, createLeafComponent} from '@react-aria/collections'; -import {baseColor, focusRing, space, style} from '../style' with {type: 'macro'}; +import {baseColor, centerPadding, focusRing, space, style} from '../style' with {type: 'macro'}; import {centerBaseline} from './CenterBaseline'; -import {centerPadding, control, controlBorderRadius, controlFont, controlSize, field, fieldInput, getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; import { checkmark, description, @@ -49,6 +48,7 @@ import { } from './Menu'; import CheckmarkIcon from '../ui-icons/Checkmark'; import ChevronIcon from '../ui-icons/Chevron'; +import {control, controlBorderRadius, controlFont, controlSize, field, fieldInput, getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; import {createContext, CSSProperties, ForwardedRef, forwardRef, ReactNode, Ref, useCallback, useContext, useEffect, useImperativeHandle, useMemo, useRef, useState} from 'react'; import {createFocusableRef} from '@react-spectrum/utils'; import {edgeToText} from '../style/spectrum-theme' with {type: 'macro'}; diff --git a/packages/@react-spectrum/s2/src/Disclosure.tsx b/packages/@react-spectrum/s2/src/Disclosure.tsx index 512cd1f4412..c1cdd576c78 100644 --- a/packages/@react-spectrum/s2/src/Disclosure.tsx +++ b/packages/@react-spectrum/s2/src/Disclosure.tsx @@ -12,12 +12,12 @@ import {ActionButtonContext} from './ActionButton'; import {AriaLabelingProps, DOMProps, DOMRef, DOMRefValue, forwardRefType, GlobalDOMAttributes} from '@react-types/shared'; -import {baseColor, focusRing, lightDark, space, style} from '../style' with { type: 'macro' }; +import {baseColor, centerPadding, focusRing, lightDark, space, style} from '../style' with { type: 'macro' }; import {Button, ContextValue, DisclosureStateContext, Heading, Provider, Disclosure as RACDisclosure, DisclosurePanel as RACDisclosurePanel, DisclosurePanelProps as RACDisclosurePanelProps, DisclosureProps as RACDisclosureProps, useLocale, useSlottedContext} from 'react-aria-components'; import {CenterBaseline} from './CenterBaseline'; -import {centerPadding, getAllowedOverrides, StyleProps, UnsafeStyles} from './style-utils' with { type: 'macro' }; import Chevron from '../ui-icons/Chevron'; import {filterDOMProps} from '@react-aria/utils'; +import {getAllowedOverrides, StyleProps, UnsafeStyles} from './style-utils' with {type: 'macro'}; import React, {createContext, forwardRef, ReactNode, useContext} from 'react'; import {useDOMRef} from '@react-spectrum/utils'; import {useSpectrumContextProps} from './useSpectrumContextProps'; diff --git a/packages/@react-spectrum/s2/src/Menu.tsx b/packages/@react-spectrum/s2/src/Menu.tsx index ea1731cffc9..d2dd826bf30 100644 --- a/packages/@react-spectrum/s2/src/Menu.tsx +++ b/packages/@react-spectrum/s2/src/Menu.tsx @@ -28,12 +28,12 @@ import { Separator, SeparatorProps } from 'react-aria-components'; -import {baseColor, focusRing, fontRelative, size, space, style} from '../style' with {type: 'macro'}; +import {baseColor, centerPadding, focusRing, fontRelative, size, space, style} from '../style' with {type: 'macro'}; import {box, iconStyles} from './Checkbox'; import {centerBaseline} from './CenterBaseline'; -import {centerPadding, control, controlFont, controlSize, getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; import CheckmarkIcon from '../ui-icons/Checkmark'; import ChevronRightIcon from '../ui-icons/Chevron'; +import {control, controlFont, controlSize, getAllowedOverrides, StyleProps} from './style-utils' with {type: 'macro'}; import {createContext, forwardRef, JSX, ReactElement, ReactNode, useContext, useRef, useState} from 'react'; import {divider} from './Divider'; import {DOMRef, DOMRefValue, GlobalDOMAttributes, PressEvent} from '@react-types/shared'; diff --git a/packages/@react-spectrum/s2/src/Modal.tsx b/packages/@react-spectrum/s2/src/Modal.tsx index a7927be2941..c6931837893 100644 --- a/packages/@react-spectrum/s2/src/Modal.tsx +++ b/packages/@react-spectrum/s2/src/Modal.tsx @@ -10,12 +10,11 @@ * governing permissions and limitations under the License. */ -import {colorScheme} from './style-utils' with {type: 'macro'}; import {ColorSchemeContext} from './Provider'; import {DOMRef, GlobalDOMAttributes} from '@react-types/shared'; import {forwardRef, MutableRefObject, useCallback, useContext} from 'react'; import {ModalOverlay, ModalOverlayProps, Modal as RACModal, useLocale} from 'react-aria-components'; -import {style} from '../style' with {type: 'macro'}; +import {setColorScheme, style} from '../style' with {type: 'macro'}; import {useDOMRef} from '@react-spectrum/utils'; interface ModalProps extends Omit { @@ -28,7 +27,7 @@ interface ModalProps extends Omit :not([slot=icon], [slot=avatar], [slot=label], [data-slot=label]) {display: none;}')) + (renderValue ? '' : ' ' + css('&> :not([slot=icon], [slot=avatar], [slot=label], [data-slot=label]) {display: none;}')) }> {({selectedItems, defaultChildren}) => { const selectedValues = selectedItems.filter((item): item is T => item != null); diff --git a/packages/@react-spectrum/s2/src/Popover.tsx b/packages/@react-spectrum/s2/src/Popover.tsx index 407598963ee..a4f85134e49 100644 --- a/packages/@react-spectrum/s2/src/Popover.tsx +++ b/packages/@react-spectrum/s2/src/Popover.tsx @@ -20,11 +20,11 @@ import { OverlayTriggerStateContext, useLocale } from 'react-aria-components'; -import {colorScheme, getAllowedOverrides, heightProperties, UnsafeStyles, widthProperties} from './style-utils' with {type: 'macro'}; import {ColorSchemeContext} from './Provider'; import {createContext, ForwardedRef, forwardRef, ReactNode, useCallback, useContext, useMemo} from 'react'; import {DOMRef, DOMRefValue, GlobalDOMAttributes} from '@react-types/shared'; -import {lightDark, style} from '../style' with {type: 'macro'}; +import {getAllowedOverrides, heightProperties, UnsafeStyles, widthProperties} from './style-utils' with {type: 'macro'}; +import {lightDark, setColorScheme, style} from '../style' with {type: 'macro'}; import {mergeRefs} from '@react-aria/utils'; import {mergeStyles} from '../style/runtime'; import {StyleString} from '../style/types' with {type: 'macro'}; @@ -62,7 +62,7 @@ export interface PopoverProps extends UnsafeStyles, Omit - + {!isEmpty && !searchFieldProps.isReadOnly && } row({ ...renderProps, ...tableVisualOptions - }) + (renderProps.isFocusVisible ? ' ' + raw('&:before { content: ""; display: inline-block; position: sticky; inset-inline-start: 0; width: 3px; height: 100%; margin-inline-end: -3px; margin-block-end: 1px; z-index: 3; background-color: var(--rowFocusIndicatorColor)') : '')} + }) + (renderProps.isFocusVisible ? ' ' + css('&:before { content: ""; display: inline-block; position: sticky; inset-inline-start: 0; width: 3px; height: 100%; margin-inline-end: -3px; margin-block-end: 1px; z-index: 3; background-color: var(--rowFocusIndicatorColor)') : '')} {...otherProps}> {selectionMode !== 'none' && selectionBehavior === 'toggle' && ( // Not sure what we want to do with this className, in Cell it currently overrides the className that would have been applied. diff --git a/packages/@react-spectrum/s2/src/TabsPicker.tsx b/packages/@react-spectrum/s2/src/TabsPicker.tsx index 47d75e6942b..16019c1f528 100644 --- a/packages/@react-spectrum/s2/src/TabsPicker.tsx +++ b/packages/@react-spectrum/s2/src/TabsPicker.tsx @@ -37,6 +37,7 @@ import { import CheckmarkIcon from '../ui-icons/Checkmark'; import ChevronIcon from '../ui-icons/Chevron'; import {controlFont, fieldInput, StyleProps} from './style-utils' with {type: 'macro'}; +import {css} from '../style/style-macro' with {type: 'macro'}; import {edgeToText} from '../style/spectrum-theme' with {type: 'macro'}; import { FieldLabel @@ -48,7 +49,6 @@ import {IconContext} from './Icon'; import {Placement, useLocale} from 'react-aria'; import {Popover} from './Popover'; import {pressScale} from './pressScale'; -import {raw} from '../style/style-macro' with {type: 'macro'}; import React, {createContext, forwardRef, ReactNode, useContext, useRef} from 'react'; import {useFocusableRef} from '@react-spectrum/utils'; import {useFormProps} from './Form'; @@ -204,7 +204,7 @@ function Picker(props: PickerProps, ref: FocusableRef - * {display: none;}')}> + * {display: none;}')}> {({defaultChildren}) => { return ( , Pick { @@ -44,7 +44,7 @@ export interface TooltipProps extends Omit({ - ...colorScheme(), + ...setColorScheme(), justifyContent: 'center', alignItems: 'center', maxWidth: 160, diff --git a/packages/@react-spectrum/s2/src/bar-utils.ts b/packages/@react-spectrum/s2/src/bar-utils.ts index 0b28be2bb50..096f3834727 100644 --- a/packages/@react-spectrum/s2/src/bar-utils.ts +++ b/packages/@react-spectrum/s2/src/bar-utils.ts @@ -10,7 +10,8 @@ * governing permissions and limitations under the License. */ -import {centerPadding, controlSize, fieldInput, staticColor} from './style-utils' with {type: 'macro'}; +import {centerPadding} from '../style' with {type: 'macro'}; +import {controlSize, fieldInput, staticColor} from './style-utils' with {type: 'macro'}; export const bar = () => ({ ...staticColor(), diff --git a/packages/@react-spectrum/s2/src/index.ts b/packages/@react-spectrum/s2/src/index.ts index 069647cbb85..6d84ca9b73f 100644 --- a/packages/@react-spectrum/s2/src/index.ts +++ b/packages/@react-spectrum/s2/src/index.ts @@ -92,6 +92,8 @@ export {TreeView, TreeViewItem, TreeViewItemContent, TreeViewLoadMoreItem} from export {pressScale} from './pressScale'; +export {mergeStyles} from '../style/runtime'; + export {Autocomplete, Collection, FileTrigger, parseColor, useLocale} from 'react-aria-components'; export {useListData, useTreeData, useAsyncList} from 'react-stately'; @@ -171,3 +173,12 @@ export type {TooltipProps} from './Tooltip'; export type {TreeViewProps, TreeViewItemProps, TreeViewItemContentProps, TreeViewLoadMoreItemProps} from './TreeView'; export type {AutocompleteProps, FileTriggerProps, TooltipTriggerComponentProps as TooltipTriggerProps, SortDescriptor, Color, Key, Selection, RouterConfig} from 'react-aria-components'; export type {ListData, TreeData, AsyncListData} from 'react-stately'; + +export type { + StylesProp, + StylesPropWithHeight, + StylesPropWithoutWidth, + UnsafeClassName, + UnsafeStyles, + StyleProps +} from './style-utils'; diff --git a/packages/@react-spectrum/s2/src/style-utils.ts b/packages/@react-spectrum/s2/src/style-utils.ts index 3c743afee54..463f2b6520b 100644 --- a/packages/@react-spectrum/s2/src/style-utils.ts +++ b/packages/@react-spectrum/s2/src/style-utils.ts @@ -11,13 +11,35 @@ */ import {CSSProperties} from 'react'; -import {fontRelative} from '../style'; +import {fontRelative as internalFontRelative} from '../style/spectrum-theme'; import {StyleString} from '../style/types'; +/** + * Calculates vertical padding to center a single line of text within a container. + * Uses the CSS `self()` function and `1lh` unit to compute the padding based on + * the container's minimum height and border widths. + * This is useful for precise vertical centering without introducing a flex/grid layout to the container. + * + * @param minHeight - A CSS expression for the minimum height to center within. Defaults to `'self(minHeight)'`. + * @returns A CSS `calc()` expression wrapped as an arbitrary style value. + * + * @example + * ```tsx + * import {centerPadding, style} from '@react-spectrum/s2/style' with {type: 'macro'}; + * + * const styles = style({ + * paddingY: centerPadding() + * }); + * ``` + */ export function centerPadding(minHeight: string = 'self(minHeight)'): `[${string}]` { return `[calc((${minHeight} - self(borderTopWidth, 0px) - self(borderBottomWidth, 0px) - 1lh) / 2)]`; } +function fontRelative(base: number, baseFontSize = 14): `[${string}]` { + return `[${internalFontRelative(base, baseFontSize)}]`; +} + export const field = () => ({ display: 'grid', gridColumnStart: { @@ -113,7 +135,23 @@ export const fieldInput = () => ({ containIntrinsicWidth: 'calc(var(--defaultWidth) - self(paddingStart, 0px) - self(paddingEnd, 0px) - self(borderStartWidth, 0px) - self(borderEndWidth, 0px))' } as const); -export const colorScheme = () => ({ +/** + * Returns style properties that set the CSS `color-scheme` for a component. + * Defaults to the page's color scheme and supports `'light'`, `'dark'`, and `'light dark'` values + * via the `colorScheme` render prop condition. + * Intended for root containers (e.g. providers, modals, and popovers), and not needed for individual components. + * + * @example + * ```tsx + * import {setColorScheme, style} from '@react-spectrum/s2/style' with {type: 'macro'}; + * + * const styles = style({ + * ...setColorScheme(), + * backgroundColor: 'layer-1' + * }); + * ``` + */ +export const setColorScheme = () => ({ colorScheme: { // Default to page color scheme if none is defined. default: '[var(--lightningcss-light, light) var(--lightningcss-dark, dark)]', diff --git a/packages/@react-spectrum/s2/style/index.ts b/packages/@react-spectrum/s2/style/index.ts index 7a847469bd7..76f145d762b 100644 --- a/packages/@react-spectrum/s2/style/index.ts +++ b/packages/@react-spectrum/s2/style/index.ts @@ -16,17 +16,63 @@ import type {MacroContext} from '@parcel/macros'; import {StyleString} from './types'; export {baseColor, color, lightDark, colorMix, size, style} from './spectrum-theme'; +export {css} from './style-macro'; +export {centerPadding, setColorScheme} from '../src/style-utils'; export type {StyleString} from './types'; -// Wrap these functions in arbitrary value syntax when called from the outside. +/** + * Converts a pixel value to a Spectrum spacing token in `rem` units. + * + * @param px - The spacing in pixels. + * @returns A `rem` value wrapped as an arbitrary style value. + * + * @example + * ```tsx + * import {space} from '@react-spectrum/s2/style' with {type: 'macro'}; + * + * const styles = style({ + * gap: space(12) // 12/16 = 0.75rem + * }); + * ``` + */ export function space(px: number): `[${string}]` { return `[${internalSpace(px)}]`; } -export function fontRelative(base: number, baseFontSize?: number): `[${string}]` { +/** + * Converts a pixel value to a font-relative `em` length. Useful for sizing elements + * relative to the current font size. Defaults to a 14px base. + * + * @param base - The pixel value to convert. + * @param baseFontSize - The base font size in pixels to divide by. Defaults to `14`. + * @returns A CSS `em` value wrapped as an arbitrary style value. + * + * @example + * ```tsx + * import {fontRelative} from '@react-spectrum/s2/style' with {type: 'macro'}; + * + * const styles = style({ + * gap: fontRelative(2) // 2/14 = ~0.143em + * }); + * ``` + */ +export function fontRelative(base: number, baseFontSize = 14): `[${string}]` { return `[${internalFontRelative(base, baseFontSize)}]`; } +/** + * Returns consistent Spectrum focus ring outline styles for interactive components. + * + * @example + * ```tsx + * import {focusRing, style} from '@react-spectrum/s2/style' with {type: 'macro'}; + * + * const styles = style({ + * ...focusRing(), + * borderRadius: 'lg' + * }); + * ``` + */ export const focusRing = () => ({ outlineStyle: { default: 'none', @@ -78,6 +124,21 @@ const iconSizes = { XL: 26 } as const; +/** + * Generates styles for an icon element with the given size, color, and layout options. + * Must be imported with `{type: 'macro'}`. + * + * @param options - Icon styling options including `size` (XS–XL), `color`, and layout properties. + * @returns A `StyleString` that can be applied to an icon element. + * + * @example + * ```tsx + * import {iconStyle} from '@react-spectrum/s2/style' with {type: 'macro'}; + * import Edit from '@react-spectrum/s2/icons/Edit'; + * + * + * ``` + */ export function iconStyle(this: MacroContext | void, options: IconStyle): StyleString> { let {size = 'M', color, ...styles} = options; diff --git a/packages/@react-spectrum/s2/style/runtime.ts b/packages/@react-spectrum/s2/style/runtime.ts index 370fcb7b675..752b5858728 100644 --- a/packages/@react-spectrum/s2/style/runtime.ts +++ b/packages/@react-spectrum/s2/style/runtime.ts @@ -36,6 +36,22 @@ import {StyleString} from './types'; // }; // } +/** + * Merges multiple style strings together, combining the CSS properties from each. + * Later styles take precedence over earlier ones for the same property. + * Useful for composing styles from multiple `style()` macro calls. + * + * @example + * ```tsx + * import {mergeStyles} from '@react-spectrum/s2'; + * import {style} from '@react-spectrum/s2/style' with {type: 'macro'}; + * + * const baseStyles = style({padding: 8}); + * const overrideStyles = style({padding: 16, color: 'heading'}); + * const merged = mergeStyles(baseStyles, overrideStyles); + * // merged has `padding: 16` and `color: heading`. + * ``` + */ export function mergeStyles(...styles: (StyleString | null | undefined)[]): StyleString { let definedStyles = styles.filter(Boolean) as StyleString[]; if (definedStyles.length === 1) { diff --git a/packages/@react-spectrum/s2/style/spectrum-theme.ts b/packages/@react-spectrum/s2/style/spectrum-theme.ts index b66fa550564..0cf5302c724 100644 --- a/packages/@react-spectrum/s2/style/spectrum-theme.ts +++ b/packages/@react-spectrum/s2/style/spectrum-theme.ts @@ -194,6 +194,22 @@ class SpectrumColorProperty extends ArbitraryProperty { type BaseColor = keyof typeof baseColors; +/** + * Returns a set of stateful color token references for the default, hovered, focus-visible, + * and pressed states of a component. + * + * @param base - A Spectrum base color token name (e.g. `'gray-100'`, `'accent-900'`). + * @returns An object with `default`, `isHovered`, `isFocusVisible`, and `isPressed` color token references. + * + * @example + * ```tsx + * import {baseColor, style} from '@react-spectrum/s2/style' with {type: 'macro'}; + * + * const styles = style({ + * backgroundColor: baseColor('gray-100') + * }); + * ``` + */ export function baseColor(base: BaseColor | C): {default: C, isHovered: C, isFocusVisible: C, isPressed: C} { return { default: base as C, @@ -204,6 +220,24 @@ export function baseColor(base: BaseColor | C): {d } type SpectrumColor = Color | ArbitraryValue; + +/** + * Resolves a Spectrum color token name to a CSS color value string. + * Supports opacity modifiers via the `color/opacity` syntax. + * + * @param value - A Spectrum color token (e.g. `'gray-800'`, `'accent-900/50'`) or an arbitrary CSS color value. + * @returns A CSS color string. + * + * @example + * ```tsx + * import {color, style} from '@react-spectrum/s2/style' with {type: 'macro'}; + * + * const styles = style({ + * color: color('gray-800'), + * borderColor: color('accent-900/50') + * }); + * ``` + */ export function color(value: SpectrumColor): string { let arbitrary = parseArbitraryValue(value); if (arbitrary) { @@ -213,10 +247,44 @@ export function color(value: SpectrumColor): string { return colorTokenToString(resolveColorToken(baseColors[colorValue]), opacity); } +/** + * Produces a `light-dark()` CSS color value that resolves to different colors + * depending on the current color scheme. + * + * @param light - The color to use in light mode. + * @param dark - The color to use in dark mode. + * @returns A CSS `light-dark()` expression wrapped as an arbitrary style value. + * + * @example + * ```tsx + * import {lightDark, style} from '@react-spectrum/s2/style' with {type: 'macro'}; + * + * const styles = style({ + * backgroundColor: lightDark('gray-25', 'gray-900') + * }); + * ``` + */ export function lightDark(light: SpectrumColor, dark: SpectrumColor): `[${string}]` { return `[light-dark(${color(light)}, ${color(dark)})]`; } +/** + * Mixes two Spectrum colors by a given percentage using CSS `color-mix()` in sRGB color space. + * + * @param a - The first color. + * @param b - The second color. + * @param percent - The percentage of the second color in the mix (0–100). + * @returns A CSS `color-mix()` expression wrapped as an arbitrary style value. + * + * @example + * ```tsx + * import {colorMix, style} from '@react-spectrum/s2/style' with {type: 'macro'}; + * + * const styles = style({ + * backgroundColor: colorMix('accent-900', 'gray-25', 50) + * }); + * ``` + */ export function colorMix(a: SpectrumColor, b: SpectrumColor, percent: number): `[${string}]` { return `[color-mix(in srgb, ${color(a)}, ${color(b)} ${percent}%)]`; } @@ -346,6 +414,24 @@ const padding = { ...relativeSpacing }; +/** + * Converts a pixel value to a scalable CSS size expression using the Spectrum 2 scale factor. + * The result is a `calc()` expression that multiplies the rem-converted value by the current scale factor. + * The scale factor differs between touch and non-touch devices. + * + * @param px - The size in pixels. + * @returns A CSS `calc()` expression. + * + * @example + * ```tsx + * import {size, style} from '@react-spectrum/s2/style' with {type: 'macro'}; + * + * const styles = style({ + * width: size(200), + * height: size(48) + * }); + * ``` + */ export function size(this: MacroContext | void, px: number): `calc(${string})` { return `calc(${pxToRem(px)} * var(--s2-scale))`; } diff --git a/packages/@react-spectrum/s2/style/style-macro.ts b/packages/@react-spectrum/s2/style/style-macro.ts index c8e604f0fe5..d889d895e6d 100644 --- a/packages/@react-spectrum/s2/style/style-macro.ts +++ b/packages/@react-spectrum/s2/style/style-macro.ts @@ -870,31 +870,51 @@ class ConditionalRule extends GroupRule { } } -export function raw(this: MacroContext | void, css: string, layer = '_.a'): string { +/** + * Injects a raw CSS string into the style system. The CSS is wrapped in a generated + * class name and placed within the specified `@layer`. Returns the generated class name. + * This is an escape hatch for advanced cases (e.g. pseudo selectors or features not yet + * available in the style macro API), and should be used sparingly. + * Must be imported with `{type: 'macro'}`. + * + * @param content - The CSS declarations to inject. + * @param layer - The CSS `@layer` to place the styles in. Defaults to `'_.a'`. + * @returns The generated class name that applies the styles. + * + * @example + * ```tsx + * import {css} from '@react-spectrum/s2/style' with {type: 'macro'}; + * + * const styles = css(` + * backdrop-filter: blur(8px); + * `); + * ``` + */ +export function css(this: MacroContext | void, content: string, layer = '_.a'): string { // Check if `this` is undefined, which means style was not called as a macro but as a normal function. // We also check if this is globalThis, which happens in non-strict mode bundles. // Also allow style to be called as a normal function in tests. // @ts-ignore if ((this == null || this === globalThis) && process.env.NODE_ENV !== 'test') { - throw new Error('The raw macro must be imported with {type: "macro"}.'); + throw new Error('The css macro must be imported with {type: "macro"}.'); } - let className = generateArbitraryValueSelector(css, true); - css = `@layer ${layer} { + let className = generateArbitraryValueSelector(content, true); + content = `@layer ${layer} { .${className} { - ${css} + ${content} } }`; // Ensure layer is always declared after the _ layer used by style macro. if (!layer.startsWith('_.')) { - css = `@layer _, ${layer};\n` + css; + content = `@layer _, ${layer};\n` + content; } if (this && typeof this.addAsset === 'function') { this.addAsset({ type: 'css', - content: css + content }); } return className; diff --git a/packages/dev/parcel-packager-docs/DocsPackager.js b/packages/dev/parcel-packager-docs/DocsPackager.js index 8f243bd0ef7..8c3d9ebdfcd 100644 --- a/packages/dev/parcel-packager-docs/DocsPackager.js +++ b/packages/dev/parcel-packager-docs/DocsPackager.js @@ -369,6 +369,7 @@ function visitChildren(obj, fn) { return: fn(obj.return, 'return'), typeParameters: obj.typeParameters.map(i => fn(i, 'typeParameters')), description: obj.description, + examples: obj.examples, access: obj.access }; case 'interface': diff --git a/packages/dev/parcel-transformer-docs/DocsTransformer.js b/packages/dev/parcel-transformer-docs/DocsTransformer.js index 5cc8cca9b2b..1797cd8f5fe 100644 --- a/packages/dev/parcel-transformer-docs/DocsTransformer.js +++ b/packages/dev/parcel-transformer-docs/DocsTransformer.js @@ -767,6 +767,10 @@ module.exports = new Transformer({ let result = { description: parsed.description }; + let extractedExamples = extractExamples(comments); + if (extractedExamples.length > 0) { + result.examples = extractedExamples; + } for (let tag of parsed.tags) { if (tag.title === 'default') { @@ -789,15 +793,74 @@ module.exports = new Transformer({ result.params[tag.name] = tag.description; } else if (tag.title === 'selector') { result.selector = tag.description; + } else if (tag.title === 'example') { + if (!result.examples) { + result.examples = []; + } + + if (tag.description) { + result.examples.push(tag.description); + } } } + if (result.examples) { + result.examples = [...new Set(result.examples.map(example => example.trim()).filter(Boolean))]; + } + return result; } return {}; } + function extractExamples(comments) { + let lines = comments.split('\n') + .map(line => line.replace(/^\s*\*?\s?/, '')); + let examples = []; + let current = null; + + for (let line of lines) { + if (/^@example\b/.test(line)) { + if (current) { + let prev = current.join('\n').trim(); + if (prev) { + examples.push(prev); + } + } + + current = []; + let inlineExample = line.replace(/^@example\b\s*/, ''); + if (inlineExample) { + current.push(inlineExample); + } + continue; + } + + if (current) { + if (/^@\w+/.test(line)) { + let example = current.join('\n').trim(); + if (example) { + examples.push(example); + } + current = null; + continue; + } + + current.push(line); + } + } + + if (current) { + let example = current.join('\n').trim(); + if (example) { + examples.push(example); + } + } + + return examples; + } + function getDocComments(path) { if (path.node.leadingComments) { return path.node.leadingComments.filter(isJSDocComment).map(c => c.value).join('\n'); @@ -857,6 +920,10 @@ module.exports = new Transformer({ if (value.return) { value.return.description = docs.return || value.return.description || null; } + + if (docs.examples) { + value.examples = docs.examples; + } } asset.type = 'json'; diff --git a/packages/dev/s2-docs/pages/s2/style-macro.mdx b/packages/dev/s2-docs/pages/s2/style-macro.mdx index 0e23cc5968d..b720d9c1520 100644 --- a/packages/dev/s2-docs/pages/s2/style-macro.mdx +++ b/packages/dev/s2-docs/pages/s2/style-macro.mdx @@ -1,6 +1,9 @@ import {Layout} from '../../src/Layout'; import {InlineAlert, Heading, Content, Link} from '@react-spectrum/s2'; +import {FunctionJSDoc} from '../../src/FunctionJSDoc'; import {StyleMacroProperties} from '../../src/StyleMacroProperties'; +import docs from 'docs:@react-spectrum/s2'; +import styleDocs from 'docs:@react-spectrum/s2/style'; import {getPropertyDefinitions} from '../../src/styleProperties'; export default Layout; @@ -55,3 +58,60 @@ Note that `font` should be applied on a per element basis rather than globally s ## Conditions + +## Utilities + +The style macro system provides built-in utility functions for common patterns. + +### baseColor + + + +### color + + + +### lightDark + + + +### colorMix + + + +### size + + + +### space + + + +### fontRelative + + + +### focusRing + + + +### iconStyle + + +See the [Icons](icons#api) page for more information. + +### css + + + +### mergeStyles + + + +### centerPadding + + + +### setColorScheme + + diff --git a/packages/dev/s2-docs/pages/s2/styling.mdx b/packages/dev/s2-docs/pages/s2/styling.mdx index f791f8fb97b..3b2f2bd39bc 100644 --- a/packages/dev/s2-docs/pages/s2/styling.mdx +++ b/packages/dev/s2-docs/pages/s2/styling.mdx @@ -238,21 +238,23 @@ const styles = style({ ### Built-in utilities -Use `focusRing()` to add the standard Spectrum focus ring. +The style macro system includes built-in utilities for common patterns like focus rings, color helpers, spacing, sizing, animations, and more. For example, use `focusRing()` to add the standard Spectrum focus ring to interactive components: ```tsx "use client"; import {style, focusRing} from '@react-spectrum/s2/style' with {type: 'macro'}; -import {Button} from '@react-spectrum/s2'; +import {Button} from 'react-aria-components'; const buttonStyle = style({ ...focusRing(), // ...other styles }); - + ``` +See the [Utilities](style-macro#utilities) section in the style macro reference page for a full list of available utilities and examples. + ## Setting CSS variables CSS variables can be directly defined in a `style` macro, allowing child elements to then access them in their own styles. diff --git a/packages/dev/s2-docs/scripts/generateMarkdownDocs.mjs b/packages/dev/s2-docs/scripts/generateMarkdownDocs.mjs index f489adefc4c..22bd7f76b3f 100644 --- a/packages/dev/s2-docs/scripts/generateMarkdownDocs.mjs +++ b/packages/dev/s2-docs/scripts/generateMarkdownDocs.mjs @@ -42,6 +42,7 @@ function getBaseUrl(library) { const __dirname = path.dirname(fileURLToPath(import.meta.url)); const REPO_ROOT = path.resolve(__dirname, '../../../../'); const S2_SRC_ROOT = path.join(REPO_ROOT, 'packages/@react-spectrum/s2/src'); +const S2_STYLE_ROOT = path.join(REPO_ROOT, 'packages/@react-spectrum/s2/style'); const RAC_SRC_ROOT = path.join(REPO_ROOT, 'packages/react-aria-components/src'); const INTL_SRC_ROOT = path.join(REPO_ROOT, 'packages/@internationalized'); const COMPONENT_SRC_ROOTS = [S2_SRC_ROOT, RAC_SRC_ROOT, INTL_SRC_ROOT]; @@ -65,6 +66,7 @@ const interfaceTableCache = new Map(); const classTableCache = new Map(); const propTableCache = new Map(); const descriptionCache = new Map(); +const functionExamplesCache = new Map(); let tsFileIndex = null; let styleMacroDataCache = null; const styleMacroTableCache = new Map(); @@ -885,38 +887,101 @@ function extractJSXText(node, file) { return ''; } -function getRootsForFile(file) { +function getCacheKey(name, file) { if (file?.path) { if (file.path.includes(path.join('pages', 'react-aria', 'internationalized'))) { - return [INTL_SRC_ROOT, S2_SRC_ROOT, RAC_SRC_ROOT]; + return `intl:${name}`; } else if (file.path.includes(path.join('pages', 'react-aria'))) { - return [RAC_SRC_ROOT, S2_SRC_ROOT, INTL_SRC_ROOT]; + return `rac:${name}`; + } else if (file.path.includes(path.join('pages', 's2'))) { + return `s2:${name}`; } } - return COMPONENT_SRC_ROOTS; + return `default:${name}`; } -function getCacheKey(name, file) { +function getDocsImportSource(identifier, file) { + return file?.data?.docsImports?.[identifier] || null; +} + +function getExistingRoots(roots) { + return [...new Set(roots.filter(root => root && fs.existsSync(root)))]; +} + +function getRootsForDocsSource(docsSource, file) { + if (!docsSource) { + return null; + } + + if (docsSource === '@react-spectrum/s2') { + return getExistingRoots([S2_SRC_ROOT, S2_STYLE_ROOT]); + } + + if (docsSource === '@react-spectrum/s2/style') { + return getExistingRoots([S2_STYLE_ROOT]); + } + + if (docsSource.startsWith('./') || docsSource.startsWith('../')) { + if (!file?.path) { + return null; + } + + const resolved = path.resolve(path.dirname(file.path), docsSource); + const candidates = [ + resolved, + `${resolved}.ts`, + `${resolved}.tsx`, + `${resolved}.d.ts`, + path.join(resolved, 'index.ts'), + path.join(resolved, 'index.tsx'), + path.join(resolved, 'index.d.ts') + ]; + + return getExistingRoots(candidates.map(candidate => { + if (!fs.existsSync(candidate)) { + return null; + } + + return fs.statSync(candidate).isDirectory() ? candidate : path.dirname(candidate); + })); + } + + const packagePath = path.join(REPO_ROOT, 'packages', docsSource); + if (fs.existsSync(packagePath)) { + if (fs.statSync(packagePath).isDirectory()) { + return getExistingRoots([path.join(packagePath, 'src'), packagePath]); + } + + return getExistingRoots([path.dirname(packagePath)]); + } + + return null; +} + +function getRootsForFile(file, docsSource) { + const docsRoots = getRootsForDocsSource(docsSource, file); + if (docsRoots?.length) { + return docsRoots; + } + if (file?.path) { if (file.path.includes(path.join('pages', 'react-aria', 'internationalized'))) { - return `intl:${name}`; + return [INTL_SRC_ROOT, S2_SRC_ROOT, RAC_SRC_ROOT]; } else if (file.path.includes(path.join('pages', 'react-aria'))) { - return `rac:${name}`; - } else if (file.path.includes(path.join('pages', 's2'))) { - return `s2:${name}`; + return [RAC_SRC_ROOT, S2_SRC_ROOT, INTL_SRC_ROOT]; } } - return `default:${name}`; + return COMPONENT_SRC_ROOTS; } -function resolveComponentPath(componentName, file) { +function resolveComponentPath(componentName, file, docsSource) { // Check unified cache first - const cacheKey = getCacheKey(componentName, file); + const cacheKey = getCacheKey(`${docsSource || 'default'}:${componentName}:path`, file); if (interfacePathCache.has(cacheKey)) { return interfacePathCache.get(cacheKey); } - - let roots = getRootsForFile(file); + + let roots = getRootsForFile(file, docsSource); // Fast path: check direct file paths first for (let root of roots) { @@ -969,14 +1034,14 @@ function resolveComponentPath(componentName, file) { /** * Extract the leading JSDoc description comment placed immediately above the export for a component. */ -function getComponentDescription(componentName, file) { +function getComponentDescription(componentName, file, docsSource) { // Check cache first - const cacheKey = getCacheKey(componentName, file); + const cacheKey = getCacheKey(`${docsSource || 'default'}:${componentName}:description`, file); if (descriptionCache.has(cacheKey)) { return descriptionCache.get(cacheKey); } - const componentPath = resolveComponentPath(componentName, file); + const componentPath = resolveComponentPath(componentName, file, docsSource); if (!componentPath) { descriptionCache.set(cacheKey, null); return null; @@ -1071,6 +1136,154 @@ function shouldOmitSymbol(sym) { }); } +/** + * Extracts one or more `@example` tag contents from JSDoc comments. + * @param {string} text - The text to extract examples from. + * @returns {string[]} An array of examples. + */ +function extractExamplesFromText(text) { + if (!text || typeof text !== 'string') { + return []; + } + + let lines = text.split('\n').map(line => line.replace(/^\s*\*?\s?/, '')); + let examples = []; + let current = null; + + for (let line of lines) { + if (/^@example\b/.test(line)) { + if (current) { + let prev = current.join('\n').trim(); + if (prev) { + examples.push(prev); + } + } + + current = []; + let inlineExample = line.replace(/^@example\b\s*/, ''); + if (inlineExample) { + current.push(inlineExample); + } + continue; + } + + if (current) { + if (/^@\w+/.test(line)) { + let example = current.join('\n').trim(); + if (example) { + examples.push(example); + } + current = null; + continue; + } + + current.push(line); + } + } + + if (current) { + let example = current.join('\n').trim(); + if (example) { + examples.push(example); + } + } + + return examples; +} + +function parseFencedCodeBlock(example) { + if (!example || typeof example !== 'string') { + return null; + } + + let trimmed = example.trim(); + let match = trimmed.match(/^```([^\n`]*)\n([\s\S]*?)\n```$/); + if (!match) { + return null; + } + + let [, lang, code] = match; + return { + lang: lang?.trim() || undefined, + code + }; +} + +function getFunctionExamples(functionName, file, docsSource) { + const cacheKey = getCacheKey(`${docsSource || 'default'}:${functionName}:examples`, file); + if (functionExamplesCache.has(cacheKey)) { + return functionExamplesCache.get(cacheKey); + } + + const functionPath = resolveComponentPath(functionName, file, docsSource); + if (!functionPath) { + functionExamplesCache.set(cacheKey, []); + return []; + } + + const source = project.addSourceFileAtPathIfExists(functionPath); + if (!source) { + functionExamplesCache.set(cacheKey, []); + return []; + } + + const exportedDecl = source.getExportedDeclarations().get(functionName)?.[0]; + const possibleNodes = [exportedDecl, source.getVariableDeclaration(functionName), source.getFunction(functionName)]; + + let firstNodeExamples = []; + for (let node of possibleNodes.filter(Boolean)) { + let current = node; + let isDirectNode = true; + + while (current) { + let docs = typeof current.getJsDocs === 'function' ? current.getJsDocs() : []; + if (!docs?.length) { + isDirectNode = false; + current = current.getParent?.(); + continue; + } + + let directExamples = []; + for (let doc of docs) { + let tags = doc.getTags?.() || []; + let tagExamples = tags + .filter(tag => tag.getTagName?.() === 'example') + .map(tag => tag.getCommentText?.()) + .filter(Boolean) + .map(value => value.trim()); + directExamples.push(...tagExamples); + + if (tagExamples.length === 0) { + let docText = doc.getInnerText?.() || doc.getText?.() || ''; + directExamples.push(...extractExamplesFromText(docText)); + } + } + + directExamples = [...new Set(directExamples.filter(Boolean))]; + if (!directExamples.length) { + isDirectNode = false; + current = current.getParent?.(); + continue; + } + + if (isDirectNode) { + functionExamplesCache.set(cacheKey, directExamples); + return directExamples; + } + + if (!firstNodeExamples.length) { + firstNodeExamples = directExamples; + } + + isDirectNode = false; + current = current.getParent?.(); + } + } + + functionExamplesCache.set(cacheKey, firstNodeExamples); + return firstNodeExamples; +} + /** * Build a markdown table of props for the given component by analyzing its interface. */ @@ -1302,11 +1515,38 @@ function generateInterfaceTable(interfaceName, file) { * Custom remark plugin that removes MDX import/export statements. */ function remarkRemoveImportsExports() { - return (tree) => { + return (tree, file) => { + let docsImports = {}; visit(tree, 'mdxjsEsm', (node, index, parent) => { + if (node.value) { + try { + const ast = babel.parse(node.value, { + sourceType: 'module', + plugins: ['jsx', 'typescript'] + }); + + for (const statement of ast.program.body) { + if (statement.type !== 'ImportDeclaration' || typeof statement.source.value !== 'string' || !statement.source.value.startsWith('docs:')) { + continue; + } + + const docsSource = statement.source.value.slice(5); + for (const specifier of statement.specifiers) { + if (specifier.local?.name) { + docsImports[specifier.local.name] = docsSource; + } + } + } + } catch { + // Ignore non-import ESM blocks. + } + } + parent.children.splice(index, 1); return index; }); + + file.data.docsImports = docsImports; }; } @@ -1515,6 +1755,63 @@ function remarkDocsComponentsToMarkdown() { return index; } + // Render function description + examples from JSDoc. + if (name === 'FunctionJSDoc') { + const functionAttr = node.attributes?.find((a) => a.name === 'function'); + let functionName = null; + let docsSource = null; + if (functionAttr && functionAttr.value?.type === 'mdxJsxAttributeValueExpression') { + const m = functionAttr.value.value.match(/^([\w$]+)\.exports\.([\w$]+)$/); + if (m) { + docsSource = getDocsImportSource(m[1], file); + functionName = m[2]; + } else { + const fallback = functionAttr.value.value.match(/\.exports\.([\w$]+)/); + if (fallback) { + functionName = fallback[1]; + } + } + } + + if (!functionName) { + parent.children.splice(index, 1); + return index; + } + + const newNodes = []; + const description = getComponentDescription(functionName, file, docsSource); + if (description) { + const descTree = unified().use(remarkParse).parse(description); + newNodes.push(...descTree.children); + } + + const examples = getFunctionExamples(functionName, file, docsSource); + for (let [exampleIndex, example] of examples.entries()) { + if (examples.length > 1) { + newNodes.push({ + type: 'paragraph', + children: [{type: 'strong', children: [{type: 'text', value: `Example ${exampleIndex + 1}:`}]}] + }); + } + + const fenced = parseFencedCodeBlock(example); + if (fenced) { + newNodes.push({ + type: 'code', + lang: fenced.lang || 'tsx', + meta: '', + value: fenced.code + }); + } else { + const exampleTree = unified().use(remarkParse).parse(example); + newNodes.push(...exampleTree.children); + } + } + + parent.children.splice(index, 1, ...newNodes); + return index + newNodes.length; + } + // Render a table of props. if (name === 'PropTable') { const compAttr = node.attributes?.find((a) => a.name === 'component'); diff --git a/packages/dev/s2-docs/src/FunctionJSDoc.tsx b/packages/dev/s2-docs/src/FunctionJSDoc.tsx new file mode 100644 index 00000000000..771b0cd56cc --- /dev/null +++ b/packages/dev/s2-docs/src/FunctionJSDoc.tsx @@ -0,0 +1,69 @@ +/* + * Copyright 2026 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {Code} from './Code'; +import React from 'react'; +import {renderHTMLfromMarkdown} from './types'; +import {standaloneCode} from './CodeBlock'; +import {style} from '@react-spectrum/s2/style' with {type: 'macro'}; + +interface FunctionJSDocProps { + function: { + description?: string | null, + examples?: string[] + } +} + +function parseFencedCodeBlock(example: string): {lang?: string, code: string} | null { + let trimmed = example.trim(); + let match = trimmed.match(/^```([^\n`]*)\n([\s\S]*?)\n```$/); + if (!match) { + return null; + } + + let [, lang, code] = match; + return { + lang: lang?.trim() || undefined, + code + }; +} + +export function FunctionJSDoc({function: func}: FunctionJSDocProps) { + let examples = Array.isArray(func.examples) + ? func.examples.filter(Boolean) + : []; + + return ( +
+ {renderHTMLfromMarkdown(func.description, {forceInline: false, forceBlock: true})} + {examples.map((example, index) => { + let parsedExample = parseFencedCodeBlock(example); + return ( +
+ {examples.length > 1 && + + Example {index + 1}: + + } + {parsedExample + ? ( + 
+                  {parsedExample.code}
+
+ ) + : renderHTMLfromMarkdown(example, {forceInline: false, forceBlock: true})} +
+ ); + })} +
+ ); +} diff --git a/packages/dev/s2-docs/src/Step.tsx b/packages/dev/s2-docs/src/Step.tsx index 69a98d9bad3..c9abbaf2034 100644 --- a/packages/dev/s2-docs/src/Step.tsx +++ b/packages/dev/s2-docs/src/Step.tsx @@ -1,4 +1,4 @@ -import {raw} from '../../../@react-spectrum/s2/style/style-macro' with {type: 'macro'}; +import {css} from '../../../@react-spectrum/s2/style/style-macro' with {type: 'macro'}; import {style} from '@react-spectrum/s2/style' with {type: 'macro'}; export function StepList({children}) { @@ -33,7 +33,7 @@ export function Counter() { return ( Date: Tue, 17 Mar 2026 16:01:20 -0500 Subject: [PATCH 09/13] fix: Focus behaviour on inputs inside a FocusScope (#8903) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Revert "Revert "fix: Focus behaviour on inputs inside a FocusScope (#8498)" (…" This reverts commit 84ff482298bad45c9ae0ca63935213c1e387148e. * Update to only happen on Tab * fix lint * undo a few other extra and unnecessary selects --- packages/@react-aria/focus/src/FocusScope.tsx | 3 +++ .../@react-aria/focus/test/FocusScope.test.js | 21 +++++++++++++++++++ .../s2/stories/Dialog.stories.tsx | 9 +++++--- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/packages/@react-aria/focus/src/FocusScope.tsx b/packages/@react-aria/focus/src/FocusScope.tsx index 0a0dc1310f4..87887359f1b 100644 --- a/packages/@react-aria/focus/src/FocusScope.tsx +++ b/packages/@react-aria/focus/src/FocusScope.tsx @@ -373,6 +373,9 @@ function useFocusContainment(scopeRef: RefObject, contain?: bo e.preventDefault(); if (nextElement) { focusElement(nextElement, true); + if (nextElement instanceof getOwnerWindow(nextElement).HTMLInputElement) { + nextElement.select(); + } } }; diff --git a/packages/@react-aria/focus/test/FocusScope.test.js b/packages/@react-aria/focus/test/FocusScope.test.js index 38648c12635..31644123abf 100644 --- a/packages/@react-aria/focus/test/FocusScope.test.js +++ b/packages/@react-aria/focus/test/FocusScope.test.js @@ -354,6 +354,27 @@ describe('FocusScope', function () { expect(document.activeElement).toBe(input2); }); + + it('should select all text in input when tabbing', async function () { + let {getByTestId} = render( + + + + + + ); + + let input1 = getByTestId('input1'); + let input2 = getByTestId('input2'); + + act(() => {input1.focus();}); + expect(document.activeElement).toBe(input1); + + await user.tab(); + expect(document.activeElement).toBe(input2); + await user.keyboard('{Delete}'); + expect(input2.value).toBe(''); + }); }); describe('focus restoration', function () { diff --git a/packages/@react-spectrum/s2/stories/Dialog.stories.tsx b/packages/@react-spectrum/s2/stories/Dialog.stories.tsx index da39010cc37..2fc7e5a0490 100644 --- a/packages/@react-spectrum/s2/stories/Dialog.stories.tsx +++ b/packages/@react-spectrum/s2/stories/Dialog.stories.tsx @@ -48,9 +48,12 @@ const ExampleRender = (args: ExampleRenderProps): ReactElement => ( Dialog title
Header
- {[...Array(args.paragraphs)].map((_, i) => -

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in

- )} + <> + {[...Array(args.paragraphs)].map((_, i) => +

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in

+ )} + +
Don't show this again
From c45205e3308081f5e683274c049401f1c42bcc92 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 17 Mar 2026 17:04:39 -0500 Subject: [PATCH 10/13] chore: Store all macro data on the client (#9771) * Store all macro debug data in the app * encode the data for css storage * fix encodings * fix memory leak, streamline look up, disable globals in tests --- .../@react-spectrum/s2/style/style-macro.ts | 19 +- .../dev/style-macro-chrome-plugin/README.md | 210 ++++-------------- .../src/background.js | 2 +- .../src/content-script.js | 20 +- .../style-macro-chrome-plugin/src/devtool.js | 143 +++++------- 5 files changed, 118 insertions(+), 276 deletions(-) diff --git a/packages/@react-spectrum/s2/style/style-macro.ts b/packages/@react-spectrum/s2/style/style-macro.ts index d889d895e6d..d3de6088b80 100644 --- a/packages/@react-spectrum/s2/style/style-macro.ts +++ b/packages/@react-spectrum/s2/style/style-macro.ts @@ -407,8 +407,23 @@ export function createTheme(theme: T): StyleFunction` - stores all dynamic macro data +- **Storage**: None - all data is read from CSS custom properties on demand - **Mutation Observer**: - Created when an element is selected via `chrome.devtools.panels.elements.onSelectionChanged` - Watches the selected element's `class` attribute for changes @@ -117,7 +108,6 @@ This extension uses Chrome's standard extension architecture with three main com - Triggers automatic panel refresh when className changes - **Communication**: - Receives: - - `port.onMessage({ action: 'stylemacro-update-macros', hash, loc, style })` from background (stores data and refreshes) - `port.onMessage({ action: 'stylemacro-class-changed', elementId })` from background (triggers refresh) - Sends: - `chrome.runtime.connect({ name: 'devtools-page' })` to establish connection @@ -149,156 +139,51 @@ Static macros are generated when style macro conditions don't change at runtime. **Key Design**: Each static macro has its own uniquely-named custom property (`--macro-data-{hash}`), which avoids CSS cascade issues when reading multiple macro data from the same element. -#### Flow 1b: Dynamic Macro Updates (Page → DevTools) +#### Flow 1b: Dynamic Macro Updates (Page → Global Variable) -Dynamic macros are generated when style macro conditions can change at runtime. Updates are sent via message passing and stored directly in DevTools. +Dynamic macros are generated when style macro conditions can change at runtime. Data is written to a global JavaScript variable; a timer on the same object cleans up entries whose class is no longer present in the DOM. ``` ┌─────────────────┐ │ Page Context │ -│ (style-macro) │ -└────────┬────────┘ - │ window.postMessage({ action: 'stylemacro-update-macros', hash, loc, style }) - ↓ -┌─────────────────┐ -│ Content Script │ Forwards message (no storage) -└────────┬────────┘ - │ chrome.runtime.sendMessage({ action: 'stylemacro-update-macros', hash, loc, style }) - ↓ -┌─────────────────┐ -│ Background │ Looks up DevTools connection for tabId +│ (style-macro) │ Runtime evaluation with dynamic conditions └────────┬────────┘ - │ port.postMessage({ action: 'stylemacro-update-macros', hash, loc, style }) + │ 1. Ensure window.__styleMacroDynamic__ exists ({ map: {}, _timer }) + │ 2. Set map["-macro-dynamic-{hash}"] = { style, loc } (plain JSON) + │ 3. Timer (e.g. every 5 min) removes entries with no matching element in DOM ↓ ┌─────────────────┐ -│ DevTools Panel │ Stores in macroData Map and triggers sidebar refresh +│ Page (global) │ window.__styleMacroDynamic__.map["-macro-dynamic-{hash}"] = { style, loc } └─────────────────┘ ``` -#### Flow 2: Display Macro Data (Synchronous Lookup) +#### Flow 2: Display Macro Data (Synchronous CSS Lookup) -When the user selects an element or the panel refreshes, DevTools looks up macro data synchronously from its local storage. +When the user selects an element or the panel refreshes, DevTools reads macro data as follows. ``` ┌─────────────────┐ -│ DevTools Panel │ User selects element with -macro-dynamic-{hash} class +│ DevTools Panel │ User selects element with -macro-static-{hash} or -macro-dynamic-{hash} class └────────┬────────┘ │ Extract hash from className ↓ ┌─────────────────┐ -│ DevTools Panel │ Look up macroData.get(hash) -│ Local Storage │ Returns { loc, style } if available +│ DevTools Panel │ Static: getComputedStyle($0).getPropertyValue('--macro-data-{hash}') → JSON.parse +│ │ Dynamic: window.__styleMacroDynamic__.map["-macro-dynamic-{hash}"] → already { style, loc } └────────┬────────┘ - │ { loc: "...", style: {...} } or null + │ ↓ ┌─────────────────┐ -│ DevTools Panel │ Display in sidebar (or show nothing if null) +│ DevTools Panel │ Parses and displays in sidebar └─────────────────┘ ``` -**Note**: If macro data hasn't been received yet for a hash, it will appear empty until the next `stylemacro-update-macros` message arrives and triggers a refresh. +**Note**: Static macros are read from CSS custom properties on the inspected element. Dynamic macros are read from the page’s global `window.__styleMacroDynamic__.map` (no CSS involved). -#### Flow 3: Macro Data Cleanup (Automated) - -Every 5 minutes, DevTools checks if stored macro hashes are still in use on the page and removes stale data. - -``` -┌─────────────────┐ -│ DevTools Panel │ Every 5 minutes -└────────┬────────┘ - │ For each hash in macroData Map: - │ chrome.devtools.inspectedWindow.eval( - │ `!!document.querySelector('.-macro-dynamic-${hash}')` - │ ) - ↓ -┌─────────────────┐ -│ Page DOM │ Checks if elements with macro classes exist -└────────┬────────┘ - │ Returns true/false for each hash - ↓ -┌─────────────────┐ -│ DevTools Panel │ Removes stale entries from macroData Map -│ │ macroData.delete(hash) for non-existent elements -└─────────────────┘ -``` - -#### Flow 4: Automatic Updates on className Changes (MutationObserver) +#### Flow 3: Automatic Updates on className Changes (MutationObserver) When you select an element, the DevTools panel automatically watches for className changes and refreshes the panel. -``` -┌─────────────────┐ -│ DevTools Panel │ User selects element in Elements panel -└────────┬────────┘ - │ chrome.devtools.panels.elements.onSelectionChanged - │ - │ chrome.devtools.inspectedWindow.eval(` - │ // Disconnect old observer (if any) - │ if (window.__styleMacroObserver) { - │ window.__styleMacroObserver.disconnect(); - │ } - │ - │ // Create new MutationObserver on $0 - │ window.__styleMacroObserver = new MutationObserver(() => { - │ window.postMessage({ - │ action: 'stylemacro-class-changed', - │ elementId: $0.__devtoolsId - │ }, '*'); - │ }); - │ - │ window.__styleMacroObserver.observe($0, { - │ attributes: true, - │ attributeFilter: ['class'] - │ }); - │ `) - ↓ -┌─────────────────┐ -│ Page DOM │ MutationObserver active on selected element -└────────┬────────┘ - │ - │ ... User interacts with page, element's className changes ... - │ - │ MutationObserver detects class attribute change - │ window.postMessage({ action: 'stylemacro-class-changed', elementId }, '*') - ↓ -┌─────────────────┐ -│ Content Script │ Receives window message, forwards to extension -└────────┬────────┘ - │ chrome.runtime.sendMessage({ action: 'stylemacro-class-changed', elementId }) - ↓ -┌─────────────────┐ -│ Background │ Looks up DevTools connection for tabId -└────────┬────────┘ - │ port.postMessage({ action: 'stylemacro-class-changed', elementId }) - ↓ -┌─────────────────┐ -│ DevTools Panel │ Verifies elementId matches currently selected element -│ │ Triggers full panel refresh (re-reads classes, re-queries macros) -└─────────────────┘ - -When selection changes or panel closes: - ↓ -┌─────────────────┐ -│ DevTools Panel │ Calls disconnectObserver() -└────────┬────────┘ - │ chrome.devtools.inspectedWindow.eval(` - │ if (window.__styleMacroObserver) { - │ window.__styleMacroObserver.disconnect(); - │ window.__styleMacroObserver = null; - │ } - │ `) - ↓ -┌─────────────────┐ -│ Page DOM │ Old observer disconnected, new observer created for new selection -└─────────────────┘ -``` - -**Key Benefits:** -- Panel automatically refreshes when element classes change (e.g., hover states, conditional styles) -- No manual refresh needed -- Observer is cleaned up properly to prevent memory leaks -- Each element has its own unique tracking ID to prevent cross-contamination - ### Key Technical Details #### Why Background Script is Needed @@ -310,25 +195,16 @@ The style macro generates different class name patterns based on whether the sty **Static Macros** (`-macro-static-{hash}`): - Used when all style conditions are static (e.g., `style({ color: 'red' })`) -- Macro data is embedded in CSS as a uniquely-named custom property: `--macro-data-{hash}: '{...JSON...}'` +- Macro data is embedded in CSS rules as a uniquely-named custom property: `--macro-data-{hash}: '{...JSON...}'` - DevTools reads the specific custom property via `getComputedStyle($0).getPropertyValue('--macro-data-{hash}')` -- Unique naming avoids CSS cascade issues when multiple macros are applied to the same element **Dynamic Macros** (`-macro-dynamic-{hash}`): - Used when style conditions can change (e.g., `style({color: {default: 'blue', isActive: 'red'}})`) -- Macro data is sent via `window.postMessage({ action: 'stylemacro-update-macros', ... })` whenever conditions change -- Content script forwards data to DevTools, which stores it in a local Map +- Macro data is stored in `window.__styleMacroDynamic__.map["-macro-dynamic-{hash}"]` as plain JSON `{ style, loc }` +- A timer on `window.__styleMacroDynamic__` periodically removes map entries whose class is no longer used in the DOM +- DevTools reads via `window.__styleMacroDynamic__.map["-macro-dynamic-{hash}"]` - Enables real-time updates when props/state change -#### Data Storage -- **Static Macros**: Data embedded in CSS as uniquely-named custom properties `--macro-data-{hash}`, read via `getComputedStyle($0).getPropertyValue('--macro-data-{hash}')` - - Each macro has its own custom property name to prevent cascade conflicts - - Example: `.-macro-static-abc123 { --macro-data-abc123: '{"style": {...}, "loc": "..."}'; }` -- **Dynamic Macros**: Data stored in DevTools panel's `macroData` Map -- **No Content Script Storage**: Content script only forwards messages, doesn't store macro data -- **Lifetime**: Macro data persists in DevTools for the duration of the DevTools session -- **Cleanup**: Stale macro data (for elements no longer in DOM) is removed every 5 minutes - #### Connection Management - **DevTools → Background**: Uses persistent `chrome.runtime.connect()` with port-based messaging - **Content Script → Background**: Uses one-time `chrome.runtime.sendMessage()` calls @@ -336,38 +212,30 @@ The style macro generates different class name patterns based on whether the sty #### Data Structure -**Static Macros (in CSS):** +**Static Macros (in main CSS):** ```css .-macro-static-zsZ9Dc { --macro-data-zsZ9Dc: '{"style":{"paddingX":"4"},"loc":"packages/@react-spectrum/s2/src/Button.tsx:67"}'; } ``` -**Dynamic Macros (in DevTools panel's macroData Map):** +**Dynamic Macros (in page global):** ```javascript -Map { - "zsZ9Dc" => { - loc: "packages/@react-spectrum/s2/src/Button.tsx:67", - style: { - "paddingX": "4", - // ... more CSS properties - } - } -} +window.__styleMacroDynamic__ = { + map: { + "-macro-dynamic-zsZ9Dc": { style: { paddingX: "4" }, loc: "packages/@react-spectrum/s2/src/Button.tsx:67" }, + "-macro-dynamic-abc123": { style: {...}, loc: "..." } + }, + _timer: 123 // setInterval for cleanup of unused entries +}; ``` -**Note**: -- Static macro data is stored in CSS with uniquely-named custom properties -- Dynamic macro data is stored directly in the DevTools panel context -- The content script acts purely as a message forwarder and doesn't store any data - #### Message Types | Message Type | Direction | Purpose | |-------------|-----------|---------| -| `stylemacro-update-macros` | Page → Content → Background → DevTools | Send macro data (hash, loc, style) to be stored in DevTools | | `stylemacro-init` | DevTools → Background | Establish connection with tabId | -| `stylemacro-class-changed` | Page → Content → Background → DevTools | Notify that selected element's className changed | +| `stylemacro-class-changed` | Page → Content → Background → DevTools | Notify that selected element's className changed, triggering panel refresh | ### Debugging diff --git a/packages/dev/style-macro-chrome-plugin/src/background.js b/packages/dev/style-macro-chrome-plugin/src/background.js index 6524078c5a0..62a6f7467a6 100644 --- a/packages/dev/style-macro-chrome-plugin/src/background.js +++ b/packages/dev/style-macro-chrome-plugin/src/background.js @@ -36,7 +36,7 @@ chrome.runtime.onMessage.addListener((message, sender, sendResponse) => { } // Forward messages from content script to DevTools - if (message.action === 'stylemacro-update-macros' || message.action === 'stylemacro-class-changed') { + if (message.action === 'stylemacro-class-changed') { console.log(`[Background] Forwarding ${message.action} from content script to DevTools, tabId: ${tabId}`); const devtoolsPort = devtoolsConnections.get(tabId); if (devtoolsPort) { diff --git a/packages/dev/style-macro-chrome-plugin/src/content-script.js b/packages/dev/style-macro-chrome-plugin/src/content-script.js index be2418379bf..1bd76336986 100644 --- a/packages/dev/style-macro-chrome-plugin/src/content-script.js +++ b/packages/dev/style-macro-chrome-plugin/src/content-script.js @@ -19,25 +19,7 @@ window.addEventListener('message', function (event) { // Only accept messages that we know are ours. Note that this is not foolproof // and the page can easily spoof messages if it wants to. if (message && typeof message === 'object') { - if (message.action === 'stylemacro-update-macros') { - debugLog('Forwarding stylemacro-update-macros for hash:', message.hash); - - // if this script is run multiple times on the page, then only handle it once - event.stopImmediatePropagation(); - event.stopPropagation(); - - // Forward message directly to background script (which forwards to DevTools) - try { - chrome.runtime.sendMessage({ - action: 'stylemacro-update-macros', - hash: message.hash, - loc: message.loc, - style: message.style - }); - } catch (err) { - debugLog('Failed to send stylemacro-update-macros message:', err); - } - } else if (message.action === 'stylemacro-class-changed') { + if (message.action === 'stylemacro-class-changed') { // Forward class-changed messages from page context to background script debugLog('Forwarding stylemacro-class-changed for element:', message.elementId); diff --git a/packages/dev/style-macro-chrome-plugin/src/devtool.js b/packages/dev/style-macro-chrome-plugin/src/devtool.js index 7587534a07f..42d3acff041 100644 --- a/packages/dev/style-macro-chrome-plugin/src/devtool.js +++ b/packages/dev/style-macro-chrome-plugin/src/devtool.js @@ -28,9 +28,6 @@ chrome.devtools.panels.elements.createSidebarPane('Style Macros', (sidebar) => { }); debugLog('Init message sent to background'); - // Store macro data directly in DevTools - const macroData = new Map(); - // Track mutation observer for selected element let currentObserver = null; let currentElementId = null; @@ -39,17 +36,7 @@ chrome.devtools.panels.elements.createSidebarPane('Style Macros', (sidebar) => { backgroundPageConnection.onMessage.addListener((message) => { debugLog('Message from background:', message); - if (message.action === 'stylemacro-update-macros') { - debugLog('Received stylemacro-update-macros for hash:', message.hash); - // Store the macro data directly in DevTools - macroData.set(message.hash, { - loc: message.loc, - style: message.style - }); - debugLog('Stored macro data, total macros:', macroData.size); - // Refresh the panel to show updated data - update(); - } else if (message.action === 'stylemacro-class-changed') { + if (message.action === 'stylemacro-class-changed') { debugLog('Received stylemacro-class-changed notification for element:', message.elementId); // Only update if the changed element is the one we're currently watching if (message.elementId === currentElementId) { @@ -59,23 +46,62 @@ chrome.devtools.panels.elements.createSidebarPane('Style Macros', (sidebar) => { } }); - // Get macro data from local storage - const getDynamicMacroData = (hash) => { - debugLog('Looking up dynamic macro with hash:', hash); - const data = macroData.get(hash); - debugLog('Found data:', !!data); - return data || null; - }; + // Get all static macro data in one eval (from CSS custom properties on $0). + // Uses the element's window so iframes work correctly. Value is plain JSON. + function getMacroDataStaticBatch(hashes) { + if (hashes.length === 0) { + return Promise.resolve([]); + } + return new Promise((resolve) => { + const hashesJson = JSON.stringify(hashes); + const staticEval = ` +(function () { + var el = $0; + if (!el) return []; + var w = (el.ownerDocument && el.ownerDocument.defaultView) || window; + var s = w.getComputedStyle(el); + var hashes = ${hashesJson}; + return hashes.map(function (h) { + var raw = s.getPropertyValue("--macro-data-" + h).trim(); + if (!raw) return null; + try { + return JSON.parse(raw); + } catch (e) { + return null; + } + }); +})(); + `.trim(); + chrome.devtools.inspectedWindow.eval(staticEval, (results) => { + resolve(Array.isArray(results) ? results : []); + }); + }); + } - function getMacroData(className) { - let promise = new Promise((resolve) => { - debugLog('Getting macro data for:', className); - chrome.devtools.inspectedWindow.eval(`window.getComputedStyle($0).getPropertyValue("--macro-data-${className}")`, (style) => { - debugLog('Got style:', style); - resolve(style ? JSON.parse(style) : null); + // Get all dynamic macro data in one eval. Uses $0's window (correct for iframes). + // Reads from (element's document defaultView).__styleMacroDynamic__.map. + function getMacroDataDynamicBatch(hashes) { + if (hashes.length === 0) { + return Promise.resolve([]); + } + return new Promise((resolve) => { + const hashesJson = JSON.stringify(hashes); + const dynamicEval = ` +(function () { + var el = $0; + var w = (el && el.ownerDocument && el.ownerDocument.defaultView) || window; + var g = w && w.__styleMacroDynamic__; + if (!g || !g.map) return []; + var hashes = ${hashesJson}; + return hashes.map(function (h) { + return g.map["-macro-dynamic-" + h] || null; + }); +})(); + `.trim(); + chrome.devtools.inspectedWindow.eval(dynamicEval, (results) => { + resolve(Array.isArray(results) ? results : []); }); }); - return promise; } // Function to disconnect the current observer @@ -167,16 +193,12 @@ chrome.devtools.panels.elements.createSidebarPane('Style Macros', (sidebar) => { debugLog('Static macro hashes:', staticMacroHashes); debugLog('Dynamic macro hashes:', dynamicMacroHashes); - // Get static macro data (async from CSS) - let staticMacros = staticMacroHashes.map(macro => getMacroData(macro)); - - debugLog('Waiting for', staticMacros.length, 'static macros...'); - let staticResults = await Promise.all(staticMacros); - - // Get dynamic macro data (sync from local storage) - let dynamicResults = dynamicMacroHashes.map(hash => getDynamicMacroData(hash)); - - // Combine results + // Get macro data: static from CSS custom properties on $0, dynamic from element's window.__styleMacroDynamic__.map + debugLog('Waiting for', staticMacroHashes.length, 'static +', dynamicMacroHashes.length, 'dynamic macros...'); + let [staticResults, dynamicResults] = await Promise.all([ + getMacroDataStaticBatch(staticMacroHashes), + getMacroDataDynamicBatch(dynamicMacroHashes) + ]); let results = [...staticResults, ...dynamicResults]; debugLog('Results:', results); @@ -220,49 +242,4 @@ chrome.devtools.panels.elements.createSidebarPane('Style Macros', (sidebar) => { // Initial observation when the panel is first opened startObserving(); - - // Cleanup stale macro data every 5 minutes - const CLEANUP_INTERVAL = 1000 * 60 * 5; - setInterval(() => { - if (macroData.size === 0) { - return; - } - - debugLog('Running macro data cleanup, checking', macroData.size, 'macros...'); - const hashes = Array.from(macroData.keys()); - - // Check all hashes in a single eval for efficiency - const checkScript = ` - (function() { - const hashes = ${JSON.stringify(hashes)}; - const results = {}; - for (const hash of hashes) { - results[hash] = !!document.querySelector('.-macro-dynamic-' + hash); - } - return results; - })(); - `; - - chrome.devtools.inspectedWindow.eval(checkScript, (results, isException) => { - if (isException) { - debugLog('Error during cleanup:', results); - return; - } - - let removedCount = 0; - for (const hash in results) { - if (!results[hash]) { - debugLog('Removing stale macro:', hash); - macroData.delete(hash); - removedCount++; - } - } - - if (removedCount > 0) { - debugLog(`Cleaned up ${removedCount} stale macro(s). Remaining: ${macroData.size}`); - } else { - debugLog('No stale macros found.'); - } - }); - }, CLEANUP_INTERVAL); }); From 1c7980bc32210089f933eb0becd9846d91f6c711 Mon Sep 17 00:00:00 2001 From: Reid Barber Date: Tue, 17 Mar 2026 17:11:50 -0500 Subject: [PATCH 11/13] feat(codemod): improvements to s1-to-s2 codemod (agent mode, E2E tests, more) (#9754) * fixed TabList render function children kept Item instead of converting to Tab * import cleanup: removeComponentImport was only checking the first matching import and could miss v3 imports when an @react-spectrum/s2 import appears first * dynamic import('@react-spectrum/s2') was incorrectly flagged as a v3 dynamic import * Fixed in codemod options parsing * more import fixes * add agent mode (non-interactive, run transforms only) * add e2e tests * have --components include related components * yarn.lock * update outdated links * bump @adobe/react-spectrum version * fix missing imports and remove unused * fix snapshot * yarn.lock * map illustrations to s2 illustrations --- eslint.config.mjs | 5 +- packages/dev/codemods/package.json | 1 + packages/dev/codemods/src/index.ts | 36 ++- .../full-project/input/package.fixture.json | 7 + .../cli/full-project/input/src/App.tsx | 10 + .../cli/full-project/input/src/Form.tsx | 6 + .../cli/full-project/input/yarn.lock | 1 + .../full-project/output/package.fixture.json | 7 + .../cli/full-project/output/src/App.tsx | 6 + .../cli/full-project/output/src/Form.tsx | 6 + .../cli/full-project/output/yarn.lock | 1 + .../subset-project/input/package.fixture.json | 7 + .../cli/subset-project/input/src/Form.tsx | 11 + .../cli/subset-project/input/yarn.lock | 1 + .../output/package.fixture.json | 7 + .../cli/subset-project/output/src/Form.tsx | 10 + .../cli/subset-project/output/yarn.lock | 1 + .../__snapshots__/combobox.test.ts.snap | 2 +- .../__snapshots__/dialog.test.ts.snap | 46 ++- .../__snapshots__/dropzone.test.ts.snap | 4 +- .../__tests__/__snapshots__/form.test.ts.snap | 3 +- .../illustratedmessage.test.ts.snap | 19 +- .../__snapshots__/imports.test.ts.snap | 12 +- .../__tests__/__snapshots__/menu.test.ts.snap | 11 +- .../__snapshots__/picker.test.ts.snap | 2 +- .../__snapshots__/subset.test.ts.snap | 130 +++++++++ .../__tests__/__snapshots__/tabs.test.ts.snap | 38 ++- .../__tests__/__snapshots__/well.test.ts.snap | 3 +- .../src/s1-to-s2/__tests__/cli.e2e.test.ts | 264 ++++++++++++++++++ .../src/s1-to-s2/__tests__/dropzone.test.ts | 2 +- .../__tests__/illustratedmessage.test.ts | 15 +- .../src/s1-to-s2/__tests__/imports.test.ts | 7 + .../src/s1-to-s2/__tests__/subset.test.ts | 120 ++++++++ .../src/s1-to-s2/__tests__/tabs.test.ts | 40 +++ .../src/s1-to-s2/src/codemods/codemod.ts | 236 +++++++++++++++- .../components/ActionGroup/transform.ts | 8 +- .../ContextualHelpTrigger/transform.ts | 4 +- .../components/DialogTrigger/transform.ts | 22 +- .../src/codemods/components/Tabs/transform.ts | 19 +- .../codemods/illustrations/illustrationMap.ts | 11 + .../src/codemods/shared/transforms.ts | 9 +- .../src/s1-to-s2/src/codemods/shared/utils.ts | 116 ++++++-- .../src/s1-to-s2/src/getComponents.ts | 17 +- .../dev/codemods/src/s1-to-s2/src/index.ts | 32 ++- .../codemods/src/s1-to-s2/src/transform.ts | 9 +- packages/dev/codemods/tsconfig.json | 2 +- rules/README.md | 2 +- yarn.lock | 3 +- 48 files changed, 1190 insertions(+), 141 deletions(-) create mode 100644 packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/input/package.fixture.json create mode 100644 packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/input/src/App.tsx create mode 100644 packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/input/src/Form.tsx create mode 100644 packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/input/yarn.lock create mode 100644 packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/output/package.fixture.json create mode 100644 packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/output/src/App.tsx create mode 100644 packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/output/src/Form.tsx create mode 100644 packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/output/yarn.lock create mode 100644 packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/subset-project/input/package.fixture.json create mode 100644 packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/subset-project/input/src/Form.tsx create mode 100644 packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/subset-project/input/yarn.lock create mode 100644 packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/subset-project/output/package.fixture.json create mode 100644 packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/subset-project/output/src/Form.tsx create mode 100644 packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/subset-project/output/yarn.lock create mode 100644 packages/dev/codemods/src/s1-to-s2/__tests__/cli.e2e.test.ts create mode 100644 packages/dev/codemods/src/s1-to-s2/src/codemods/illustrations/illustrationMap.ts diff --git a/eslint.config.mjs b/eslint.config.mjs index febc4cebfdd..c3593ff37f4 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -60,7 +60,8 @@ export default [{ "packages/dev/storybook-builder-parcel/*", "packages/dev/storybook-react-parcel/*", "packages/dev/s2-docs/pages/**", - "packages/dev/mcp/*/dist" + "packages/dev/mcp/*/dist", + "packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/**" ], }, ...compat.extends("eslint:recommended"), { plugins: { @@ -534,4 +535,4 @@ export default [{ ...globals.browser } } -}]; \ No newline at end of file +}]; diff --git a/packages/dev/codemods/package.json b/packages/dev/codemods/package.json index 432b48facc1..5e7a927b54e 100644 --- a/packages/dev/codemods/package.json +++ b/packages/dev/codemods/package.json @@ -21,6 +21,7 @@ "url": "https://github.com/adobe/react-spectrum" }, "dependencies": { + "@adobe/react-spectrum": "^3.46.2", "@babel/parser": "^7.24.5", "@babel/traverse": "^7.24.5", "@babel/types": "^7.24.5", diff --git a/packages/dev/codemods/src/index.ts b/packages/dev/codemods/src/index.ts index 3bb350ba739..e6859e47d7d 100644 --- a/packages/dev/codemods/src/index.ts +++ b/packages/dev/codemods/src/index.ts @@ -35,7 +35,14 @@ export interface S1ToS2CodemodOptions extends JSCodeshiftOptions { * An optional subset of components to have the s1-to-s2 codemod apply to. * Provide a comma-separated list of component names. */ - components?: string + components?: string, + /** + * Whether to run the codemod in agent mode, which skips interactive prompts + * and package installation. This matches the shipped CLI behavior. + * + * @default false + */ + agent?: boolean } export interface UseMonopackagesCodemodOptions extends JSCodeshiftOptions { @@ -67,6 +74,9 @@ const options = { }, 'components': { type: 'string' + }, + 'agent': { + type: 'boolean' } }; @@ -80,22 +90,24 @@ if (positionals.length < 1) { process.exit(1); } -const codemodName = positionals[0]; -const codemodFunction = codemods[codemodName]; +async function main() { + const codemodName = positionals[0]; + const codemodFunction = codemods[codemodName]; -if (!codemodFunction) { - console.error(`Unknown codemod: ${codemodName}, available codemods: ${Object.keys(codemods).join(', ')}`); - process.exit(1); -} + if (!codemodFunction) { + console.error(`Unknown codemod: ${codemodName}, available codemods: ${Object.keys(codemods).join(', ')}`); + process.exit(1); + } -try { - codemodFunction({ + await Promise.resolve(codemodFunction({ parser: 'tsx', ignorePattern: '**/node_modules/**', path: '.', ...values - }); -} catch (error) { + })); +} + +main().catch((error) => { console.error(`Error running codemod: ${error}`); process.exit(1); -} +}); diff --git a/packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/input/package.fixture.json b/packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/input/package.fixture.json new file mode 100644 index 00000000000..edb334cd571 --- /dev/null +++ b/packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/input/package.fixture.json @@ -0,0 +1,7 @@ +{ + "name": "s1-to-s2-cli-fixture", + "private": true, + "devDependencies": { + "parcel": "^2.12.0" + } +} diff --git a/packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/input/src/App.tsx b/packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/input/src/App.tsx new file mode 100644 index 00000000000..52665e97a53 --- /dev/null +++ b/packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/input/src/App.tsx @@ -0,0 +1,10 @@ +import {Button} from '@adobe/react-spectrum'; +import React from 'react'; + +export function App() { + return ( + + ); +} diff --git a/packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/input/src/Form.tsx b/packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/input/src/Form.tsx new file mode 100644 index 00000000000..057d14955fb --- /dev/null +++ b/packages/dev/codemods/src/s1-to-s2/__testfixtures__/cli/full-project/input/src/Form.tsx @@ -0,0 +1,6 @@ +import React from 'react'; +import {TextArea} from '@adobe/react-spectrum'; + +export function Form() { + return