Skip to content

Commit

Permalink
Fix synthetic links not using useHref (#6346)
Browse files Browse the repository at this point in the history
  • Loading branch information
joshuajaco authored Jun 12, 2024
1 parent 91f63a6 commit a4dee6e
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 11 deletions.
5 changes: 3 additions & 2 deletions packages/@react-aria/gridlist/src/useGridListItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* governing permissions and limitations under the License.
*/

import {chain, getScrollParent, getSyntheticLinkProps, mergeProps, scrollIntoViewport, useSlotId} from '@react-aria/utils';
import {chain, getScrollParent, mergeProps, scrollIntoViewport, useSlotId, useSyntheticLinkProps} from '@react-aria/utils';
import {DOMAttributes, FocusableElement, Node as RSNode} from '@react-types/shared';
import {focusSafely, getFocusableTreeWalker} from '@react-aria/focus';
import {getLastItem} from '@react-stately/collections';
Expand Down Expand Up @@ -242,7 +242,8 @@ export function useGridListItem<T>(props: AriaGridListItemOptions, state: ListSt
}
};

let linkProps = itemStates.hasAction ? getSyntheticLinkProps(node.props) : {};
let syntheticLinkProps = useSyntheticLinkProps(node.props);
let linkProps = itemStates.hasAction ? syntheticLinkProps : {};
// TODO: re-add when we get translations and fix this for iOS VO
// let rowAnnouncement;
// if (onAction) {
Expand Down
5 changes: 3 additions & 2 deletions packages/@react-aria/table/src/useTableRow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@
import {FocusableElement} from '@react-types/shared';
import {getLastItem} from '@react-stately/collections';
import {getRowLabelledBy} from './utils';
import {getSyntheticLinkProps, mergeProps} from '@react-aria/utils';
import type {GridNode} from '@react-types/grid';
import {GridRowAria, GridRowProps, useGridRow} from '@react-aria/grid';
import {HTMLAttributes, RefObject} from 'react';
import {mergeProps, useSyntheticLinkProps} from '@react-aria/utils';
import {TableCollection} from '@react-types/table';
import {tableNestedRows} from '@react-stately/flags';
import {TableState, TreeGridState} from '@react-stately/table';
Expand Down Expand Up @@ -74,7 +74,8 @@ export function useTableRow<T>(props: GridRowProps<T>, state: TableState<T> | Tr
}
}

