diff --git a/packages/@react-aria/focus/src/useFocusRing.ts b/packages/@react-aria/focus/src/useFocusRing.ts index 50cf8258115..e8c98c4dfcd 100644 --- a/packages/@react-aria/focus/src/useFocusRing.ts +++ b/packages/@react-aria/focus/src/useFocusRing.ts @@ -51,6 +51,7 @@ export function useFocusRing(props: AriaFocusRingProps = {}): FocusRingAria { let onFocusChange = useCallback(isFocused => { state.current.isFocused = isFocused; + state.current.isFocusVisible = isFocusVisible(); setFocused(isFocused); updateState(); }, [updateState]); @@ -58,7 +59,7 @@ export function useFocusRing(props: AriaFocusRingProps = {}): FocusRingAria { useFocusVisibleListener((isFocusVisible) => { state.current.isFocusVisible = isFocusVisible; updateState(); - }, [], {isTextInput}); + }, [isTextInput, isFocused], {enabled: isFocused, isTextInput}); let {focusProps} = useFocus({ isDisabled: within, diff --git a/packages/@react-aria/interactions/src/useFocusVisible.ts b/packages/@react-aria/interactions/src/useFocusVisible.ts index 80ac9c0940c..07626782dea 100644 --- a/packages/@react-aria/interactions/src/useFocusVisible.ts +++ b/packages/@react-aria/interactions/src/useFocusVisible.ts @@ -39,7 +39,7 @@ export interface FocusVisibleResult { let currentModality: null | Modality = null; let currentPointerType: PointerType = 'keyboard'; -let changeHandlers = new Set(); +export const changeHandlers = new Set(); interface GlobalListenerData { focus: () => void } @@ -333,10 +333,13 @@ export function useFocusVisible(props: FocusVisibleProps = {}): FocusVisibleResu /** * Listens for trigger change and reports if focus is visible (i.e., modality is not pointer). */ -export function useFocusVisibleListener(fn: FocusVisibleHandler, deps: ReadonlyArray, opts?: {isTextInput?: boolean}): void { +export function useFocusVisibleListener(fn: FocusVisibleHandler, deps: ReadonlyArray, opts?: {enabled?: boolean, isTextInput?: boolean}): void { setupGlobalFocusEvents(); useEffect(() => { + if (opts?.enabled === false) { + return; + } let handler = (modality: Modality, e: HandlerEvent) => { // We want to early return for any keyboard events that occur inside text inputs EXCEPT for Tab and Escape if (!isKeyboardFocusEvent(!!(opts?.isTextInput), modality, e)) { @@ -351,3 +354,4 @@ export function useFocusVisibleListener(fn: FocusVisibleHandler, deps: ReadonlyA // eslint-disable-next-line react-hooks/exhaustive-deps }, deps); } + diff --git a/packages/@react-aria/interactions/test/useFocusVisible.test.js b/packages/@react-aria/interactions/test/useFocusVisible.test.js index 8cb554b70d1..4bca6e5705d 100644 --- a/packages/@react-aria/interactions/test/useFocusVisible.test.js +++ b/packages/@react-aria/interactions/test/useFocusVisible.test.js @@ -11,7 +11,7 @@ */ import {act, fireEvent, pointerMap, render, renderHook, screen, waitFor} from '@react-spectrum/test-utils-internal'; import {addWindowFocusTracking, useFocusVisible, useFocusVisibleListener} from '../'; -import {hasSetupGlobalListeners} from '../src/useFocusVisible'; +import {changeHandlers, hasSetupGlobalListeners} from '../src/useFocusVisible'; import {mergeProps} from '@react-aria/utils'; import React from 'react'; import {useButton} from '@react-aria/button'; @@ -65,7 +65,7 @@ describe('useFocusVisible', function () { beforeAll(() => { user = userEvent.setup({delay: null, pointerMap}); }); - + beforeEach(() => { fireEvent.focus(document.body); }); @@ -84,7 +84,7 @@ describe('useFocusVisible', function () { render(); await user.tab(); let el = screen.getByText('example-focusVisible'); - + await user.click(el); toggleBrowserTabs(); @@ -165,7 +165,7 @@ describe('useFocusVisible', function () { await user.click(el); expect(el.textContent).toBe('example'); - + // Focus events after beforeunload no longer work fireEvent(iframe.contentWindow, new Event('beforeunload')); await user.keyboard('{Enter}'); @@ -355,4 +355,102 @@ describe('useFocusVisibleListener', function () { expect(fnMock).toHaveBeenCalledTimes(3); expect(fnMock.mock.calls).toEqual([[true], [true], [false]]); }); + + describe('subscription model', function () { + let user; + beforeAll(() => { + user = userEvent.setup({delay: null, pointerMap}); + }); + + function Example(props) { + return ( +
+ + +
+ ); + } + + function ButtonExample(props) { + const ref = React.useRef(null); + const {buttonProps} = useButton({}, ref); + const {focusProps, isFocusVisible} = useFocusRing(); + + return ; + } + it('does not call changeHandlers when unneeded', async function () { + // Save original methods + const originalAdd = changeHandlers.add.bind(changeHandlers); + const originalDelete = changeHandlers.delete.bind(changeHandlers); + // Map so we can also track references to the original handlers to remove them later + const handlerSpies = new Map(); + + // Intercept handler registration and wrap with spy + changeHandlers.add = function (handler) { + const spy = jest.fn(handler); + handlerSpies.set(handler, spy); + return originalAdd.call(this, spy); + }; + + changeHandlers.delete = function (handler) { + const spy = handlerSpies.get(handler); + if (spy) { + handlerSpies.delete(handler); + return originalDelete.call(this, spy); + } + return originalDelete.call(this, handler); + }; + + // Possibly a little extra cautious with the unmount, but better safe than sorry with cleanup. + let {getByTestId, unmount} = render(); + + let button1 = getByTestId('button1'); + let button2 = getByTestId('button2'); + expect(button1).not.toHaveAttribute('data-focus-visible'); + expect(button2).not.toHaveAttribute('data-focus-visible'); + // No handlers registered yet because nothing is focused + expect(handlerSpies.size).toBe(0); + + // Tab to first button, this should add its handler + await user.tab(); + expect(document.activeElement).toBe(button1); + expect(button1).toHaveAttribute('data-focus-visible'); + expect(button2).not.toHaveAttribute('data-focus-visible'); + + expect(handlerSpies.size).toBe(1); + let [button1Spy] = [...handlerSpies.values()]; + + button1Spy.mockClear(); + + // Tab to second button - first handler should be removed, second added + await user.tab(); + expect(button1).not.toHaveAttribute('data-focus-visible'); + expect(button2).toHaveAttribute('data-focus-visible'); + + // Still only 1 handler registered (swapped from button1 to button2) + expect(handlerSpies.size).toBe(1); + let [button2Spy] = [...handlerSpies.values()]; + expect(button2Spy).not.toBe(button1Spy); // Should be a different spy + + // button1's handler was called during tab (keydown/keyup before removal) + // the handler is removed later in an effect + expect(button1Spy.mock.calls.length).toBeGreaterThan(0); + + button1Spy.mockClear(); + button2Spy.mockClear(); + + // After button1's handler is removed, it should NOT be called + // for subsequent modality changes - only button2's handler should be called + await user.click(button2); + expect(button1).not.toHaveAttribute('data-focus-visible'); + expect(button2).not.toHaveAttribute('data-focus-visible'); + expect(button1Spy).toHaveBeenCalledTimes(0); // button1's handler should NOT be called + expect(button2Spy).toHaveBeenCalledTimes(1); // Only button2's handler called to change modality to pointer + + // Cleanup + unmount(); + changeHandlers.add = originalAdd; + changeHandlers.delete = originalDelete; + }); + }); }); diff --git a/packages/dev/s2-docs/pages/react-aria/GridList.mdx b/packages/dev/s2-docs/pages/react-aria/GridList.mdx index 7dad11de248..50be2017586 100644 --- a/packages/dev/s2-docs/pages/react-aria/GridList.mdx +++ b/packages/dev/s2-docs/pages/react-aria/GridList.mdx @@ -564,8 +564,8 @@ import {GridListHeader, GridListSection, Text} from 'react-aria-components'; - Vegetables - + Vegetables + Broccoli PNG • 5/30/2023 diff --git a/packages/dev/s2-docs/src/MobileHeader.tsx b/packages/dev/s2-docs/src/MobileHeader.tsx index 9b7815cf00f..e013f5edfb2 100644 --- a/packages/dev/s2-docs/src/MobileHeader.tsx +++ b/packages/dev/s2-docs/src/MobileHeader.tsx @@ -344,7 +344,7 @@ export function MobileHeader({toc}) { width: 'full', height: '--visual-viewport-height' })}> - + diff --git a/packages/dev/s2-docs/src/SearchMenuWrapper.tsx b/packages/dev/s2-docs/src/SearchMenuWrapper.tsx index 9dd91c52098..10b02ac075b 100644 --- a/packages/dev/s2-docs/src/SearchMenuWrapper.tsx +++ b/packages/dev/s2-docs/src/SearchMenuWrapper.tsx @@ -23,7 +23,7 @@ export default function SearchMenuWrapper({children}: {children: ReactNode}) { {children} - + diff --git a/packages/react-aria-components/src/Table.tsx b/packages/react-aria-components/src/Table.tsx index 96836b8f5e8..81b6e4dbebd 100644 --- a/packages/react-aria-components/src/Table.tsx +++ b/packages/react-aria-components/src/Table.tsx @@ -1321,7 +1321,11 @@ export interface CellRenderProps { /** * The unique id of the cell. **/ - id?: Key + id?: Key, + /** + * The index of the column that this cell belongs to. Respects col spanning. + */ + colIndex?: number | null } export interface CellProps extends RenderProps, GlobalDOMAttributes { @@ -1369,6 +1373,8 @@ export const Cell = /*#__PURE__*/ createLeafComponent(TableCellNode, (props: Cel let {isFocused, isFocusVisible, focusProps} = useFocusRing(); let {hoverProps, isHovered} = useHover({}); let isSelected = cell.parentKey != null ? state.selectionManager.isSelected(cell.parentKey) : false; + // colIndex is null, when there is so span, falling back to using the index + let colIndex = cell.colIndex || cell.index; let renderProps = useRenderProps({ ...props, @@ -1380,7 +1386,8 @@ export const Cell = /*#__PURE__*/ createLeafComponent(TableCellNode, (props: Cel isPressed, isHovered, isSelected, - id: cell.key + id: cell.key, + colIndex } }); @@ -1394,7 +1401,8 @@ export const Cell = /*#__PURE__*/ createLeafComponent(TableCellNode, (props: Cel data-focused={isFocused || undefined} data-focus-visible={isFocusVisible || undefined} data-pressed={isPressed || undefined} - data-selected={isSelected || undefined}> + data-selected={isSelected || undefined} + data-col-index={colIndex}> {renderProps.children} diff --git a/packages/react-aria-components/test/Table.test.js b/packages/react-aria-components/test/Table.test.js index 127e9b21222..c1a502e6d37 100644 --- a/packages/react-aria-components/test/Table.test.js +++ b/packages/react-aria-components/test/Table.test.js @@ -858,6 +858,104 @@ describe('Table', () => { expect(cells[0]).toHaveTextContent('Foo (focused)'); }); + it('should support column index in render props', () => { + let {getAllByRole} = render( + + + Name + Type + Price + Total + + + + + {({colIndex}) => `cell index: ${colIndex}`} + + + {({colIndex}) => `cell index: ${colIndex}`} + + + {({colIndex}) => `cell index: ${colIndex}`} + + + {({colIndex}) => `cell index: ${colIndex}`} + + + +
+ ); + + let cells = getAllByRole('rowheader'); + expect(cells[0]).toHaveTextContent('cell index: 0'); + expect(cells[1]).toHaveTextContent('cell index: 1'); + expect(cells[2]).toHaveTextContent('cell index: 2'); + expect(cells[3]).toHaveTextContent('cell index: 3'); + expect(cells[0]).toHaveAttribute('data-col-index', '0'); + expect(cells[1]).toHaveAttribute('data-col-index', '1'); + expect(cells[2]).toHaveAttribute('data-col-index', '2'); + expect(cells[3]).toHaveAttribute('data-col-index', '3'); + }); + + it('should support colspan with cell index', () => { + let {getAllByRole} = render( + + + Name + Type + Price + Total + + + + + {({colIndex}) => `cell index: ${colIndex}`} + + + {({colIndex}) => `cell index: ${colIndex}`} + + + {({colIndex}) => `cell index: ${colIndex}`} + + + + + {({colIndex}) => `cell index: ${colIndex}`} + + + {({colIndex}) => `cell index: ${colIndex}`} + + + {({colIndex}) => `cell index: ${colIndex}`} + + + {({colIndex}) => `cell index: ${colIndex}`} + + + +
+ ); + + let cells = getAllByRole('rowheader'); + // first row + expect(cells[0]).toHaveTextContent('cell index: 0'); + expect(cells[1]).toHaveTextContent('cell index: 2'); + expect(cells[2]).toHaveTextContent('cell index: 3'); + expect(cells[0]).toHaveAttribute('data-col-index', '0'); + expect(cells[1]).toHaveAttribute('data-col-index', '2'); + expect(cells[2]).toHaveAttribute('data-col-index', '3'); + + // second row + expect(cells[3]).toHaveTextContent('cell index: 0'); + expect(cells[4]).toHaveTextContent('cell index: 1'); + expect(cells[5]).toHaveTextContent('cell index: 2'); + expect(cells[6]).toHaveTextContent('cell index: 3'); + expect(cells[3]).toHaveAttribute('data-col-index', '0'); + expect(cells[4]).toHaveAttribute('data-col-index', '1'); + expect(cells[5]).toHaveAttribute('data-col-index', '2'); + expect(cells[6]).toHaveAttribute('data-col-index', '3'); + }); + it('should support columnHeader typeahead', async () => { let {getAllByRole} = render( @@ -1321,13 +1419,13 @@ describe('Table', () => { expect(document.activeElement).toHaveAttribute('aria-label', 'Insert between Adobe Photoshop and Adobe XD'); await user.tab(); expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on'); - + const labels = ['Pictures', 'Adobe Fresco', 'Apps', 'Adobe Illustrator', 'Adobe Lightroom', 'Adobe Dreamweaver']; - + for (let i = 0; i <= labels.length; i++) { fireEvent.keyDown(document.activeElement, {key: 'ArrowDown'}); fireEvent.keyUp(document.activeElement, {key: 'ArrowDown'}); - + if (i === 0) { expect(document.activeElement).toHaveAttribute('aria-label', `Insert before ${labels[i]}`); } else if (i === labels.length) { @@ -1336,14 +1434,14 @@ describe('Table', () => { expect(document.activeElement).toHaveAttribute('aria-label', `Insert between ${labels[i - 1]} and ${labels[i]}`); } } - + await user.keyboard('{Home}'); expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on'); - + for (let i = labels.length; i >= 0; i--) { fireEvent.keyDown(document.activeElement, {key: 'ArrowUp'}); fireEvent.keyUp(document.activeElement, {key: 'ArrowUp'}); - + if (i === 0) { expect(document.activeElement).toHaveAttribute('aria-label', `Insert before ${labels[i]}`); } else if (i === labels.length) { @@ -1352,7 +1450,7 @@ describe('Table', () => { expect(document.activeElement).toHaveAttribute('aria-label', `Insert between ${labels[i - 1]} and ${labels[i]}`); } } - + await user.keyboard('{End}'); expect(document.activeElement).toHaveAttribute('aria-label', 'Insert after Adobe Dreamweaver'); await user.keyboard('{ArrowDown}'); @@ -2696,7 +2794,7 @@ describe('Table', () => { let {getByRole} = renderTable({rowProps: {onAction, onPressStart, onPressEnd, onPress, onClick}}); let tableTester = testUtilUser.createTester('Table', {root: getByRole('grid')}); await tableTester.triggerRowAction({row: 1, interactionType}); - + expect(onAction).toHaveBeenCalledTimes(1); expect(onPressStart).toHaveBeenCalledTimes(1); expect(onPressEnd).toHaveBeenCalledTimes(1); diff --git a/starters/docs/src/GridList.css b/starters/docs/src/GridList.css index 91d8f407e79..78b5f0ae1d5 100644 --- a/starters/docs/src/GridList.css +++ b/starters/docs/src/GridList.css @@ -16,6 +16,10 @@ box-sizing: border-box; --grid-item-size: 200px; + &:has(.react-aria-GridListSection) { + scroll-padding: 50px; + } + &[data-layout=grid]:not(:has(.react-aria-GridListSection)) { display: grid; grid-template-columns: repeat(auto-fit, minmax(100px, var(--grid-item-size))); @@ -266,7 +270,7 @@ font-size: var(--font-size-lg); font-weight: 500; background: var(--gray-100); - border-block: 0.5px solid var(--gray-400); + border: 0.5px solid var(--gray-400); cursor: default; user-select: none; box-shadow: inset 0px 1px 0px white, inset 0px -4px 8px var(--gray-200); diff --git a/starters/docs/src/ListBox.css b/starters/docs/src/ListBox.css index dc9674b5470..8e06d22d065 100644 --- a/starters/docs/src/ListBox.css +++ b/starters/docs/src/ListBox.css @@ -12,6 +12,10 @@ font: var(--font-size) system-ui; color: var(--text-color); + &:has(.react-aria-ListBoxSection){ + scroll-padding: 24px; + } + &[data-focus-visible] { outline: 2px solid var(--focus-ring-color); outline-offset: -1px; diff --git a/starters/docs/stories/ListBox.stories.tsx b/starters/docs/stories/ListBox.stories.tsx index 565072638f5..4fd19000021 100644 --- a/starters/docs/stories/ListBox.stories.tsx +++ b/starters/docs/stories/ListBox.stories.tsx @@ -1,4 +1,5 @@ -import {ListBox, ListBoxItem} from '../src/ListBox'; +import {ListBox, ListBoxItem, ListBoxSection} from '../src/ListBox'; +import {Header} from 'react-aria-components'; import type {Meta, StoryFn} from '@storybook/react'; @@ -26,3 +27,26 @@ Example.args = { onAction: undefined, selectionMode: 'single' }; + +export const Sections: Story = (args) => ( + + +
Veggies
+ Lettuce + Tomato + Onion + + +
Protein
+ Ham + Tuna + Tofu + + +
Condiments
+ Mayonaise + Mustard + Ranch + + +)