From 7b0fc7019e2289b16edbbe08f820a890bc573efd Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 15 Jan 2025 17:10:12 +1100 Subject: [PATCH 01/16] chore: Update TreeView API --- packages/@react-spectrum/s2/src/TreeView.tsx | 148 ++++++------ packages/@react-spectrum/s2/src/index.ts | 2 +- .../s2/stories/TreeView.stories.tsx | 186 ++++++++------- .../@react-spectrum/s2/test/Tree.test.tsx | 212 ++++++++++++------ .../tree/chromatic-fc/TreeView.stories.tsx | 114 +++++----- .../tree/chromatic/TreeView.stories.tsx | 118 +++++----- .../@react-spectrum/tree/src/TreeView.tsx | 131 +++++------ packages/@react-spectrum/tree/src/index.ts | 2 +- .../tree/stories/TreeView.stories.tsx | 209 +++++++++-------- .../tree/test/TreeView.test.tsx | 195 ++++++++++------ .../react-aria-components/test/Tree.test.tsx | 4 +- 11 files changed, 751 insertions(+), 570 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 639678a7e0b..5a6e2e2261d 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -15,10 +15,10 @@ import {ActionMenuContext} from './ActionMenu'; import { Button, ButtonContext, - Collection, Provider, TreeItemProps as RACTreeItemProps, TreeProps as RACTreeProps, + TreeItemContentProps, UNSTABLE_ListLayout, UNSTABLE_Tree, UNSTABLE_TreeItem, @@ -35,8 +35,8 @@ import {getAllowedOverrides, StylesPropWithHeight, UnsafeStyles} from './style-u import {IconContext} from './Icon'; import {isAndroid} from '@react-aria/utils'; import {raw} from '../style/style-macro' with {type: 'macro'}; -import React, {createContext, forwardRef, isValidElement, JSXElementConstructor, ReactElement, useContext, useMemo, useRef} from 'react'; -import {Text, TextContext} from './Content'; +import React, {createContext, forwardRef, JSXElementConstructor, ReactElement, ReactNode, useContext, useMemo, useRef} from 'react'; +import {TextContext} from './Content'; import {useDOMRef} from '@react-spectrum/utils'; import {useLocale} from 'react-aria'; @@ -65,10 +65,6 @@ interface TreeRendererContextValue { } const TreeRendererContext = createContext({}); -function useTreeRendererContext(): TreeRendererContextValue { - return useContext(TreeRendererContext)!; -} - let InternalTreeContext = createContext<{isDetached?: boolean, isEmphasized?: boolean}>({}); @@ -343,91 +339,79 @@ const treeRowFocusIndicator = raw(` }` ); +let TreeItemContext = createContext<{hasChildItems?: boolean}>({}); export const TreeViewItem = (props: TreeViewItemProps) => { let { - children, - childItems, - hasChildItems, href } = props; - - let content; - let nestedRows; - let {renderer} = useTreeRendererContext(); let {isDetached, isEmphasized} = useContext(InternalTreeContext); - if (typeof children === 'string') { - content = {children}; - } else { - content = []; - nestedRows = []; - React.Children.forEach(children, node => { - if (isValidElement(node) && node.type === TreeViewItem) { - nestedRows.push(node); - } else { - content.push(node); - } - }); - } + return ( + // TODO right now all the tree rows have the various data attributes applied on their dom nodes, should they? Doesn't feel very useful + + treeRow({ + ...renderProps, + isLink: !!href, isEmphasized + }) + (renderProps.isFocusVisible && !isDetached ? ' ' + treeRowFocusIndicator : '')} /> + + ); +}; - if (childItems != null && renderer) { - nestedRows = ( - - {renderer} - - ); - } +export const TreeItemContent = (props: Omit & {children: ReactNode}) => { + let { + children + } = props; + let {isDetached, isEmphasized} = useContext(InternalTreeContext); + let {hasChildItems} = useContext(TreeItemContext); return ( - treeRow({...renderProps, isLink: !!href, isEmphasized}) + (renderProps.isFocusVisible && !isDetached ? ' ' + treeRowFocusIndicator : '')}> - - {({isExpanded, hasChildRows, selectionMode, selectionBehavior, isDisabled, isFocusVisible, isSelected, id, state}) => { - let isNextSelected = false; - let isNextFocused = false; - let keyAfter = state.collection.getKeyAfter(id); - if (keyAfter != null) { - isNextSelected = state.selectionManager.isSelected(keyAfter); - } - let isFirst = state.collection.getFirstKey() === id; - return ( -
- {selectionMode !== 'none' && selectionBehavior === 'toggle' && ( - // TODO: add transition? -
- -
- )} -
- {/* TODO: revisit when we do async loading, at the moment hasChildItems will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} - {(hasChildRows || hasChildItems) && } - - {content} - - {isFocusVisible && isDetached &&
} -
- ); - }} - - {nestedRows} - + + {({isExpanded, hasChildRows, selectionMode, selectionBehavior, isDisabled, isFocusVisible, isSelected, id, state}) => { + let isNextSelected = false; + let isNextFocused = false; + let keyAfter = state.collection.getKeyAfter(id); + if (keyAfter != null) { + isNextSelected = state.selectionManager.isSelected(keyAfter); + } + let isFirst = state.collection.getFirstKey() === id; + return ( +
+ {selectionMode !== 'none' && selectionBehavior === 'toggle' && ( + // TODO: add transition? +
+ +
+ )} +
+ {/* TODO: revisit when we do async loading, at the moment hasChildItems will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} + {(hasChildRows || hasChildItems) && } + + {children} + + {isFocusVisible && isDetached &&
} +
+ ); + }} + ); }; diff --git a/packages/@react-spectrum/s2/src/index.ts b/packages/@react-spectrum/s2/src/index.ts index 3e84f881344..52c6cf7fce0 100644 --- a/packages/@react-spectrum/s2/src/index.ts +++ b/packages/@react-spectrum/s2/src/index.ts @@ -76,7 +76,7 @@ export {TextArea, TextField, TextAreaContext, TextFieldContext} from './TextFiel export {ToggleButton, ToggleButtonContext} from './ToggleButton'; export {ToggleButtonGroup, ToggleButtonGroupContext} from './ToggleButtonGroup'; export {Tooltip, TooltipTrigger} from './Tooltip'; -export {TreeView, TreeViewItem} from './TreeView'; +export {TreeView, TreeViewItem, TreeItemContent} from './TreeView'; export {pressScale} from './pressScale'; diff --git a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx index 326775797a6..e272ae1d5c4 100644 --- a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx +++ b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx @@ -13,12 +13,14 @@ import {action} from '@storybook/addon-actions'; import { ActionMenu, + Collection, Content, Heading, IllustratedMessage, Link, MenuItem, Text, + TreeItemContent, TreeView, TreeViewItem } from '../src'; @@ -77,34 +79,24 @@ const TreeExampleStatic = (args) => ( onExpandedChange={action('onExpandedChange')} onSelectionChange={action('onSelectionChange')}> - Photos - - - - - Edit - - - - Delete - - + + Photos + + + + + Edit + + + + Delete + + + - Projects - - - - - Edit - - - - Delete - - - - Projects-1 + + Projects @@ -116,9 +108,11 @@ const TreeExampleStatic = (args) => ( Delete - - Projects-1A - + + + + Projects-1 + @@ -129,35 +123,55 @@ const TreeExampleStatic = (args) => ( Delete + + + + Projects-1A + + + + + Edit + + + + Delete + + + - Projects-2 - - - - - Edit - - - - Delete - - + + Projects-2 + + + + + Edit + + + + Delete + + + - Projects-3 - - - - - Edit - - - - Delete - - + + Projects-3 + + + + + Edit + + + + Delete + + + @@ -202,13 +216,14 @@ let rows = [ ]} ]; -const TreeExampleDynamic = (args) => ( -
- - {(item: any) => ( - - {item.name} - {item.icon} +const DynamicTreeItem = (props) => { + let {childItems, name, icon} = props; + return ( + <> + + + {name} + {icon} @@ -219,7 +234,35 @@ const TreeExampleDynamic = (args) => ( Delete - + + + {(item: any) => ( + + {item.name} + + )} + + + + ); +}; + +const TreeExampleDynamic = (args) => ( +
+ + {(item: any) => ( + )}
@@ -259,20 +302,13 @@ const TreeExampleWithLinks = (args) => (
{(item) => ( - - {item.name} - {item.icon} - - - - Edit - - - - Delete - - - + )}
diff --git a/packages/@react-spectrum/s2/test/Tree.test.tsx b/packages/@react-spectrum/s2/test/Tree.test.tsx index 5b9f4aa1cff..0023e3e28bd 100644 --- a/packages/@react-spectrum/s2/test/Tree.test.tsx +++ b/packages/@react-spectrum/s2/test/Tree.test.tsx @@ -6,58 +6,94 @@ import React from 'react'; import {render} from '@react-spectrum/test-utils-internal'; import { Text, + TreeItemContent, TreeView, TreeViewItem } from '../src'; AriaTreeTests({ prefix: 'spectrum2-static', + setup: () => { + let offsetWidth, offsetHeight; + + beforeAll(function () { + offsetWidth = jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 1000); + offsetHeight = jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 1000); + }); + + afterAll(function () { + offsetWidth.mockReset(); + offsetHeight.mockReset(); + }); + }, renderers: { // todo - we don't support isDisabled on TreeViewItems? standard: () => render( - Photos - + + Photos + + - Projects - - - Projects-1 + + Projects + + + + Projects-1 + + - Projects-1A - + + Projects-1A + + - Projects-2 - + + Projects-2 + + - Projects-3 - + + Projects-3 + + - School - - - Homework-1 + + School + + + + Homework-1 + + - Homework-1A - + + Homework-1A + + - Homework-2 - + + Homework-2 + + - Homework-3 - + + Homework-3 + + @@ -65,47 +101,69 @@ AriaTreeTests({ singleSelection: () => render( - Photos - + + Photos + + - Projects - - - Projects-1 + + Projects + + + + Projects-1 + + - Projects-1A - + + Projects-1A + + - Projects-2 - + + Projects-2 + + - Projects-3 - + + Projects-3 + + - School - - - Homework-1 + + School + + + + Homework-1 + + - Homework-1A - + + Homework-1A + + - Homework-2 - + + Homework-2 + + - Homework-3 - + + Homework-3 + + @@ -113,47 +171,69 @@ AriaTreeTests({ allInteractionsDisabled: () => render( - Photos - + + Photos + + - Projects - - - Projects-1 + + Projects + + + + Projects-1 + + - Projects-1A - + + Projects-1A + + - Projects-2 - + + Projects-2 + + - Projects-3 - + + Projects-3 + + - School - - - Homework-1 + + School + + + + Homework-1 + + - Homework-1A - + + Homework-1A + + - Homework-2 - + + Homework-2 + + - Homework-3 - + + Homework-3 + + diff --git a/packages/@react-spectrum/tree/chromatic-fc/TreeView.stories.tsx b/packages/@react-spectrum/tree/chromatic-fc/TreeView.stories.tsx index 9e5ea417c39..19728d0823c 100644 --- a/packages/@react-spectrum/tree/chromatic-fc/TreeView.stories.tsx +++ b/packages/@react-spectrum/tree/chromatic-fc/TreeView.stories.tsx @@ -19,7 +19,7 @@ import FileTxt from '@spectrum-icons/workflow/FileTxt'; import Folder from '@spectrum-icons/workflow/Folder'; import React from 'react'; import {Text} from '@react-spectrum/text'; -import {TreeView, TreeViewItem} from '../src'; +import {TreeItemContent, TreeView, TreeViewItem} from '../src'; export default { title: 'TreeView' @@ -30,34 +30,8 @@ function TestTree(props) {
- Photos - - - - - Edit - - - - Delete - - - - - Projects - - - - - Add - - - - Delete - - - - Projects-1 + + Photos @@ -69,38 +43,76 @@ function TestTree(props) { Delete - - Projects-1A - - - - - Add + + + + + Projects + + + + + Add + + + + Delete + + + + + + Projects-1 + + + + + Edit Delete - + + + + + Projects-1A + + + + + Add + + + + Delete + + + - Projects-2 - - - - - Edit - - - - Delete - - + + Projects-2 + + + + + Edit + + + + Delete + + + - Projects-3 - + + Projects-3 + + diff --git a/packages/@react-spectrum/tree/chromatic/TreeView.stories.tsx b/packages/@react-spectrum/tree/chromatic/TreeView.stories.tsx index 37c3a71ae67..20fc3b429c5 100644 --- a/packages/@react-spectrum/tree/chromatic/TreeView.stories.tsx +++ b/packages/@react-spectrum/tree/chromatic/TreeView.stories.tsx @@ -24,7 +24,7 @@ import {Heading, Text} from '@react-spectrum/text'; import {IllustratedMessage} from '@react-spectrum/illustratedmessage'; import {Meta} from '@storybook/react'; import React from 'react'; -import {SpectrumTreeViewProps, TreeView, TreeViewItem} from '../src'; +import {SpectrumTreeViewProps, TreeItemContent, TreeView, TreeViewItem} from '../src'; let states = [ {selectionMode: ['multiple', 'single']}, @@ -78,34 +78,8 @@ const Template = ({combos}) => ( - Photos - - - - - Edit - - - - Delete - - - - - Projects - - - - - Add - - - - Delete - - - - Projects-1 + + Photos @@ -117,38 +91,76 @@ const Template = ({combos}) => ( Delete - - Projects-1A - - - - - Add + + + + + Projects + + + + + Add + + + + Delete + + + + + + Projects-1 + + + + + Edit Delete - + + + + + Projects-1A + + + + + Add + + + + Delete + + + - Projects-2 - - - - - Edit - - - - Delete - - + + Projects-2 + + + + + Edit + + + + Delete + + + - Projects-3 - + + Projects-3 + + @@ -178,7 +190,9 @@ const EmptyTemplate = () => renderEmptyState={renderEmptyState}> {() => ( - dummy content + + dummy content + )} diff --git a/packages/@react-spectrum/tree/src/TreeView.tsx b/packages/@react-spectrum/tree/src/TreeView.tsx index ccf90601ec0..361e3276333 100644 --- a/packages/@react-spectrum/tree/src/TreeView.tsx +++ b/packages/@react-spectrum/tree/src/TreeView.tsx @@ -11,16 +11,15 @@ */ import {AriaTreeGridListProps} from '@react-aria/tree'; -import {ButtonContext, Collection, TreeItemContentRenderProps, TreeItemProps, TreeItemRenderProps, TreeRenderProps, UNSTABLE_Tree, UNSTABLE_TreeItem, UNSTABLE_TreeItemContent, useContextProps} from 'react-aria-components'; +import {ButtonContext, TreeItemContentProps, TreeItemContentRenderProps, TreeItemProps, TreeItemRenderProps, TreeRenderProps, UNSTABLE_Tree, UNSTABLE_TreeItem, UNSTABLE_TreeItemContent, useContextProps} from 'react-aria-components'; import {Checkbox} from '@react-spectrum/checkbox'; import ChevronLeftMedium from '@spectrum-icons/ui/ChevronLeftMedium'; import ChevronRightMedium from '@spectrum-icons/ui/ChevronRightMedium'; import {DOMRef, Expandable, Key, SelectionBehavior, SpectrumSelectionProps, StyleProps} from '@react-types/shared'; import {isAndroid} from '@react-aria/utils'; -import React, {createContext, isValidElement, JSX, JSXElementConstructor, ReactElement, ReactNode, useContext, useRef} from 'react'; +import React, {createContext, JSX, JSXElementConstructor, ReactElement, ReactNode, useContext, useRef} from 'react'; import {SlotProvider, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {style} from '@react-spectrum/style-macro-s1' with {type: 'macro'}; -import {Text} from '@react-spectrum/text'; import {useButton} from '@react-aria/button'; import {useLocale} from '@react-aria/i18n'; @@ -52,10 +51,6 @@ interface TreeRendererContextValue { } const TreeRendererContext = createContext({}); -function useTreeRendererContext(): TreeRendererContextValue { - return useContext(TreeRendererContext)!; -} - // TODO: add animations for rows appearing and disappearing // TODO: the below is needed so the borders of the top and bottom row isn't cut off if the TreeView is wrapped within a container by always reserving the 2px needed for the @@ -224,89 +219,73 @@ const treeRowOutline = style({ } }); +let TreeItemContext = createContext<{hasChildItems?: boolean}>({}); + export const TreeViewItem = (props: SpectrumTreeViewItemProps) => { let { - children, - childItems, - hasChildItems, href } = props; - let content; - let nestedRows; - let {renderer} = useTreeRendererContext(); - // TODO alternative api is that we have a separate prop for the TreeItems contents and expect the child to then be - // a nested tree item + return ( + // TODO right now all the tree rows have the various data attributes applied on their dom nodes, should they? Doesn't feel very useful + + treeRow({ + ...renderProps, + isLink: !!href + })} /> + + ); +}; - if (typeof children === 'string') { - content = {children}; - } else { - content = []; - nestedRows = []; - React.Children.forEach(children, node => { - if (isValidElement(node) && node.type === TreeViewItem) { - nestedRows.push(node); - } else { - content.push(node); - } - }); - } - if (childItems != null && renderer) { - nestedRows = ( - - {renderer} - - ); - } +export const TreeItemContent = (props: Omit & {children: ReactNode}) => { + let { + children + } = props; + let {hasChildItems} = useContext(TreeItemContext); return ( - // TODO right now all the tree rows have the various data attributes applied on their dom nodes, should they? Doesn't feel very useful - treeRow({ - ...renderProps, - isLink: !!href - })}> - - {({isExpanded, hasChildRows, level, selectionMode, selectionBehavior, isDisabled, isSelected, isFocusVisible}) => ( -
- {selectionMode !== 'none' && selectionBehavior === 'toggle' && ( + + {({isExpanded, hasChildRows, level, selectionMode, selectionBehavior, isDisabled, isSelected, isFocusVisible}) => ( +
+ {selectionMode !== 'none' && selectionBehavior === 'toggle' && ( // TODO: add transition? - + )} -
- {/* TODO: revisit when we do async loading, at the moment hasChildItems will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} - {(hasChildRows || hasChildItems) && } - + {/* TODO: revisit when we do async loading, at the moment hasChildItems will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} + {(hasChildRows || hasChildItems) && } + - {content} - -
-
+ icon: {UNSAFE_className: treeIcon(), size: 'S'}, + actionButton: {UNSAFE_className: treeActions(), isQuiet: true}, + actionGroup: { + UNSAFE_className: treeActions(), + isQuiet: true, + density: 'compact', + buttonLabelBehavior: 'hide', + isDisabled, + overflowMode: 'collapse' + }, + actionMenu: {UNSAFE_className: treeActionMenu(), UNSAFE_style: {marginInlineEnd: '.5rem'}, isQuiet: true} + }}> + + {children} + + +
+
)} - - {nestedRows} - + ); }; diff --git a/packages/@react-spectrum/tree/src/index.ts b/packages/@react-spectrum/tree/src/index.ts index dd7a4e0acc9..4d3d6a34990 100644 --- a/packages/@react-spectrum/tree/src/index.ts +++ b/packages/@react-spectrum/tree/src/index.ts @@ -12,5 +12,5 @@ /// -export {TreeViewItem, TreeView} from './TreeView'; +export {TreeViewItem, TreeView, TreeItemContent} from './TreeView'; export type {SpectrumTreeViewProps, SpectrumTreeViewItemProps} from './TreeView'; diff --git a/packages/@react-spectrum/tree/stories/TreeView.stories.tsx b/packages/@react-spectrum/tree/stories/TreeView.stories.tsx index 9f1b3b10473..2d68536f804 100644 --- a/packages/@react-spectrum/tree/stories/TreeView.stories.tsx +++ b/packages/@react-spectrum/tree/stories/TreeView.stories.tsx @@ -12,8 +12,7 @@ import {action} from '@storybook/addon-actions'; import {ActionGroup, Item} from '@react-spectrum/actiongroup'; -import {ActionMenu} from '@react-spectrum/menu'; -import Add from '@spectrum-icons/workflow/Add'; +import {Collection} from 'react-aria-components'; import {Content} from '@react-spectrum/view'; import Delete from '@spectrum-icons/workflow/Delete'; import Edit from '@spectrum-icons/workflow/Edit'; @@ -23,7 +22,7 @@ import {Heading, Text} from '@react-spectrum/text'; import {IllustratedMessage} from '@react-spectrum/illustratedmessage'; import {Link} from '@react-spectrum/link'; import React from 'react'; -import {SpectrumTreeViewProps, TreeView, TreeViewItem} from '../src'; +import {SpectrumTreeViewProps, TreeItemContent, TreeView, TreeViewItem} from '../src'; export default { title: 'TreeView', @@ -49,34 +48,24 @@ export const TreeExampleStatic = (args: SpectrumTreeViewProps) => (
- Photos - - - - - Edit - - - - Delete - - + + Photos + + + + + Edit + + + + Delete + + + - Projects - - - - - Edit - - - - Delete - - - - Projects-1 + + Projects @@ -88,9 +77,11 @@ export const TreeExampleStatic = (args: SpectrumTreeViewProps) => ( Delete - - Projects-1A - + + + + Projects-1 + @@ -101,35 +92,55 @@ export const TreeExampleStatic = (args: SpectrumTreeViewProps) => ( Delete + + + + Projects-1A + + + + + Edit + + + + Delete + + + - Projects-2 - - - - - Edit - - - - Delete - - + + Projects-2 + + + + + Edit + + + + Delete + + + - Projects-3 - - - - - Edit - - - - Delete - - + + Projects-3 + + + + + Edit + + + + Delete + + + @@ -193,13 +204,14 @@ let rows = [ ]} ]; -export const TreeExampleDynamic = (args: SpectrumTreeViewProps) => ( -
- - {(item: any) => ( - - {item.name} - {item.icon} +const DynamicTreeItem = (props) => { + let {childItems, name, icon} = props; + return ( + <> + + + {name} + {icon} @@ -210,7 +222,35 @@ export const TreeExampleDynamic = (args: SpectrumTreeViewProps) => ( Delete - + + + {(item: any) => ( + + {item.name} + + )} + + + + ); +}; + +export const TreeExampleDynamic = (args: SpectrumTreeViewProps) => ( +
+ + {(item: any) => ( + )}
@@ -236,20 +276,13 @@ export const WithLinks = (args: SpectrumTreeViewProps) => (
{(item) => ( - - {item.name} - {item.icon} - - - - Edit - - - - Delete - - - + )}
@@ -292,20 +325,12 @@ export const WithActionMenu = (args: SpectrumTreeViewProps) => (
{(item) => ( - - {item.name} - {item.icon} - - - - Add - - - - Delete - - - + )}
diff --git a/packages/@react-spectrum/tree/test/TreeView.test.tsx b/packages/@react-spectrum/tree/test/TreeView.test.tsx index 00cc151aa79..d82df73f14d 100644 --- a/packages/@react-spectrum/tree/test/TreeView.test.tsx +++ b/packages/@react-spectrum/tree/test/TreeView.test.tsx @@ -12,6 +12,7 @@ import {act, fireEvent, mockClickDefault, pointerMap, render as renderComponent, within} from '@react-spectrum/test-utils-internal'; import {ActionGroup, Item} from '@react-spectrum/actiongroup'; +import {Collection} from 'react-aria-components'; import {Content} from '@react-spectrum/view'; import Delete from '@spectrum-icons/workflow/Delete'; import Edit from '@spectrum-icons/workflow/Edit'; @@ -21,7 +22,7 @@ import {IllustratedMessage} from '@react-spectrum/illustratedmessage'; import {Provider} from '@react-spectrum/provider'; import React from 'react'; import {theme} from '@react-spectrum/theme-default'; -import {TreeView, TreeViewItem} from '../'; +import {TreeItemContent, TreeView, TreeViewItem} from '../'; import {User} from '@react-aria/test-utils'; import userEvent from '@testing-library/user-event'; @@ -32,34 +33,24 @@ let onExpandedChange = jest.fn(); let StaticTree = ({treeProps = {}, rowProps = {}}) => ( - Photos - - - - - Edit - - - - Delete - - + + Photos + + + + + Edit + + + + Delete + + + - Projects - - - - - Edit - - - - Delete - - - - Projects-1 + + Projects @@ -71,8 +62,10 @@ let StaticTree = ({treeProps = {}, rowProps = {}}) => ( Delete - - Projects-1A + + + + Projects-1 @@ -84,35 +77,55 @@ let StaticTree = ({treeProps = {}, rowProps = {}}) => ( Delete + + + + Projects-1A + + + + + Edit + + + + Delete + + + - Projects-2 - - - - - Edit - - - - Delete - - + + Projects-2 + + + + + Edit + + + + Delete + + + - Projects-3 - - - - - Edit - - - - Delete - - + + Projects-3 + + + + + Edit + + + + Delete + + + @@ -148,23 +161,51 @@ let rows = [ ]} ]; +const DynamicTreeItem = (props) => { + let {childItems, name} = props; + return ( + <> + + + {name} + + + + Edit + + + + Delete + + + + + {(item: any) => ( + + {item.name} + + )} + + + + ); +}; + let DynamicTree = ({treeProps = {}, rowProps = {}}) => ( {(item: any) => ( - - {item.name} - {item.icon} - - - - Edit - - - - Delete - - - + )} ); @@ -426,7 +467,9 @@ describe('Tree', () => { let {getAllByRole} = render( - Test + + Test + ); @@ -445,7 +488,9 @@ describe('Tree', () => { let {getAllByRole} = render( - Test + + Test + ); @@ -1193,7 +1238,9 @@ describe('Tree', () => { let tree = render( - Test + + Test + ); @@ -1205,7 +1252,9 @@ describe('Tree', () => { tree, - Test + + Test + ); @@ -1217,7 +1266,9 @@ describe('Tree', () => { tree, - Test + + Test + ); diff --git a/packages/react-aria-components/test/Tree.test.tsx b/packages/react-aria-components/test/Tree.test.tsx index 6670fa19a63..ebe938a635f 100644 --- a/packages/react-aria-components/test/Tree.test.tsx +++ b/packages/react-aria-components/test/Tree.test.tsx @@ -49,7 +49,7 @@ let StaticTreeItem = (props) => { }; let StaticTree = ({treeProps = {}, rowProps = {}}) => ( - + Photos @@ -1344,7 +1344,7 @@ AriaTreeTests({ ), allInteractionsDisabled: () => render( - + ) } }); From 4dbff99ccefe0ef063c686a62fef0a43c46aa547 Mon Sep 17 00:00:00 2001 From: GitHub Date: Thu, 16 Jan 2025 10:41:18 +1100 Subject: [PATCH 02/16] fix docs --- .../@react-spectrum/tree/docs/TreeView.mdx | 138 +++++++++++------- 1 file changed, 85 insertions(+), 53 deletions(-) diff --git a/packages/@react-spectrum/tree/docs/TreeView.mdx b/packages/@react-spectrum/tree/docs/TreeView.mdx index 9e1fd3a0d75..18f46a2929c 100644 --- a/packages/@react-spectrum/tree/docs/TreeView.mdx +++ b/packages/@react-spectrum/tree/docs/TreeView.mdx @@ -26,7 +26,7 @@ import Image from '@spectrum-icons/workflow/Image'; import Edit from '@spectrum-icons/workflow/Edit'; import Delete from '@spectrum-icons/workflow/Delete'; import {Text} from '@react-spectrum/text'; -import {TreeView, TreeViewItem} from '@react-spectrum/tree'; +import {TreeView, TreeViewItem, TreeItemContent} from '@react-spectrum/tree'; import {JSX} from "react"; import {Key} from "@react-types/shared"; import {ActionGroup, Item} from '@react-spectrum/actiongroup'; @@ -54,39 +54,57 @@ keywords: [tree, grid] ```tsx example - Documents - - - Project A + + Documents + + + + Project A + + - Weekly Report - + + Weekly Report + + - Document 1 - + + Document 1 + + - Document 2 - + + Document 2 + + - Photos - + + Photos + + - Image 1 - + + Image 1 + + - Image 2 - + + Image 2 + + - Image 3 - + + Image 3 + + @@ -129,8 +147,10 @@ function ExampleTree(props) { {(item: MyItem) => ( - {item.name} - {item.icon} + + {item.name} + {item.icon} + )} @@ -340,19 +360,27 @@ Tree items may also be links to another page or website. This can be achieved by ```tsx example - Bookmarks - + + Bookmarks + + - Adobe - + + Adobe + + - Google - + + Google + + - New York Times - + + New York Times + + @@ -368,18 +396,20 @@ The `` component works with frameworks and client side routers lik {(item: MyItem) => ( - {item.name} - {item.icon} - alert(`Item: ${item.id}, Action: ${key}`)}> - - - Edit - - - - Delete - - + + {item.name} + {item.icon} + alert(`Item: ${item.id}, Action: ${key}`)}> + + + Edit + + + + Delete + + + )} @@ -391,18 +421,20 @@ The `` component works with frameworks and client side routers lik {(item: MyItem) => ( - {item.name} - {item.icon} - alert(`Item: ${item.id}, Action: ${key}`)}> - - - Edit - - - - Delete - - + + {item.name} + {item.icon} + alert(`Item: ${item.id}, Action: ${key}`)}> + + + Edit + + + + Delete + + + )} From a7749a471f2ab436253748c69ee146ed5dde5e80 Mon Sep 17 00:00:00 2001 From: GitHub Date: Thu, 16 Jan 2025 11:10:14 +1100 Subject: [PATCH 03/16] fix dynamic example --- .../@react-spectrum/tree/docs/TreeView.mdx | 39 ++++++++++++++++--- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/packages/@react-spectrum/tree/docs/TreeView.mdx b/packages/@react-spectrum/tree/docs/TreeView.mdx index 18f46a2929c..5ffd866249c 100644 --- a/packages/@react-spectrum/tree/docs/TreeView.mdx +++ b/packages/@react-spectrum/tree/docs/TreeView.mdx @@ -27,6 +27,7 @@ import Edit from '@spectrum-icons/workflow/Edit'; import Delete from '@spectrum-icons/workflow/Delete'; import {Text} from '@react-spectrum/text'; import {TreeView, TreeViewItem, TreeItemContent} from '@react-spectrum/tree'; +import {Collection} from 'react-aria-components'; import {JSX} from "react"; import {Key} from "@react-types/shared"; import {ActionGroup, Item} from '@react-spectrum/actiongroup'; @@ -142,16 +143,42 @@ let items: MyItem[] = [ ]} ]; +const DynamicTreeItem = (props) => { + let {childItems, name, icon} = props; + return ( + <> + + + {props.name} + {props.icon} + + + {(item: any) => ( + + {item.name} + + )} + + + + ); +}; + function ExampleTree(props) { return ( {(item: MyItem) => ( - - - {item.name} - {item.icon} - - + )} ); From 1fef29274df16c4d7c8556b2d6c1ca269e805b01 Mon Sep 17 00:00:00 2001 From: GitHub Date: Thu, 16 Jan 2025 11:34:35 +1100 Subject: [PATCH 04/16] actually save --- packages/@react-spectrum/tree/docs/TreeView.mdx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/@react-spectrum/tree/docs/TreeView.mdx b/packages/@react-spectrum/tree/docs/TreeView.mdx index 5ffd866249c..0b0c417b9de 100644 --- a/packages/@react-spectrum/tree/docs/TreeView.mdx +++ b/packages/@react-spectrum/tree/docs/TreeView.mdx @@ -144,10 +144,9 @@ let items: MyItem[] = [ ]; const DynamicTreeItem = (props) => { - let {childItems, name, icon} = props; return ( <> - + {props.name} {props.icon} From 956e5eeea5846d97065475179014897f290fc886 Mon Sep 17 00:00:00 2001 From: GitHub Date: Thu, 16 Jan 2025 11:59:17 +1100 Subject: [PATCH 05/16] fix docs --- packages/@react-spectrum/tree/docs/TreeView.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tree/docs/TreeView.mdx b/packages/@react-spectrum/tree/docs/TreeView.mdx index 0b0c417b9de..62549e4de2c 100644 --- a/packages/@react-spectrum/tree/docs/TreeView.mdx +++ b/packages/@react-spectrum/tree/docs/TreeView.mdx @@ -151,7 +151,7 @@ const DynamicTreeItem = (props) => { {props.name} {props.icon} - + {(item: any) => ( Date: Wed, 22 Jan 2025 13:22:14 +1100 Subject: [PATCH 06/16] review comments --- packages/@react-spectrum/s2/src/TreeView.tsx | 24 +++++++---------- .../@react-spectrum/tree/docs/TreeView.mdx | 3 +-- .../@react-spectrum/tree/src/TreeView.tsx | 27 +++++++------------ packages/@react-spectrum/tree/src/index.ts | 1 + 4 files changed, 21 insertions(+), 34 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 5a6e2e2261d..e85b197e620 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -339,8 +339,6 @@ const treeRowFocusIndicator = raw(` }` ); -let TreeItemContext = createContext<{hasChildItems?: boolean}>({}); - export const TreeViewItem = (props: TreeViewItemProps) => { let { href @@ -349,14 +347,12 @@ export const TreeViewItem = (props: TreeViewItemProps) => { return ( // TODO right now all the tree rows have the various data attributes applied on their dom nodes, should they? Doesn't feel very useful - - treeRow({ - ...renderProps, - isLink: !!href, isEmphasized - }) + (renderProps.isFocusVisible && !isDetached ? ' ' + treeRowFocusIndicator : '')} /> - + treeRow({ + ...renderProps, + isLink: !!href, isEmphasized + }) + (renderProps.isFocusVisible && !isDetached ? ' ' + treeRowFocusIndicator : '')} /> ); }; @@ -365,7 +361,6 @@ export const TreeItemContent = (props: Omit & children } = props; let {isDetached, isEmphasized} = useContext(InternalTreeContext); - let {hasChildItems} = useContext(TreeItemContext); return ( @@ -392,8 +387,8 @@ export const TreeItemContent = (props: Omit & gridArea: 'level-padding', width: '[calc(calc(var(--tree-item-level, 0) - 1) * var(--indent))]' })} /> - {/* TODO: revisit when we do async loading, at the moment hasChildItems will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} - {(hasChildRows || hasChildItems) && } + {/* TODO: revisit when we do async loading, at the moment hasChildRows will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} + {hasChildRows && } & styles: style({size: fontRelative(20), flexShrink: 0}) }], [ActionButtonGroupContext, {styles: treeActions}], - [ActionMenuContext, {styles: treeActionMenu, isQuiet: true, 'aria-label': 'Actions'}], - [TreeItemContext, {}] + [ActionMenuContext, {styles: treeActionMenu, isQuiet: true, 'aria-label': 'Actions'}] ]}> {children} diff --git a/packages/@react-spectrum/tree/docs/TreeView.mdx b/packages/@react-spectrum/tree/docs/TreeView.mdx index 62549e4de2c..ccf22103977 100644 --- a/packages/@react-spectrum/tree/docs/TreeView.mdx +++ b/packages/@react-spectrum/tree/docs/TreeView.mdx @@ -26,8 +26,7 @@ import Image from '@spectrum-icons/workflow/Image'; import Edit from '@spectrum-icons/workflow/Edit'; import Delete from '@spectrum-icons/workflow/Delete'; import {Text} from '@react-spectrum/text'; -import {TreeView, TreeViewItem, TreeItemContent} from '@react-spectrum/tree'; -import {Collection} from 'react-aria-components'; +import {Collection, TreeView, TreeViewItem, TreeItemContent} from '@react-spectrum/tree'; import {JSX} from "react"; import {Key} from "@react-types/shared"; import {ActionGroup, Item} from '@react-spectrum/actiongroup'; diff --git a/packages/@react-spectrum/tree/src/TreeView.tsx b/packages/@react-spectrum/tree/src/TreeView.tsx index 361e3276333..eec31ccf94b 100644 --- a/packages/@react-spectrum/tree/src/TreeView.tsx +++ b/packages/@react-spectrum/tree/src/TreeView.tsx @@ -17,7 +17,7 @@ import ChevronLeftMedium from '@spectrum-icons/ui/ChevronLeftMedium'; import ChevronRightMedium from '@spectrum-icons/ui/ChevronRightMedium'; import {DOMRef, Expandable, Key, SelectionBehavior, SpectrumSelectionProps, StyleProps} from '@react-types/shared'; import {isAndroid} from '@react-aria/utils'; -import React, {createContext, JSX, JSXElementConstructor, ReactElement, ReactNode, useContext, useRef} from 'react'; +import React, {createContext, JSX, JSXElementConstructor, ReactElement, ReactNode, useRef} from 'react'; import {SlotProvider, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {style} from '@react-spectrum/style-macro-s1' with {type: 'macro'}; import {useButton} from '@react-aria/button'; @@ -219,8 +219,6 @@ const treeRowOutline = style({ } }); -let TreeItemContext = createContext<{hasChildItems?: boolean}>({}); - export const TreeViewItem = (props: SpectrumTreeViewItemProps) => { let { href @@ -228,14 +226,12 @@ export const TreeViewItem = (props: SpectrumTreeViewItemProps< return ( // TODO right now all the tree rows have the various data attributes applied on their dom nodes, should they? Doesn't feel very useful - - treeRow({ - ...renderProps, - isLink: !!href - })} /> - + treeRow({ + ...renderProps, + isLink: !!href + })} /> ); }; @@ -244,7 +240,6 @@ export const TreeItemContent = (props: Omit & let { children } = props; - let {hasChildItems} = useContext(TreeItemContext); return ( @@ -259,8 +254,8 @@ export const TreeItemContent = (props: Omit & slot="selection" /> )}
- {/* TODO: revisit when we do async loading, at the moment hasChildItems will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} - {(hasChildRows || hasChildItems) && } + {/* TODO: revisit when we do async loading, at the moment hasChildRows will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} + {hasChildRows && } & }, actionMenu: {UNSAFE_className: treeActionMenu(), UNSAFE_style: {marginInlineEnd: '.5rem'}, isQuiet: true} }}> - - {children} - + {children}
diff --git a/packages/@react-spectrum/tree/src/index.ts b/packages/@react-spectrum/tree/src/index.ts index 4d3d6a34990..c6b9cae2689 100644 --- a/packages/@react-spectrum/tree/src/index.ts +++ b/packages/@react-spectrum/tree/src/index.ts @@ -13,4 +13,5 @@ /// export {TreeViewItem, TreeView, TreeItemContent} from './TreeView'; +export {Collection} from 'react-aria-components'; export type {SpectrumTreeViewProps, SpectrumTreeViewItemProps} from './TreeView'; From f8cc10f1621901c9cb76c26066968267b27c73b5 Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 22 Jan 2025 15:27:49 +1100 Subject: [PATCH 07/16] Revert change to not use hasChildItems --- packages/@react-spectrum/s2/src/TreeView.tsx | 24 ++++++++++------- .../@react-spectrum/tree/src/TreeView.tsx | 27 ++++++++++++------- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index e85b197e620..5a6e2e2261d 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -339,6 +339,8 @@ const treeRowFocusIndicator = raw(` }` ); +let TreeItemContext = createContext<{hasChildItems?: boolean}>({}); + export const TreeViewItem = (props: TreeViewItemProps) => { let { href @@ -347,12 +349,14 @@ export const TreeViewItem = (props: TreeViewItemProps) => { return ( // TODO right now all the tree rows have the various data attributes applied on their dom nodes, should they? Doesn't feel very useful - treeRow({ - ...renderProps, - isLink: !!href, isEmphasized - }) + (renderProps.isFocusVisible && !isDetached ? ' ' + treeRowFocusIndicator : '')} /> + + treeRow({ + ...renderProps, + isLink: !!href, isEmphasized + }) + (renderProps.isFocusVisible && !isDetached ? ' ' + treeRowFocusIndicator : '')} /> + ); }; @@ -361,6 +365,7 @@ export const TreeItemContent = (props: Omit & children } = props; let {isDetached, isEmphasized} = useContext(InternalTreeContext); + let {hasChildItems} = useContext(TreeItemContext); return ( @@ -387,8 +392,8 @@ export const TreeItemContent = (props: Omit & gridArea: 'level-padding', width: '[calc(calc(var(--tree-item-level, 0) - 1) * var(--indent))]' })} /> - {/* TODO: revisit when we do async loading, at the moment hasChildRows will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} - {hasChildRows && } + {/* TODO: revisit when we do async loading, at the moment hasChildItems will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} + {(hasChildRows || hasChildItems) && } & styles: style({size: fontRelative(20), flexShrink: 0}) }], [ActionButtonGroupContext, {styles: treeActions}], - [ActionMenuContext, {styles: treeActionMenu, isQuiet: true, 'aria-label': 'Actions'}] + [ActionMenuContext, {styles: treeActionMenu, isQuiet: true, 'aria-label': 'Actions'}], + [TreeItemContext, {}] ]}> {children} diff --git a/packages/@react-spectrum/tree/src/TreeView.tsx b/packages/@react-spectrum/tree/src/TreeView.tsx index eec31ccf94b..361e3276333 100644 --- a/packages/@react-spectrum/tree/src/TreeView.tsx +++ b/packages/@react-spectrum/tree/src/TreeView.tsx @@ -17,7 +17,7 @@ import ChevronLeftMedium from '@spectrum-icons/ui/ChevronLeftMedium'; import ChevronRightMedium from '@spectrum-icons/ui/ChevronRightMedium'; import {DOMRef, Expandable, Key, SelectionBehavior, SpectrumSelectionProps, StyleProps} from '@react-types/shared'; import {isAndroid} from '@react-aria/utils'; -import React, {createContext, JSX, JSXElementConstructor, ReactElement, ReactNode, useRef} from 'react'; +import React, {createContext, JSX, JSXElementConstructor, ReactElement, ReactNode, useContext, useRef} from 'react'; import {SlotProvider, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {style} from '@react-spectrum/style-macro-s1' with {type: 'macro'}; import {useButton} from '@react-aria/button'; @@ -219,6 +219,8 @@ const treeRowOutline = style({ } }); +let TreeItemContext = createContext<{hasChildItems?: boolean}>({}); + export const TreeViewItem = (props: SpectrumTreeViewItemProps) => { let { href @@ -226,12 +228,14 @@ export const TreeViewItem = (props: SpectrumTreeViewItemProps< return ( // TODO right now all the tree rows have the various data attributes applied on their dom nodes, should they? Doesn't feel very useful - treeRow({ - ...renderProps, - isLink: !!href - })} /> + + treeRow({ + ...renderProps, + isLink: !!href + })} /> + ); }; @@ -240,6 +244,7 @@ export const TreeItemContent = (props: Omit & let { children } = props; + let {hasChildItems} = useContext(TreeItemContext); return ( @@ -254,8 +259,8 @@ export const TreeItemContent = (props: Omit & slot="selection" /> )}
- {/* TODO: revisit when we do async loading, at the moment hasChildRows will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} - {hasChildRows && } + {/* TODO: revisit when we do async loading, at the moment hasChildItems will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} + {(hasChildRows || hasChildItems) && } & }, actionMenu: {UNSAFE_className: treeActionMenu(), UNSAFE_style: {marginInlineEnd: '.5rem'}, isQuiet: true} }}> - {children} + + {children} +
From dcfd36661d19f86074dc12fd519f334fd59c4f9f Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 29 Jan 2025 15:45:35 +1100 Subject: [PATCH 08/16] add prop table --- packages/@react-spectrum/tree/docs/TreeView.mdx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tree/docs/TreeView.mdx b/packages/@react-spectrum/tree/docs/TreeView.mdx index ccf22103977..833d4962c99 100644 --- a/packages/@react-spectrum/tree/docs/TreeView.mdx +++ b/packages/@react-spectrum/tree/docs/TreeView.mdx @@ -52,7 +52,7 @@ keywords: [tree, grid] ## Example ```tsx example - + Documents @@ -471,6 +471,10 @@ The `` component works with frameworks and client side routers lik +### TreeItemContent props + + + ### TreeViewItem props From ba69dc249b8914e2ab5ea3c8097a05461f39e37a Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 29 Jan 2025 16:07:31 +1100 Subject: [PATCH 09/16] fix docs example --- packages/@react-spectrum/tree/docs/TreeView.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tree/docs/TreeView.mdx b/packages/@react-spectrum/tree/docs/TreeView.mdx index 833d4962c99..44d74459b20 100644 --- a/packages/@react-spectrum/tree/docs/TreeView.mdx +++ b/packages/@react-spectrum/tree/docs/TreeView.mdx @@ -145,7 +145,7 @@ let items: MyItem[] = [ const DynamicTreeItem = (props) => { return ( <> - + {props.name} {props.icon} From 7045d90b2f3ee55c0bc5b052beb6c9d754a3faee Mon Sep 17 00:00:00 2001 From: GitHub Date: Thu, 30 Jan 2025 11:06:49 +1100 Subject: [PATCH 10/16] remove extra border attributes in theme --- packages/@react-spectrum/s2/src/TreeView.tsx | 55 ++----------------- .../s2/style/spectrum-theme.ts | 30 ++++------ 2 files changed, 16 insertions(+), 69 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 5a6e2e2261d..b82e6337bec 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -215,66 +215,21 @@ const treeCellGrid = style({ forcedColors: 'Highlight' } }, - borderTopColor: { - default: 'transparent', - isSelected: { - isFirst: 'transparent' - }, - isDetached: { - default: 'transparent', - isSelected: '--rowSelectedBorderColor' - } - }, - borderInlineEndColor: { - default: 'transparent', - isSelected: 'transparent', - isDetached: { - default: 'transparent', - isSelected: '--rowSelectedBorderColor' - } - }, - borderBottomColor: { - default: 'transparent', - isSelected: 'transparent', - isNextSelected: 'transparent', - isNextFocused: 'transparent', - isDetached: { - default: 'transparent', - isSelected: '--rowSelectedBorderColor' - } - }, - borderInlineStartColor: { - default: 'transparent', - isSelected: 'transparent', + borderColor: { isDetached: { default: 'transparent', isSelected: '--rowSelectedBorderColor' } }, - borderTopWidth: { - default: 0, - isFirst: { - default: 1, - forcedColors: 0 - }, - isDetached: 1 - }, - borderBottomWidth: { - default: 0, - isDetached: 1 - }, - borderStartWidth: { - default: 0, - isDetached: 1 - }, - borderEndWidth: { - default: 0, + borderWidth: { isDetached: 1 }, borderRadius: { isDetached: 'default' }, - borderStyle: 'solid' + borderStyle: { + isDetached: 'solid' + } }); const treeCheckbox = style({ diff --git a/packages/@react-spectrum/s2/style/spectrum-theme.ts b/packages/@react-spectrum/s2/style/spectrum-theme.ts index 8e81810afa9..c6c5cdc929c 100644 --- a/packages/@react-spectrum/s2/style/spectrum-theme.ts +++ b/packages/@react-spectrum/s2/style/spectrum-theme.ts @@ -320,19 +320,6 @@ const borderWidth = { 4: getToken('border-width-400') }; -const borderColor = createColorProperty({ - ...color, - negative: { - default: colorToken('negative-border-color-default'), - isHovered: colorToken('negative-border-color-hover'), - isFocusVisible: colorToken('negative-border-color-key-focus'), - isPressed: colorToken('negative-border-color-down') - }, - disabled: colorToken('disabled-border-color') - // forcedColors: 'GrayText' - -}); - const radius = { none: getToken('corner-radius-none'), // 0px sm: pxToRem(getToken('corner-radius-small-default')), // 4px @@ -598,7 +585,17 @@ export const style = createTheme({ pasteboard: weirdColorToken('background-pasteboard-color'), elevated: weirdColorToken('background-elevated-color') }), - + borderColor: createColorProperty({ + ...color, + negative: { + default: colorToken('negative-border-color-default'), + isHovered: colorToken('negative-border-color-hover'), + isFocusVisible: colorToken('negative-border-color-key-focus'), + isPressed: colorToken('negative-border-color-down') + }, + disabled: colorToken('disabled-border-color') + // forcedColors: 'GrayText' + }), outlineColor: createColorProperty({ ...color, 'focus-ring': { @@ -672,10 +669,6 @@ export const style = createTheme({ borderEndWidth: createRenamedProperty('borderInlineEndWidth', borderWidth), borderTopWidth: borderWidth, borderBottomWidth: borderWidth, - borderInlineStartColor: borderColor, // don't know why i can't use renamed - borderInlineEndColor: borderColor, - borderTopColor: borderColor, - borderBottomColor: borderColor, borderStyle: ['solid', 'dashed', 'dotted', 'double', 'hidden', 'none'] as const, strokeWidth: { 0: '0', @@ -990,7 +983,6 @@ export const style = createTheme({ scrollMarginX: ['scrollMarginStart', 'scrollMarginEnd'] as const, scrollMarginY: ['scrollMarginTop', 'scrollMarginBottom'] as const, borderWidth: ['borderTopWidth', 'borderBottomWidth', 'borderStartWidth', 'borderEndWidth'] as const, - borderColor: ['borderTopColor', 'borderBottomColor', 'borderInlineStartColor', 'borderInlineEndColor'] as const, borderXWidth: ['borderStartWidth', 'borderEndWidth'] as const, borderYWidth: ['borderTopWidth', 'borderBottomWidth'] as const, borderRadius: ['borderTopStartRadius', 'borderTopEndRadius', 'borderBottomStartRadius', 'borderBottomEndRadius'] as const, From 04adce4556229d1adf3a15959aca38d51e8ce25f Mon Sep 17 00:00:00 2001 From: GitHub Date: Thu, 30 Jan 2025 11:23:45 +1100 Subject: [PATCH 11/16] fix snapshots --- .../s2/style/__tests__/style-macro.test.js | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/@react-spectrum/s2/style/__tests__/style-macro.test.js b/packages/@react-spectrum/s2/style/__tests__/style-macro.test.js index 4e44c17866f..3762768c1b6 100644 --- a/packages/@react-spectrum/s2/style/__tests__/style-macro.test.js +++ b/packages/@react-spectrum/s2/style/__tests__/style-macro.test.js @@ -40,7 +40,7 @@ describe('style-macro', () => { "@layer _.a, _.b, _.c; @layer _.b { - .D-13alit4c { + .A-13alit4c { &:first-child { margin-top: 0.25rem; } @@ -49,7 +49,7 @@ describe('style-macro', () => { @layer _.c.e { @media (min-width: 1024px) { - .D-13alit4ed { + .A-13alit4ed { &:first-child { margin-top: 0.5rem; } @@ -59,7 +59,7 @@ describe('style-macro', () => { " `); - expect(js).toMatchInlineSnapshot('" D-13alit4c D-13alit4ed"'); + expect(js).toMatchInlineSnapshot('" A-13alit4c A-13alit4ed"'); }); it('should support self references', () => { @@ -73,48 +73,48 @@ describe('style-macro', () => { "@layer _.a, _.b; @layer _.a { - .tc { + .uc { border-top-width: 2px; } - .uc { + .vc { border-bottom-width: 2px; } - .r-375tox { - border-inline-start-width: var(--r); + .s-375toy { + border-inline-start-width: var(--s); } - .sc { + .tc { border-inline-end-width: 2px; } - .F-375tnp { - padding-inline-start: var(--F); + .C-375tnm { + padding-inline-start: var(--C); } - .GI { - padding-inline-end: calc(var(--j, var(--n)) * 3 / 8); + .DI { + padding-inline-end: calc(var(--k, var(--o)) * 3 / 8); } - .k-4s570k { - width: calc(200px - var(--r) - var(--F)); + .l-4s570k { + width: calc(200px - var(--s) - var(--C)); } - .-_375tox_r-c { - --r: 2px; + .-_375toy_s-c { + --s: 2px; } - .-_375tnp_F-I { - --F: calc(var(--j, var(--n)) * 3 / 8); + .-_375tnm_C-I { + --C: calc(var(--k, var(--o)) * 3 / 8); } } From 9e7b7b2c538e2639f32e4f04c9c308a9c6d23fa4 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Wed, 12 Feb 2025 09:54:30 +1100 Subject: [PATCH 12/16] fix: S2 treeview haschildnodes (#7694) * fix logic to determine hasChildNodes * revert breaking changes but use prop override --- .../gridlist/src/useGridListItem.ts | 7 ++- packages/@react-spectrum/s2/src/TreeView.tsx | 25 ++++------ .../@react-spectrum/tree/src/TreeView.tsx | 30 ++++-------- .../tree/test/TreeView.test.tsx | 38 +++++++-------- packages/react-aria-components/docs/Tree.mdx | 2 +- packages/react-aria-components/src/Tree.tsx | 20 ++++---- .../stories/Tree.stories.tsx | 18 ++++---- .../react-aria-components/test/Table.test.js | 6 +-- .../react-aria-components/test/Tree.test.tsx | 46 +++++++++---------- 9 files changed, 88 insertions(+), 104 deletions(-) diff --git a/packages/@react-aria/gridlist/src/useGridListItem.ts b/packages/@react-aria/gridlist/src/useGridListItem.ts index a6e5a910838..0e83bc86839 100644 --- a/packages/@react-aria/gridlist/src/useGridListItem.ts +++ b/packages/@react-aria/gridlist/src/useGridListItem.ts @@ -28,7 +28,9 @@ export interface AriaGridListItemOptions { /** Whether the list row is contained in a virtual scroller. */ isVirtualized?: boolean, /** Whether selection should occur on press up instead of press down. */ - shouldSelectOnPressUp?: boolean + shouldSelectOnPressUp?: boolean, + /** Whether this item has children, even if not loaded yet. */ + hasChildItems?: boolean } export interface GridListItemAria extends SelectableItemStates { @@ -86,13 +88,14 @@ export function useGridListItem(props: AriaGridListItemOptions, state: ListSt }; let treeGridRowProps: HTMLAttributes = {}; - let hasChildRows; + let hasChildRows = props.hasChildItems; let hasLink = state.selectionManager.isLink(node.key); if (node != null && 'expandedKeys' in state) { // TODO: ideally node.hasChildNodes would be a way to tell if a row has child nodes, but the row's contents make it so that value is always // true... let children = state.collection.getChildren?.(node.key); hasChildRows = [...(children ?? [])].length > 1; + if (onAction == null && !hasLink && state.selectionManager.selectionMode === 'none' && hasChildRows) { onAction = () => state.toggleKey(node.key); } diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index b82e6337bec..40cc5194ede 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -294,8 +294,6 @@ const treeRowFocusIndicator = raw(` }` ); -let TreeItemContext = createContext<{hasChildItems?: boolean}>({}); - export const TreeViewItem = (props: TreeViewItemProps) => { let { href @@ -303,15 +301,12 @@ export const TreeViewItem = (props: TreeViewItemProps) => { let {isDetached, isEmphasized} = useContext(InternalTreeContext); return ( - // TODO right now all the tree rows have the various data attributes applied on their dom nodes, should they? Doesn't feel very useful - - treeRow({ - ...renderProps, - isLink: !!href, isEmphasized - }) + (renderProps.isFocusVisible && !isDetached ? ' ' + treeRowFocusIndicator : '')} /> - + treeRow({ + ...renderProps, + isLink: !!href, isEmphasized + }) + (renderProps.isFocusVisible && !isDetached ? ' ' + treeRowFocusIndicator : '')} /> ); }; @@ -320,11 +315,10 @@ export const TreeItemContent = (props: Omit & children } = props; let {isDetached, isEmphasized} = useContext(InternalTreeContext); - let {hasChildItems} = useContext(TreeItemContext); return ( - {({isExpanded, hasChildRows, selectionMode, selectionBehavior, isDisabled, isFocusVisible, isSelected, id, state}) => { + {({isExpanded, hasChildItems, selectionMode, selectionBehavior, isDisabled, isFocusVisible, isSelected, id, state}) => { let isNextSelected = false; let isNextFocused = false; let keyAfter = state.collection.getKeyAfter(id); @@ -348,7 +342,7 @@ export const TreeItemContent = (props: Omit & width: '[calc(calc(var(--tree-item-level, 0) - 1) * var(--indent))]' })} /> {/* TODO: revisit when we do async loading, at the moment hasChildItems will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} - {(hasChildRows || hasChildItems) && } + {hasChildItems && } & styles: style({size: fontRelative(20), flexShrink: 0}) }], [ActionButtonGroupContext, {styles: treeActions}], - [ActionMenuContext, {styles: treeActionMenu, isQuiet: true, 'aria-label': 'Actions'}], - [TreeItemContext, {}] + [ActionMenuContext, {styles: treeActionMenu, isQuiet: true, 'aria-label': 'Actions'}] ]}> {children} diff --git a/packages/@react-spectrum/tree/src/TreeView.tsx b/packages/@react-spectrum/tree/src/TreeView.tsx index 361e3276333..a42d0152a93 100644 --- a/packages/@react-spectrum/tree/src/TreeView.tsx +++ b/packages/@react-spectrum/tree/src/TreeView.tsx @@ -17,7 +17,7 @@ import ChevronLeftMedium from '@spectrum-icons/ui/ChevronLeftMedium'; import ChevronRightMedium from '@spectrum-icons/ui/ChevronRightMedium'; import {DOMRef, Expandable, Key, SelectionBehavior, SpectrumSelectionProps, StyleProps} from '@react-types/shared'; import {isAndroid} from '@react-aria/utils'; -import React, {createContext, JSX, JSXElementConstructor, ReactElement, ReactNode, useContext, useRef} from 'react'; +import React, {createContext, JSX, JSXElementConstructor, ReactElement, ReactNode, useRef} from 'react'; import {SlotProvider, useDOMRef, useStyleProps} from '@react-spectrum/utils'; import {style} from '@react-spectrum/style-macro-s1' with {type: 'macro'}; import {useButton} from '@react-aria/button'; @@ -40,8 +40,6 @@ export interface SpectrumTreeViewProps extends Omit, export interface SpectrumTreeViewItemProps extends Omit { /** Rendered contents of the tree item or child items. */ children: ReactNode, - /** Whether this item has children, even if not loaded yet. */ - hasChildItems?: boolean, /** A list of child tree item objects used when dynamically rendering the tree item children. */ childItems?: Iterable } @@ -219,23 +217,18 @@ const treeRowOutline = style({ } }); -let TreeItemContext = createContext<{hasChildItems?: boolean}>({}); - export const TreeViewItem = (props: SpectrumTreeViewItemProps) => { let { href } = props; return ( - // TODO right now all the tree rows have the various data attributes applied on their dom nodes, should they? Doesn't feel very useful - - treeRow({ - ...renderProps, - isLink: !!href - })} /> - + treeRow({ + ...renderProps, + isLink: !!href + })} /> ); }; @@ -244,11 +237,10 @@ export const TreeItemContent = (props: Omit & let { children } = props; - let {hasChildItems} = useContext(TreeItemContext); return ( - {({isExpanded, hasChildRows, level, selectionMode, selectionBehavior, isDisabled, isSelected, isFocusVisible}) => ( + {({isExpanded, hasChildItems, level, selectionMode, selectionBehavior, isDisabled, isSelected, isFocusVisible}) => (
{selectionMode !== 'none' && selectionBehavior === 'toggle' && ( // TODO: add transition? @@ -260,7 +252,7 @@ export const TreeItemContent = (props: Omit & )}
{/* TODO: revisit when we do async loading, at the moment hasChildItems will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} - {(hasChildRows || hasChildItems) && } + {hasChildItems && } & }, actionMenu: {UNSAFE_className: treeActionMenu(), UNSAFE_style: {marginInlineEnd: '.5rem'}, isQuiet: true} }}> - - {children} - + {children}
diff --git a/packages/@react-spectrum/tree/test/TreeView.test.tsx b/packages/@react-spectrum/tree/test/TreeView.test.tsx index d82df73f14d..ca58de623bd 100644 --- a/packages/@react-spectrum/tree/test/TreeView.test.tsx +++ b/packages/@react-spectrum/tree/test/TreeView.test.tsx @@ -293,7 +293,7 @@ describe('Tree', () => { expect(rowNoChild).toHaveAttribute('data-level', '1'); expect(rowNoChild).toHaveAttribute('aria-posinset', '1'); expect(rowNoChild).toHaveAttribute('aria-setsize', '2'); - expect(rowNoChild).not.toHaveAttribute('data-has-child-rows'); + expect(rowNoChild).not.toHaveAttribute('data-has-child-items'); expect(rowNoChild).toHaveAttribute('data-rac'); let rowWithChildren = rows[1]; @@ -305,7 +305,7 @@ describe('Tree', () => { expect(rowWithChildren).toHaveAttribute('data-level', '1'); expect(rowWithChildren).toHaveAttribute('aria-posinset', '2'); expect(rowWithChildren).toHaveAttribute('aria-setsize', '2'); - expect(rowWithChildren).toHaveAttribute('data-has-child-rows', 'true'); + expect(rowWithChildren).toHaveAttribute('data-has-child-items', 'true'); expect(rowWithChildren).toHaveAttribute('data-rac'); let level2ChildRow = rows[2]; @@ -316,7 +316,7 @@ describe('Tree', () => { expect(level2ChildRow).toHaveAttribute('data-level', '2'); expect(level2ChildRow).toHaveAttribute('aria-posinset', '1'); expect(level2ChildRow).toHaveAttribute('aria-setsize', '3'); - expect(level2ChildRow).toHaveAttribute('data-has-child-rows', 'true'); + expect(level2ChildRow).toHaveAttribute('data-has-child-items', 'true'); expect(level2ChildRow).toHaveAttribute('data-rac'); let level3ChildRow = rows[3]; @@ -327,7 +327,7 @@ describe('Tree', () => { expect(level3ChildRow).toHaveAttribute('data-level', '3'); expect(level3ChildRow).toHaveAttribute('aria-posinset', '1'); expect(level3ChildRow).toHaveAttribute('aria-setsize', '1'); - expect(level3ChildRow).not.toHaveAttribute('data-has-child-rows'); + expect(level3ChildRow).not.toHaveAttribute('data-has-child-items'); expect(level3ChildRow).toHaveAttribute('data-rac'); let level2ChildRow2 = rows[4]; @@ -338,7 +338,7 @@ describe('Tree', () => { expect(level2ChildRow2).toHaveAttribute('data-level', '2'); expect(level2ChildRow2).toHaveAttribute('aria-posinset', '2'); expect(level2ChildRow2).toHaveAttribute('aria-setsize', '3'); - expect(level2ChildRow2).not.toHaveAttribute('data-has-child-rows'); + expect(level2ChildRow2).not.toHaveAttribute('data-has-child-items'); expect(level2ChildRow2).toHaveAttribute('data-rac'); let level2ChildRow3 = rows[5]; @@ -349,7 +349,7 @@ describe('Tree', () => { expect(level2ChildRow3).toHaveAttribute('data-level', '2'); expect(level2ChildRow3).toHaveAttribute('aria-posinset', '3'); expect(level2ChildRow3).toHaveAttribute('aria-setsize', '3'); - expect(level2ChildRow3).not.toHaveAttribute('data-has-child-rows'); + expect(level2ChildRow3).not.toHaveAttribute('data-has-child-items'); expect(level2ChildRow3).toHaveAttribute('data-rac'); }); @@ -358,7 +358,7 @@ describe('Tree', () => { let rows = getAllByRole('row'); expect(rows[1]).toHaveAttribute('aria-label', 'Projects'); - expect(rows[1]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[1]).toHaveAttribute('data-has-child-items', 'true'); expect(rows[1]).toHaveAttribute('aria-selected', 'false'); }); @@ -374,28 +374,28 @@ describe('Tree', () => { expect(rows[0]).toHaveAttribute('aria-level', '1'); expect(rows[0]).toHaveAttribute('aria-posinset', '1'); expect(rows[0]).toHaveAttribute('aria-setsize', '2'); - expect(rows[0]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[0]).toHaveAttribute('data-has-child-items', 'true'); expect(rows[2]).toHaveAttribute('aria-label', 'Project 2'); expect(rows[2]).toHaveAttribute('aria-expanded', 'true'); expect(rows[2]).toHaveAttribute('aria-level', '2'); expect(rows[2]).toHaveAttribute('aria-posinset', '2'); expect(rows[2]).toHaveAttribute('aria-setsize', '5'); - expect(rows[2]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[2]).toHaveAttribute('data-has-child-items', 'true'); expect(rows[8]).toHaveAttribute('aria-label', 'Project 5'); expect(rows[8]).toHaveAttribute('aria-expanded', 'true'); expect(rows[8]).toHaveAttribute('aria-level', '2'); expect(rows[8]).toHaveAttribute('aria-posinset', '5'); expect(rows[8]).toHaveAttribute('aria-setsize', '5'); - expect(rows[8]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[8]).toHaveAttribute('data-has-child-items', 'true'); expect(rows[12]).toHaveAttribute('aria-label', 'Reports'); expect(rows[12]).toHaveAttribute('aria-expanded', 'true'); expect(rows[12]).toHaveAttribute('aria-level', '1'); expect(rows[12]).toHaveAttribute('aria-posinset', '2'); expect(rows[12]).toHaveAttribute('aria-setsize', '2'); - expect(rows[12]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[12]).toHaveAttribute('data-has-child-items', 'true'); expect(rows[16]).toHaveAttribute('aria-label', 'Reports 1ABC'); expect(rows[16]).toHaveAttribute('aria-level', '5'); @@ -463,7 +463,7 @@ describe('Tree', () => { expect(treeTester.selectedRows[0]).toBe(row1); }); - it('should render a chevron for an expandable row marked with hasChildRows', () => { + it('should render a chevron for an expandable row marked with hasChildItems', () => { let {getAllByRole} = render( @@ -480,7 +480,7 @@ describe('Tree', () => { expect(rows[0]).toHaveAttribute('aria-label', 'Test'); // Until the row gets children, don't mark it with the aria/data attributes. expect(rows[0]).not.toHaveAttribute('aria-expanded'); - expect(rows[0]).not.toHaveAttribute('data-has-child-rows'); + expect(rows[0]).toHaveAttribute('data-has-child-items'); expect(chevron).toBeTruthy(); }); @@ -893,7 +893,7 @@ describe('Tree', () => { expect(rows[0]).toHaveAttribute('aria-level', '1'); expect(rows[0]).toHaveAttribute('aria-posinset', '1'); expect(rows[0]).toHaveAttribute('aria-setsize', '2'); - expect(rows[0]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[0]).toHaveAttribute('data-has-child-items', 'true'); expect(onExpandedChange).toHaveBeenCalledTimes(0); // Check we can open/close a top level row @@ -904,7 +904,7 @@ describe('Tree', () => { expect(rows[0]).toHaveAttribute('aria-level', '1'); expect(rows[0]).toHaveAttribute('aria-posinset', '1'); expect(rows[0]).toHaveAttribute('aria-setsize', '2'); - expect(rows[0]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[0]).toHaveAttribute('data-has-child-items', 'true'); expect(onExpandedChange).toHaveBeenCalledTimes(1); // Note that the children of the parent row will still be in the "expanded" array expect(new Set(onExpandedChange.mock.calls[0][0])).toEqual(new Set(['Project-2', 'Project-5', 'Reports', 'Reports-1', 'Reports-1A', 'Reports-1AB'])); @@ -918,7 +918,7 @@ describe('Tree', () => { expect(rows[0]).toHaveAttribute('aria-level', '1'); expect(rows[0]).toHaveAttribute('aria-posinset', '1'); expect(rows[0]).toHaveAttribute('aria-setsize', '2'); - expect(rows[0]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[0]).toHaveAttribute('data-has-child-items', 'true'); expect(onExpandedChange).toHaveBeenCalledTimes(2); expect(new Set(onExpandedChange.mock.calls[1][0])).toEqual(new Set(['Projects', 'Project-2', 'Project-5', 'Reports', 'Reports-1', 'Reports-1A', 'Reports-1AB'])); rows = treeTester.rows; @@ -932,7 +932,7 @@ describe('Tree', () => { expect(rows[2]).toHaveAttribute('aria-level', '2'); expect(rows[2]).toHaveAttribute('aria-posinset', '2'); expect(rows[2]).toHaveAttribute('aria-setsize', '5'); - expect(rows[2]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[2]).toHaveAttribute('data-has-child-items', 'true'); // Check we can close a nested row and it doesn't affect the parent await treeTester.toggleRowExpansion({row: rows[2], interactionType: type as 'mouse' | 'keyboard'}); @@ -942,13 +942,13 @@ describe('Tree', () => { expect(rows[2]).toHaveAttribute('aria-level', '2'); expect(rows[2]).toHaveAttribute('aria-posinset', '2'); expect(rows[2]).toHaveAttribute('aria-setsize', '5'); - expect(rows[2]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[2]).toHaveAttribute('data-has-child-items', 'true'); expect(rows[0]).toHaveAttribute('aria-expanded', 'true'); expect(rows[0]).toHaveAttribute('data-expanded', 'true'); expect(rows[0]).toHaveAttribute('aria-level', '1'); expect(rows[0]).toHaveAttribute('aria-posinset', '1'); expect(rows[0]).toHaveAttribute('aria-setsize', '2'); - expect(rows[0]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[0]).toHaveAttribute('data-has-child-items', 'true'); expect(onExpandedChange).toHaveBeenCalledTimes(3); expect(new Set(onExpandedChange.mock.calls[2][0])).toEqual(new Set(['Projects', 'Project-5', 'Reports', 'Reports-1', 'Reports-1A', 'Reports-1AB'])); rows = treeTester.rows; diff --git a/packages/react-aria-components/docs/Tree.mdx b/packages/react-aria-components/docs/Tree.mdx index 2b9a2de801d..ff9253ed617 100644 --- a/packages/react-aria-components/docs/Tree.mdx +++ b/packages/react-aria-components/docs/Tree.mdx @@ -134,7 +134,7 @@ let items = [ position: relative; transform: translateZ(0); - &[data-has-child-rows] { + &[data-has-child-items] { --padding: 0px; } diff --git a/packages/react-aria-components/src/Tree.tsx b/packages/react-aria-components/src/Tree.tsx index 072d041d489..24c4a24e80b 100644 --- a/packages/react-aria-components/src/Tree.tsx +++ b/packages/react-aria-components/src/Tree.tsx @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {AriaTreeGridListProps, useTreeGridList, useTreeGridListItem} from '@react-aria/tree'; +import {AriaTreeGridListItemOptions, AriaTreeGridListProps, useTreeGridList, useTreeGridListItem} from '@react-aria/tree'; import {ButtonContext} from './Button'; import {CheckboxContext} from './RSPContexts'; import {Collection, CollectionBuilder, CollectionNode, createBranchComponent, createLeafComponent, useCachedChildren} from '@react-aria/collections'; @@ -271,8 +271,8 @@ export interface TreeItemRenderProps extends Omit(null); -export interface TreeItemProps extends StyleRenderProps, LinkDOMProps, HoverEvents { +export interface TreeItemProps extends StyleRenderProps, LinkDOMProps, HoverEvents, Pick { /** The unique id of the tree row. */ id?: Key, /** The object value that this tree item represents. When using dynamic collections, this is set automatically. */ @@ -325,7 +325,7 @@ export const UNSTABLE_TreeItem = /*#__PURE__*/ createBranchComponent('item', 1; + let hasChildItems = props.hasChildItems || [...state.collection.getChildren!(item.key)]?.length > 1;; let level = rowProps['aria-level'] || 1; let {hoverProps, isHovered} = useHover({ @@ -350,14 +350,14 @@ export const UNSTABLE_TreeItem = /*#__PURE__*/ createBranchComponent('item', (null); useEffect(() => { - if (hasChildRows && !expandButtonRef.current) { + if (hasChildItems && !expandButtonRef.current) { console.warn('Expandable tree items must contain a expand button so screen reader users can expand/collapse the item.'); } // eslint-disable-next-line @@ -410,8 +410,8 @@ export const UNSTABLE_TreeItem = /*#__PURE__*/ createBranchComponent('item', { hovered: isHovered })}> - {({isExpanded, hasChildRows, level, selectionMode, selectionBehavior}) => ( + {({isExpanded, hasChildItems, level, selectionMode, selectionBehavior}) => ( <> {selectionMode !== 'none' && selectionBehavior === 'toggle' && ( )}
- {hasChildRows && } + style={{marginInlineStart: `${(!hasChildItems ? 20 : 0) + (level - 1) * 15}px`}}> + {hasChildItems && } {props.title || props.children} @@ -230,13 +230,13 @@ const DynamicTreeItem = (props: DynamicTreeItemProps) => { hovered: isHovered })}> - {({isExpanded, hasChildRows, level, selectionBehavior, selectionMode}) => ( + {({isExpanded, hasChildItems, level, selectionBehavior, selectionMode}) => ( <> {selectionMode !== 'none' && selectionBehavior === 'toggle' && ( )} -
- {hasChildRows && } +
+ {hasChildItems && } {props.children} @@ -429,13 +429,13 @@ const DynamicTreeItemWithButtonLoader = (props: DynamicTreeItemProps) => { hovered: isHovered })}> - {({isExpanded, hasChildRows, level, selectionBehavior, selectionMode}) => ( + {({isExpanded, hasChildItems, level, selectionBehavior, selectionMode}) => ( <> {selectionMode !== 'none' && selectionBehavior === 'toggle' && ( )} -
- {hasChildRows && } +
+ {hasChildItems && } {props.children} diff --git a/packages/react-aria-components/test/Table.test.js b/packages/react-aria-components/test/Table.test.js index 5c5f5e61228..e1e6c3c1c4d 100644 --- a/packages/react-aria-components/test/Table.test.js +++ b/packages/react-aria-components/test/Table.test.js @@ -668,10 +668,8 @@ describe('Table', () => { let tableTester = testUtilUser.createTester('Table', {root: getByRole('grid')}); await user.tab(); - fireEvent.keyDown(document.activeElement, {key: 'ArrowDown'}); - fireEvent.keyUp(document.activeElement, {key: 'ArrowDown'}); - fireEvent.keyDown(document.activeElement, {key: 'ArrowRight'}); - fireEvent.keyUp(document.activeElement, {key: 'ArrowRight'}); + await user.keyboard('{ArrowDown}'); + await user.keyboard('{ArrowRight}'); let gridRows = tableTester.rows; expect(gridRows).toHaveLength(4); diff --git a/packages/react-aria-components/test/Tree.test.tsx b/packages/react-aria-components/test/Tree.test.tsx index ebe938a635f..366782d3136 100644 --- a/packages/react-aria-components/test/Tree.test.tsx +++ b/packages/react-aria-components/test/Tree.test.tsx @@ -31,12 +31,12 @@ let StaticTreeItem = (props) => { return ( - {({isExpanded, hasChildRows, selectionMode, selectionBehavior}) => ( + {({isExpanded, hasChildItems, selectionMode, selectionBehavior}) => ( <> {(selectionMode !== 'none' || props.href != null) && selectionBehavior === 'toggle' && ( )} - {hasChildRows && } + {hasChildItems && } {props.title || props.children} @@ -101,12 +101,12 @@ let DynamicTreeItem = (props) => { return ( - {({isExpanded, hasChildRows, selectionMode, selectionBehavior}) => ( + {({isExpanded, hasChildItems, selectionMode, selectionBehavior}) => ( <> {(selectionMode !== 'none' || props.href != null) && selectionBehavior === 'toggle' && ( )} - {hasChildRows && } + {hasChildItems && } {props.title || props.children} @@ -217,7 +217,7 @@ describe('Tree', () => { expect(rowNoChild).not.toHaveAttribute('aria-expanded'); expect(rowNoChild).not.toHaveAttribute('data-expanded'); expect(rowNoChild).toHaveAttribute('data-level', '1'); - expect(rowNoChild).not.toHaveAttribute('data-has-child-rows'); + expect(rowNoChild).not.toHaveAttribute('data-has-child-items'); expect(rowNoChild).toHaveAttribute('data-rac'); let rowWithChildren = rows[1]; @@ -225,35 +225,35 @@ describe('Tree', () => { expect(rowWithChildren).toHaveAttribute('aria-label', 'Projects'); expect(rowWithChildren).toHaveAttribute('data-expanded', 'true'); expect(rowWithChildren).toHaveAttribute('data-level', '1'); - expect(rowWithChildren).toHaveAttribute('data-has-child-rows', 'true'); + expect(rowWithChildren).toHaveAttribute('data-has-child-items', 'true'); expect(rowWithChildren).toHaveAttribute('data-rac'); let level2ChildRow = rows[2]; expect(level2ChildRow).toHaveAttribute('aria-label', 'Projects-1'); expect(level2ChildRow).toHaveAttribute('data-expanded', 'true'); expect(level2ChildRow).toHaveAttribute('data-level', '2'); - expect(level2ChildRow).toHaveAttribute('data-has-child-rows', 'true'); + expect(level2ChildRow).toHaveAttribute('data-has-child-items', 'true'); expect(level2ChildRow).toHaveAttribute('data-rac'); let level3ChildRow = rows[3]; expect(level3ChildRow).toHaveAttribute('aria-label', 'Projects-1A'); expect(level3ChildRow).not.toHaveAttribute('data-expanded'); expect(level3ChildRow).toHaveAttribute('data-level', '3'); - expect(level3ChildRow).not.toHaveAttribute('data-has-child-rows'); + expect(level3ChildRow).not.toHaveAttribute('data-has-child-items'); expect(level3ChildRow).toHaveAttribute('data-rac'); let level2ChildRow2 = rows[4]; expect(level2ChildRow2).toHaveAttribute('aria-label', 'Projects-2'); expect(level2ChildRow2).not.toHaveAttribute('data-expanded'); expect(level2ChildRow2).toHaveAttribute('data-level', '2'); - expect(level2ChildRow2).not.toHaveAttribute('data-has-child-rows'); + expect(level2ChildRow2).not.toHaveAttribute('data-has-child-items'); expect(level2ChildRow2).toHaveAttribute('data-rac'); let level2ChildRow3 = rows[5]; expect(level2ChildRow3).toHaveAttribute('aria-label', 'Projects-3'); expect(level2ChildRow3).not.toHaveAttribute('data-expanded'); expect(level2ChildRow3).toHaveAttribute('data-level', '2'); - expect(level2ChildRow3).not.toHaveAttribute('data-has-child-rows'); + expect(level2ChildRow3).not.toHaveAttribute('data-has-child-items'); expect(level2ChildRow3).toHaveAttribute('data-rac'); }); @@ -261,7 +261,7 @@ describe('Tree', () => { let {getAllByRole} = render(); let rows = getAllByRole('row'); - expect(rows[1]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[1]).toHaveAttribute('data-has-child-items', 'true'); }); it('should support dynamic trees', () => { @@ -278,28 +278,28 @@ describe('Tree', () => { expect(rows[0]).toHaveAttribute('aria-level', '1'); expect(rows[0]).toHaveAttribute('aria-posinset', '1'); expect(rows[0]).toHaveAttribute('aria-setsize', '2'); - expect(rows[0]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[0]).toHaveAttribute('data-has-child-items', 'true'); expect(rows[2]).toHaveAttribute('aria-label', 'Project 2'); expect(rows[2]).toHaveAttribute('aria-expanded', 'true'); expect(rows[2]).toHaveAttribute('aria-level', '2'); expect(rows[2]).toHaveAttribute('aria-posinset', '2'); expect(rows[2]).toHaveAttribute('aria-setsize', '5'); - expect(rows[2]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[2]).toHaveAttribute('data-has-child-items', 'true'); expect(rows[8]).toHaveAttribute('aria-label', 'Project 5'); expect(rows[8]).toHaveAttribute('aria-expanded', 'true'); expect(rows[8]).toHaveAttribute('aria-level', '2'); expect(rows[8]).toHaveAttribute('aria-posinset', '5'); expect(rows[8]).toHaveAttribute('aria-setsize', '5'); - expect(rows[8]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[8]).toHaveAttribute('data-has-child-items', 'true'); expect(rows[12]).toHaveAttribute('aria-label', 'Reports'); expect(rows[12]).toHaveAttribute('aria-expanded', 'true'); expect(rows[12]).toHaveAttribute('aria-level', '1'); expect(rows[12]).toHaveAttribute('aria-posinset', '2'); expect(rows[12]).toHaveAttribute('aria-setsize', '2'); - expect(rows[12]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[12]).toHaveAttribute('data-has-child-items', 'true'); expect(rows[16]).toHaveAttribute('aria-label', 'Reports 1ABC'); expect(rows[16]).toHaveAttribute('aria-level', '5'); @@ -843,14 +843,14 @@ describe('Tree', () => { await user.tab(); expect(document.activeElement).toBe(rows[0]); expect(rows[0]).toHaveAttribute('data-expanded', 'true'); - expect(rows[0]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[0]).toHaveAttribute('data-has-child-items', 'true'); expect(onExpandedChange).toHaveBeenCalledTimes(0); // Check we can open/close a top level row await trigger(rows[0], 'Enter'); expect(document.activeElement).toBe(rows[0]); expect(rows[0]).not.toHaveAttribute('data-expanded'); - expect(rows[0]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[0]).toHaveAttribute('data-has-child-items', 'true'); expect(onExpandedChange).toHaveBeenCalledTimes(1); // Note that the children of the parent row will still be in the "expanded" array expect(new Set(onExpandedChange.mock.calls[0][0])).toEqual(new Set(['project-2', 'project-5', 'reports', 'reports-1', 'reports-1A', 'reports-1AB'])); @@ -860,7 +860,7 @@ describe('Tree', () => { await trigger(rows[0], 'Enter'); expect(document.activeElement).toBe(rows[0]); expect(rows[0]).toHaveAttribute('data-expanded', 'true'); - expect(rows[0]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[0]).toHaveAttribute('data-has-child-items', 'true'); expect(onExpandedChange).toHaveBeenCalledTimes(2); expect(new Set(onExpandedChange.mock.calls[1][0])).toEqual(new Set(['projects', 'project-2', 'project-5', 'reports', 'reports-1', 'reports-1A', 'reports-1AB'])); rows = getAllByRole('row'); @@ -870,15 +870,15 @@ describe('Tree', () => { await user.keyboard('{ArrowDown}'); expect(document.activeElement).toBe(rows[2]); expect(rows[2]).toHaveAttribute('data-expanded', 'true'); - expect(rows[2]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[2]).toHaveAttribute('data-has-child-items', 'true'); // Check we can close a nested row and it doesn't affect the parent await trigger(rows[2], 'ArrowLeft'); expect(document.activeElement).toBe(rows[2]); expect(rows[2]).not.toHaveAttribute('data-expanded'); - expect(rows[2]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[2]).toHaveAttribute('data-has-child-items', 'true'); expect(rows[0]).toHaveAttribute('data-expanded', 'true'); - expect(rows[0]).toHaveAttribute('data-has-child-rows', 'true'); + expect(rows[0]).toHaveAttribute('data-has-child-items', 'true'); expect(onExpandedChange).toHaveBeenCalledTimes(3); expect(new Set(onExpandedChange.mock.calls[2][0])).toEqual(new Set(['projects', 'project-5', 'reports', 'reports-1', 'reports-1A', 'reports-1AB'])); rows = getAllByRole('row'); @@ -1297,12 +1297,12 @@ let ControlledDynamicTreeItem = (props) => { return ( - {({isExpanded, hasChildRows, selectionMode, selectionBehavior}) => ( + {({isExpanded, hasChildItems, selectionMode, selectionBehavior}) => ( <> {(selectionMode !== 'none' || props.href != null) && selectionBehavior === 'toggle' && ( )} - {hasChildRows && } + {hasChildItems && } {props.title || props.children} From b519ebdaa285e00ab3e946ad15b385e1128abf5c Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 12 Feb 2025 11:39:00 +1100 Subject: [PATCH 13/16] match rac logic with hook --- packages/@react-aria/gridlist/src/useGridListItem.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-aria/gridlist/src/useGridListItem.ts b/packages/@react-aria/gridlist/src/useGridListItem.ts index 0e83bc86839..42064ecee4b 100644 --- a/packages/@react-aria/gridlist/src/useGridListItem.ts +++ b/packages/@react-aria/gridlist/src/useGridListItem.ts @@ -94,7 +94,7 @@ export function useGridListItem(props: AriaGridListItemOptions, state: ListSt // TODO: ideally node.hasChildNodes would be a way to tell if a row has child nodes, but the row's contents make it so that value is always // true... let children = state.collection.getChildren?.(node.key); - hasChildRows = [...(children ?? [])].length > 1; + hasChildRows = hasChildRows || [...(children ?? [])].length > 1; if (onAction == null && !hasLink && state.selectionManager.selectionMode === 'none' && hasChildRows) { onAction = () => state.toggleKey(node.key); From 5af1fd357272cbbbd5684e115486ad2909d2b52b Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 12 Feb 2025 13:52:03 +1100 Subject: [PATCH 14/16] Fix spacing regression --- packages/@react-spectrum/s2/src/TreeView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index e67da841209..7c138028884 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -348,7 +348,7 @@ export const TreeItemContent = (props: Omit & width: '[calc(calc(var(--tree-item-level, 0) - 1) * var(--indent))]' })} /> {/* TODO: revisit when we do async loading, at the moment hasChildItems will only cause the chevron to be rendered, no aria/data attributes indicating the row's expandability are added */} - {hasChildItems && } + Date: Wed, 12 Feb 2025 14:09:41 +1100 Subject: [PATCH 15/16] fix chromatic merge --- .../s2/chromatic/TreeView.stories.tsx | 82 +++++++++++++++---- 1 file changed, 67 insertions(+), 15 deletions(-) diff --git a/packages/@react-spectrum/s2/chromatic/TreeView.stories.tsx b/packages/@react-spectrum/s2/chromatic/TreeView.stories.tsx index 580a3810d4a..964d8ce3560 100644 --- a/packages/@react-spectrum/s2/chromatic/TreeView.stories.tsx +++ b/packages/@react-spectrum/s2/chromatic/TreeView.stories.tsx @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {ActionMenu, Content, Heading, IllustratedMessage, Link, MenuItem, Text, TreeView, TreeViewItem} from '../src'; +import {ActionMenu, Content, Heading, IllustratedMessage, Link, MenuItem, Text, TreeView, TreeViewItem, TreeItemContent, Collection} from '../src'; import Delete from '../s2wf-icons/S2_Icon_Delete_20_N.svg'; import Edit from '../s2wf-icons/S2_Icon_Edit_20_N.svg'; import FileTxt from '../s2wf-icons/S2_Icon_FileText_20_N.svg'; @@ -36,7 +36,9 @@ function TreeExample(props) { disabledKeys={['projects-1']} aria-label="test static tree" expandedKeys={['projects']}> - + + + Photos @@ -49,8 +51,10 @@ function TreeExample(props) { Delete - - + + + + Projects @@ -63,7 +67,9 @@ function TreeExample(props) { Delete - + + + Projects-1 @@ -76,7 +82,9 @@ function TreeExample(props) { Delete - + + + Projects-1A @@ -89,9 +97,11 @@ function TreeExample(props) { Delete - + - + + + Projects-2 @@ -104,8 +114,10 @@ function TreeExample(props) { Delete - - + + + + Projects-3 @@ -118,8 +130,9 @@ function TreeExample(props) { Delete - + +
); @@ -207,14 +220,53 @@ let rows = [ ]} ]; +const DynamicTreeItem = (props) => { + let {childItems, name, icon} = props; + return ( + <> + + + {name} + {icon} + + + + Edit + + + + Delete + + + + + {(item: any) => ( + + {item.name} + + )} + + + + ); +}; + const TreeExampleDynamic = (args) => (
{(item: any) => ( - - {item.name} - {item.icon} - + )}
From 2cf6ef1d523b94ccd78cc7c10f3c2ec7379ac51a Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 12 Feb 2025 14:19:10 +1100 Subject: [PATCH 16/16] fix lint --- .../s2/chromatic/TreeView.stories.tsx | 138 +++++++++--------- 1 file changed, 69 insertions(+), 69 deletions(-) diff --git a/packages/@react-spectrum/s2/chromatic/TreeView.stories.tsx b/packages/@react-spectrum/s2/chromatic/TreeView.stories.tsx index 964d8ce3560..f2fff3b9725 100644 --- a/packages/@react-spectrum/s2/chromatic/TreeView.stories.tsx +++ b/packages/@react-spectrum/s2/chromatic/TreeView.stories.tsx @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {ActionMenu, Content, Heading, IllustratedMessage, Link, MenuItem, Text, TreeView, TreeViewItem, TreeItemContent, Collection} from '../src'; +import {ActionMenu, Collection, Content, Heading, IllustratedMessage, Link, MenuItem, Text, TreeItemContent, TreeView, TreeViewItem} from '../src'; import Delete from '../s2wf-icons/S2_Icon_Delete_20_N.svg'; import Edit from '../s2wf-icons/S2_Icon_Edit_20_N.svg'; import FileTxt from '../s2wf-icons/S2_Icon_FileText_20_N.svg'; @@ -37,40 +37,9 @@ function TreeExample(props) { aria-label="test static tree" expandedKeys={['projects']}> - - - Photos - - - - - Edit - - - - Delete - - - - - - - Projects - - - - - Edit - - - - Delete - - - - + - Projects-1 + Photos @@ -83,9 +52,73 @@ function TreeExample(props) { - + + + + Projects + + + + + Edit + + + + Delete + + + + - Projects-1A + Projects-1 + + + + + Edit + + + + Delete + + + + + + Projects-1A + + + + + Edit + + + + Delete + + + + + + + + Projects-2 + + + + + Edit + + + + Delete + + + + + + + Projects-3 @@ -100,39 +133,6 @@ function TreeExample(props) { - - - Projects-2 - - - - - Edit - - - - Delete - - - - - - - Projects-3 - - - - - Edit - - - - Delete - - - - -
);