let linkProps = states.hasAction ? getSyntheticLinkProps(node.props) : {};
let syntheticLinkProps = useSyntheticLinkProps(node.props);
let linkProps = states.hasAction ? syntheticLinkProps : {};
return {
rowProps: {
...mergeProps(rowProps, treeGridRowProps, linkProps),
Expand Down
4 changes: 2 additions & 2 deletions packages/@react-aria/tag/src/useTag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import {AriaButtonProps} from '@react-types/button';
import {DOMAttributes, FocusableElement, Node} from '@react-types/shared';
import {filterDOMProps, getSyntheticLinkProps, mergeProps, useDescription, useId} from '@react-aria/utils';
import {filterDOMProps, mergeProps, useDescription, useId, useSyntheticLinkProps} from '@react-aria/utils';
import {hookData} from './useTagGroup';
// @ts-ignore
import intlMessages from '../intl/*.json';
Expand Down Expand Up @@ -82,7 +82,7 @@ export function useTag<T>(props: AriaTagProps<T>, state: ListState<T>, ref: RefO
let isFocused = item.key === state.selectionManager.focusedKey;
// @ts-ignore - data attributes are ok but TS doesn't know about them.
let domProps = filterDOMProps(item.props);
let linkProps = getSyntheticLinkProps(item.props);
let linkProps = useSyntheticLinkProps(item.props);
return {
removeButtonProps: {
'aria-label': stringFormatter.format('removeButtonLabel'),
Expand Down
2 changes: 1 addition & 1 deletion packages/@react-aria/utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export {mergeRefs} from './mergeRefs';
export {filterDOMProps} from './filterDOMProps';
export {focusWithoutScrolling} from './focusWithoutScrolling';
export {getOffset} from './getOffset';
export {openLink, getSyntheticLinkProps, RouterProvider, shouldClientNavigate, useRouter, useLinkProps} from './openLink';
export {openLink, useSyntheticLinkProps, RouterProvider, shouldClientNavigate, useRouter, useLinkProps} from './openLink';
export {runAfterTransition} from './runAfterTransition';
export {useDrag1D} from './useDrag1D';
export {useGlobalListeners} from './useGlobalListeners';
Expand Down
5 changes: 3 additions & 2 deletions packages/@react-aria/utils/src/openLink.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,10 @@ function openSyntheticLink(target: Element, modifiers: Modifiers) {
getSyntheticLink(target, link => openLink(link, modifiers));
}

export function getSyntheticLinkProps(props: LinkDOMProps) {
export function useSyntheticLinkProps(props: LinkDOMProps) {
let router = useRouter();
return {
'data-href': props.href,
'data-href': props.href ? router.useHref(props.href) : undefined,
'data-target': props.target,
'data-rel': props.rel,
'data-download': props.download,
Expand Down
28 changes: 27 additions & 1 deletion packages/react-aria-components/test/GridList.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,16 @@
*/

import {act, fireEvent, mockClickDefault, pointerMap, render, within} from '@react-spectrum/test-utils-internal';
import {Button, Checkbox, DropIndicator, GridList, GridListContext, GridListItem, useDragAndDrop} from '../';
import {
Button,
Checkbox,
DropIndicator,
GridList,
GridListContext,
GridListItem,
RouterProvider,
useDragAndDrop
} from '../';
import React from 'react';
import userEvent from '@testing-library/user-event';

Expand Down Expand Up @@ -522,6 +531,23 @@ describe('GridList', () => {
expect(onClick.mock.calls[0][0].target).toBeInstanceOf(HTMLAnchorElement);
expect(onClick.mock.calls[0][0].target.href).toBe('https://google.com/');
});

it('should work with RouterProvider', async () => {
let navigate = jest.fn();
let useHref = href => '/base' + href;
let {getAllByRole} = render(
<RouterProvider navigate={navigate} useHref={useHref}>
<GridList aria-label="listview">
<GridListItem href="/foo" routerOptions={{foo: 'bar'}}>One</GridListItem>
</GridList>
</RouterProvider>
);

let items = getAllByRole('row');
expect(items[0]).toHaveAttribute('data-href', '/base/foo');
await trigger(items[0]);
expect(navigate).toHaveBeenCalledWith('/foo', {foo: 'bar'});
});
});
});
});
22 changes: 21 additions & 1 deletion packages/react-aria-components/test/TagGroup.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
* governing permissions and limitations under the License.
*/

import {Button, Label, Tag, TagGroup, TagList, Text} from '../';
import {Button, Label, RouterProvider, Tag, TagGroup, TagList, Text} from '../';
import {fireEvent, mockClickDefault, pointerMap, render} from '@react-spectrum/test-utils-internal';
import React from 'react';
import userEvent from '@testing-library/user-event';
Expand Down Expand Up @@ -329,6 +329,26 @@ describe('TagGroup', () => {
document.removeEventListener('click', onClick);
}
});

it('should work with RouterProvider', async () => {
let navigate = jest.fn();
let useHref = href => '/base' + href;
let {getAllByRole} = render(
<RouterProvider navigate={navigate} useHref={useHref}>
<TagGroup selectionMode="none">
<Label>Tags</Label>
<TagList>
<Tag href="/foo" routerOptions={{foo: 'bar'}}>One</Tag>
</TagList>
</TagGroup>
</RouterProvider>
);

let items = getAllByRole('row');
expect(items[0]).toHaveAttribute('data-href', '/base/foo');
await trigger(items[0]);
expect(navigate).toHaveBeenCalledWith('/foo', {foo: 'bar'});
});
});
});
});

1 comment on commit a4dee6e

@rspbot
Copy link

@rspbot rspbot commented on a4dee6e Jun 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.