From 81aa028c6ebaa62c2b90a2305d8be8bdaae057b1 Mon Sep 17 00:00:00 2001 From: GitHub Date: Tue, 5 Nov 2024 16:38:05 -0600 Subject: [PATCH 01/31] Render a TreeView --- packages/@react-spectrum/s2/src/TreeView.tsx | 339 ++++++++++++++++++ packages/@react-spectrum/s2/src/index.ts | 2 + .../s2/stories/TreeView.stories.tsx | 227 ++++++++++++ 3 files changed, 568 insertions(+) create mode 100644 packages/@react-spectrum/s2/src/TreeView.tsx create mode 100644 packages/@react-spectrum/s2/stories/TreeView.stories.tsx diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx new file mode 100644 index 00000000000..14acd18cd04 --- /dev/null +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -0,0 +1,339 @@ +/* + * Copyright 2024 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {AriaTreeGridListProps} from '@react-aria/tree'; +import {ButtonContext, Collection, TreeItemContentRenderProps, TreeItemProps, TreeItemRenderProps, TreeRenderProps, UNSTABLE_Tree, UNSTABLE_TreeItem, UNSTABLE_TreeItemContent, useContextProps} from 'react-aria-components'; +import {Checkbox, IconContext, Text, TextContext} from '@react-spectrum/s2'; +import Chevron from '../ui-icons/Chevron'; +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 {useDOMRef, useStyleProps} from '@react-spectrum/utils'; +import {focusRing, style} from '../style' with {type: 'macro'}; +import {useButton} from '@react-aria/button'; +import {useLocale} from '@react-aria/i18n'; +import {Provider} from 'react-aria-components'; + +export interface SpectrumTreeViewProps extends Omit, 'children'>, StyleProps, SpectrumSelectionProps, Expandable { + /** Provides content to display when there are no items in the tree. */ + renderEmptyState?: () => JSX.Element, + /** + * Handler that is called when a user performs an action on an item. The exact user event depends on + * the collection's `selectionStyle` prop and the interaction modality. + */ + onAction?: (key: Key) => void, + /** + * The contents of the tree. + */ + children?: ReactNode | ((item: T) => ReactNode) +} + +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 +} + +interface TreeRendererContextValue { + renderer?: (item) => ReactElement> +} +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 +// keyboard focus ring. Perhaps find a different way of rendering the outlines since the top of the item doesn't +// scroll into view due to how the ring is offset. Alternatively, have the tree render the top/bottom outline like it does in Listview +const tree = style>({ + borderWidth: 2, + boxSizing: 'border-box', + borderXWidth: 0, + borderStyle: 'solid', + borderColor: { + default: 'transparent', + forcedColors: 'Background' + }, + justifyContent: { + isEmpty: 'center' + }, + alignItems: { + isEmpty: 'center' + }, + width: { + isEmpty: 'full' + }, + height: { + isEmpty: 'full' + }, + display: { + isEmpty: 'flex' + } +}); + +function TreeView(props: SpectrumTreeViewProps, ref: DOMRef) { + let {children, selectionStyle} = props; + + let renderer; + if (typeof children === 'function') { + renderer = children; + } + + let {styleProps} = useStyleProps(props); + let domRef = useDOMRef(ref); + let selectionBehavior = selectionStyle === 'highlight' ? 'replace' : 'toggle'; + + return ( + + tree({isEmpty})} selectionBehavior={selectionBehavior as SelectionBehavior} ref={domRef}> + {props.children} + + + ); +} + +interface TreeRowRenderProps extends TreeItemRenderProps { + isLink?: boolean +} + +const treeRow = style({ + position: 'relative', + display: 'flex', + height: 40, + width: 'full', + boxSizing: 'border-box', + font: 'ui', + color: 'body', + outlineStyle: 'none', + cursor: { + default: 'default', + isLink: 'pointer' + }, + // TODO: not sure where to get the equivalent colors here, for instance isHovered is spectrum 600 with 10% opacity but I don't think that exists in the theme + backgroundColor: { + isHovered: '[var(--spectrum-table-row-background-color-hover)]', + isFocusVisibleWithin: '[var(--spectrum-table-row-background-color-hover)]', + isPressed: '[var(--spectrum-table-row-background-color-down)]', + isSelected: '[var(--spectrum-table-row-background-color-selected)]' + } +}); + +const treeCellGrid = style({ + display: 'grid', + width: 'full', + alignItems: 'center', + gridTemplateColumns: ['minmax(0, auto)', 'minmax(0, auto)', 'minmax(0, auto)', 10, 'minmax(0, auto)', '1fr', 'minmax(0, auto)', 'auto'], + gridTemplateRows: '1fr', + gridTemplateAreas: [ + 'drag-handle checkbox level-padding expand-button icon content actions actionmenu' + ], + color: { + isDisabled: { + default: 'gray-400', + forcedColors: 'GrayText' + } + } +}); + +// TODO: These styles lose against the spectrum class names, so I've did unsafe for the ones that get overridden +const treeCheckbox = style({ + gridArea: 'checkbox', + transitionDuration: '160ms', + marginStart: 12, + marginEnd: 0 +}); + +const treeIcon = style({ + gridArea: 'icon', + marginEnd: 2 +}); + +const treeContent = style>({ + gridArea: 'content', + textOverflow: 'ellipsis', + whiteSpace: 'nowrap', + overflow: 'hidden' +}); + +const treeActions = style({ + gridArea: 'actions', + flexGrow: 0, + flexShrink: 0, + marginStart: 2, + marginEnd: 2 +}); + +const treeActionMenu = style({ + gridArea: 'actionmenu', + width: 8 +}); + +const treeRowOutline = style({ + ...focusRing(), + content: '', + display: 'block', + position: 'absolute', + insetStart: 0, + insetEnd: 0, + top: { + default: 0, + isFocusVisible: '[-2px]', + isSelected: { + default: '[-1px]', + isFocusVisible: '[-2px]' + } + }, + bottom: 0, + pointerEvents: 'none', + forcedColorAdjust: 'none', +}); + +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 + + 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} + + ); + } + + 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' && ( + // 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} + +
+
+ )} + + {nestedRows} + + ); +}; + +interface ExpandableRowChevronProps { + isExpanded?: boolean, + isDisabled?: boolean, + isRTL?: boolean +} + +const expandButton = style({ + gridArea: 'expand-button', + height: 'full', + aspectRatio: 'square', + display: 'flex', + flexWrap: 'wrap', + alignContent: 'center', + justifyContent: 'center', + outlineStyle: 'none', + transform: { + isExpanded: { + default: 'rotate(90deg)', + isRTL: 'rotate(-90deg)' + } + }, + transition: '[transform ease var(--spectrum-global-animation-duration-100)]' +}); + +function ExpandableRowChevron(props: ExpandableRowChevronProps) { + let expandButtonRef = useRef(null); + let [fullProps, ref] = useContextProps({...props, slot: 'chevron'}, expandButtonRef, ButtonContext); + let {isExpanded, isDisabled} = fullProps; + let {direction} = useLocale(); + + // Will need to keep the chevron as a button for iOS VO at all times since VO doesn't focus the cell. Also keep as button if cellAction is defined by the user in the future + let {buttonProps} = useButton({ + ...fullProps, + elementType: 'span' + }, ref); + + return ( + + + + ); +} + +/** + * A tree view provides users with a way to navigate nested hierarchical information. + */ +const _TreeView = React.forwardRef(TreeView) as (props: SpectrumTreeViewProps & {ref?: DOMRef}) => ReactElement; +export {_TreeView as TreeView}; diff --git a/packages/@react-spectrum/s2/src/index.ts b/packages/@react-spectrum/s2/src/index.ts index c13c5a51afe..6055e69ffb7 100644 --- a/packages/@react-spectrum/s2/src/index.ts +++ b/packages/@react-spectrum/s2/src/index.ts @@ -69,6 +69,7 @@ export {TagGroup, Tag, TagGroupContext} from './TagGroup'; export {TextArea, TextField, TextAreaContext, TextFieldContext} from './TextField'; export {ToggleButton, ToggleButtonContext} from './ToggleButton'; export {Tooltip, TooltipTrigger} from './Tooltip'; +export {TreeView, TreeViewItem} from './TreeView'; export {pressScale} from './pressScale'; @@ -130,4 +131,5 @@ export type {TagGroupProps, TagProps} from './TagGroup'; export type {TextFieldProps, TextAreaProps} from './TextField'; export type {ToggleButtonProps} from './ToggleButton'; export type {TooltipProps} from './Tooltip'; +export type {SpectrumTreeViewProps, SpectrumTreeViewItemProps} from './TreeView'; export type {FileTriggerProps, TooltipTriggerComponentProps as TooltipTriggerProps} from 'react-aria-components'; diff --git a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx new file mode 100644 index 00000000000..4bbdeb315c0 --- /dev/null +++ b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx @@ -0,0 +1,227 @@ +/** + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {action} from '@storybook/addon-actions'; +import { + // ActionMenu, + // MenuItem, + Content, + Heading, + Text, + IllustratedMessage, + Link, + TreeView, + TreeViewItem} from '../src'; +// import Add from '../s2wf-icons/S2_Icon_Add_20_N.svg'; +// 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'; +import Folder from '../s2wf-icons/S2_Icon_Folder_20_N.svg'; +import React from 'react'; + +let onActionFunc = action('onAction'); +let noOnAction = null; +const onActionOptions = {onActionFunc, noOnAction}; + +import {categorizeArgTypes} from './utils'; +import type {Meta, StoryObj} from '@storybook/react'; + +const meta: Meta = { + component: TreeView, + parameters: { + layout: 'centered' + }, + tags: ['autodocs'], + args: { + // Make sure onAction isn't autogenerated + // @ts-ignore + onAction: null + }, + argTypes: { + ...categorizeArgTypes('Events', ['onAction', 'onSelectionChange']), + onAction: { + options: Object.keys(onActionOptions), // An array of serializable values + mapping: onActionOptions, // Maps serializable option values to complex arg values + control: { + type: 'select', // Type 'select' is automatically inferred when 'options' is defined + labels: { + // 'labels' maps option values to string labels + onActionFunc: 'onAction enabled', + noOnAction: 'onAction disabled' + } + }, + table: {category: 'Events'} + } + } +}; + +export default meta; +type Story = StoryObj; + +const TreeExampleStatic = (args) => ( +
+ + + Photos + + {/* + + + Edit + + + + Delete + + */} + + + Projects + + {/* + + + Edit + + + + Delete + + */} + + Projects-1 + + {/* + + + Edit + + + + Delete + + */} + + Projects-1A + + {/* + + + Edit + + + + Delete + + */} + + + + Projects-2 + + {/* + + + Edit + + + + Delete + + */} + + + Projects-3 + + {/* + + + Edit + + + + Delete + + */} + + + +
+); + +export const Example = { + render: TreeExampleStatic, + args: { + selectionMode: 'multiple' + } +}; + +let rows = [ + {id: 'projects', name: 'Projects', icon: , childItems: [ + {id: 'project-1', name: 'Project 1', icon: }, + {id: 'project-2', name: 'Project 2', icon: , childItems: [ + {id: 'project-2A', name: 'Project 2A', icon: }, + {id: 'project-2B', name: 'Project 2B', icon: }, + {id: 'project-2C', name: 'Project 2C', icon: } + ]}, + {id: 'project-3', name: 'Project 3', icon: }, + {id: 'project-4', name: 'Project 4', icon: }, + {id: 'project-5', name: 'Project 5', icon: , childItems: [ + {id: 'project-5A', name: 'Project 5A', icon: }, + {id: 'project-5B', name: 'Project 5B', icon: }, + {id: 'project-5C', name: 'Project 5C', icon: } + ]} + ]}, + {id: 'reports', name: 'Reports', icon: , childItems: [ + {id: 'reports-1', name: 'Reports 1', icon: , childItems: [ + {id: 'reports-1A', name: 'Reports 1A', icon: , childItems: [ + {id: 'reports-1AB', name: 'Reports 1AB', icon: , childItems: [ + {id: 'reports-1ABC', name: 'Reports 1ABC', icon: } + ]} + ]}, + {id: 'reports-1B', name: 'Reports 1B', icon: }, + {id: 'reports-1C', name: 'Reports 1C', icon: } + ]}, + {id: 'reports-2', name: 'Reports 2', icon: } + ]} +]; + +const TreeExampleDynamic = (args) => ( +
+ + {(item: any) => ( + + {item.name} + {item.icon} + {/* + + + Edit + + + + Delete + + */} + + )} + +
+); + +export const Dynamic = { + render: TreeExampleDynamic, + args: { + ...Example.args, + disabledKeys: ['Foo 5'] + } +}; From 8dbce274e3716a57d9e1313f21dead42dc13906e Mon Sep 17 00:00:00 2001 From: GitHub Date: Fri, 8 Nov 2024 15:24:12 -0600 Subject: [PATCH 02/31] Correct styles --- packages/@react-spectrum/s2/src/TreeView.tsx | 60 ++++++++++---------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 14acd18cd04..51797bf9c34 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -130,6 +130,10 @@ const treeRow = style({ isFocusVisibleWithin: '[var(--spectrum-table-row-background-color-hover)]', isPressed: '[var(--spectrum-table-row-background-color-down)]', isSelected: '[var(--spectrum-table-row-background-color-selected)]' + }, + '--indent': { + type: 'width', + value: 16 } }); @@ -137,10 +141,10 @@ const treeCellGrid = style({ display: 'grid', width: 'full', alignItems: 'center', - gridTemplateColumns: ['minmax(0, auto)', 'minmax(0, auto)', 'minmax(0, auto)', 10, 'minmax(0, auto)', '1fr', 'minmax(0, auto)', 'auto'], + gridTemplateColumns: ['minmax(0, auto)', 'minmax(0, auto)', 'minmax(0, auto)', 40, 'minmax(0, auto)', '1fr'], gridTemplateRows: '1fr', gridTemplateAreas: [ - 'drag-handle checkbox level-padding expand-button icon content actions actionmenu' + 'drag-handle checkbox level-padding expand-button icon content' ], color: { isDisabled: { @@ -150,17 +154,16 @@ const treeCellGrid = style({ } }); -// TODO: These styles lose against the spectrum class names, so I've did unsafe for the ones that get overridden const treeCheckbox = style({ gridArea: 'checkbox', - transitionDuration: '160ms', marginStart: 12, - marginEnd: 0 + marginEnd: 0, + paddingEnd: 0 }); const treeIcon = style({ gridArea: 'icon', - marginEnd: 2 + marginEnd: 'text-to-visual' }); const treeContent = style>({ @@ -170,19 +173,6 @@ const treeContent = style>({ overflow: 'hidden' }); -const treeActions = style({ - gridArea: 'actions', - flexGrow: 0, - flexShrink: 0, - marginStart: 2, - marginEnd: 2 -}); - -const treeActionMenu = style({ - gridArea: 'actionmenu', - width: 8 -}); - const treeRowOutline = style({ ...focusRing(), content: '', @@ -252,19 +242,23 @@ export const TreeViewItem = (props: SpectrumTreeViewItemProps<
{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} @@ -298,7 +292,7 @@ const expandButton = style({ isRTL: 'rotate(-90deg)' } }, - transition: '[transform ease var(--spectrum-global-animation-duration-100)]' + transition: 'default' }); function ExpandableRowChevron(props: ExpandableRowChevronProps) { @@ -321,11 +315,15 @@ function ExpandableRowChevron(props: ExpandableRowChevronProps) { tabIndex={isAndroid() && !isDisabled ? -1 : undefined} className={expandButton({isExpanded, isDisabled, isRTL: direction === 'rtl'})}> From b297017e4073292074291b10fbeb99e4da3a1db0 Mon Sep 17 00:00:00 2001 From: GitHub Date: Fri, 8 Nov 2024 15:39:13 -0600 Subject: [PATCH 03/31] update styles to better fix s2 --- packages/@react-spectrum/s2/package.json | 2 + packages/@react-spectrum/s2/src/TreeView.tsx | 55 +++++++++++-------- .../s2/stories/TreeView.stories.tsx | 30 +++++----- 3 files changed, 50 insertions(+), 37 deletions(-) diff --git a/packages/@react-spectrum/s2/package.json b/packages/@react-spectrum/s2/package.json index 82edf2ac590..110c9ec15fb 100644 --- a/packages/@react-spectrum/s2/package.json +++ b/packages/@react-spectrum/s2/package.json @@ -124,9 +124,11 @@ "@react-aria/test-utils": "1.0.0-alpha.1" }, "dependencies": { + "@react-aria/button": "^3.10.1", "@react-aria/collections": "3.0.0-alpha.5", "@react-aria/i18n": "^3.12.3", "@react-aria/interactions": "^3.22.4", + "@react-aria/tree": "3.0.0-beta.1", "@react-aria/utils": "^3.25.3", "@react-spectrum/utils": "^3.11.11", "@react-stately/layout": "^4.0.3", diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 51797bf9c34..0d3939c77a6 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -11,17 +11,27 @@ */ 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, + Collection, + Provider, + TreeItemProps, + TreeItemRenderProps, + TreeRenderProps, + UNSTABLE_Tree, + UNSTABLE_TreeItem, + UNSTABLE_TreeItemContent, + useContextProps +} from 'react-aria-components'; import {Checkbox, IconContext, Text, TextContext} from '@react-spectrum/s2'; import Chevron from '../ui-icons/Chevron'; import {DOMRef, Expandable, Key, SelectionBehavior, SpectrumSelectionProps, StyleProps} from '@react-types/shared'; +import {focusRing, style} from '../style' with {type: 'macro'}; import {isAndroid} from '@react-aria/utils'; import React, {createContext, isValidElement, JSX, JSXElementConstructor, ReactElement, ReactNode, useContext, useRef} from 'react'; -import {useDOMRef, useStyleProps} from '@react-spectrum/utils'; -import {focusRing, style} from '../style' with {type: 'macro'}; import {useButton} from '@react-aria/button'; +import {useDOMRef} from '@react-spectrum/utils'; import {useLocale} from '@react-aria/i18n'; -import {Provider} from 'react-aria-components'; export interface SpectrumTreeViewProps extends Omit, 'children'>, StyleProps, SpectrumSelectionProps, Expandable { /** Provides content to display when there are no items in the tree. */ @@ -94,13 +104,12 @@ function TreeView(props: SpectrumTreeViewProps, ref: DOMRef renderer = children; } - let {styleProps} = useStyleProps(props); let domRef = useDOMRef(ref); let selectionBehavior = selectionStyle === 'highlight' ? 'replace' : 'toggle'; return ( - tree({isEmpty})} selectionBehavior={selectionBehavior as SelectionBehavior} ref={domRef}> + tree({isEmpty})} selectionBehavior={selectionBehavior as SelectionBehavior} ref={domRef}> {props.children} @@ -166,7 +175,7 @@ const treeIcon = style({ marginEnd: 'text-to-visual' }); -const treeContent = style>({ +const treeContent = style({ gridArea: 'content', textOverflow: 'ellipsis', whiteSpace: 'nowrap', @@ -190,7 +199,7 @@ const treeRowOutline = style({ }, bottom: 0, pointerEvents: 'none', - forcedColorAdjust: 'none', + forcedColorAdjust: 'none' }); export const TreeViewItem = (props: SpectrumTreeViewItemProps) => { @@ -238,7 +247,7 @@ export const TreeViewItem = (props: SpectrumTreeViewItemProps< isLink: !!href })}> - {({isExpanded, hasChildRows, level, selectionMode, selectionBehavior, isDisabled, isSelected, isFocusVisible}) => ( + {({isExpanded, hasChildRows, selectionMode, selectionBehavior, isDisabled, isSelected, isFocusVisible}) => (
{selectionMode !== 'none' && selectionBehavior === 'toggle' && ( // TODO: add transition? @@ -251,14 +260,14 @@ export const TreeViewItem = (props: SpectrumTreeViewItemProps<
{/* 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} @@ -297,6 +306,7 @@ const expandButton = style({ function ExpandableRowChevron(props: ExpandableRowChevronProps) { let expandButtonRef = useRef(null); + // @ts-ignore - check back on this let [fullProps, ref] = useContextProps({...props, slot: 'chevron'}, expandButtonRef, ButtonContext); let {isExpanded, isDisabled} = fullProps; let {direction} = useLocale(); @@ -314,18 +324,19 @@ function ExpandableRowChevron(props: ExpandableRowChevronProps) { // Override tabindex so that grid keyboard nav skips over it. Needs -1 so android talkback can actually "focus" it tabIndex={isAndroid() && !isDisabled ? -1 : undefined} className={expandButton({isExpanded, isDisabled, isRTL: direction === 'rtl'})}> - + })({direction})} /> ); } diff --git a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx index 4bbdeb315c0..00c9a264ad6 100644 --- a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx +++ b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx @@ -2,7 +2,7 @@ * Copyright 2020 Adobe. All rights reserved. * This file is licensed to you under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * of the License at http://www.apache.org/licenses/LICENSE-2.0. * * Unless required by applicable law or agreed to in writing, software distributed under * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS @@ -11,29 +11,30 @@ */ import {action} from '@storybook/addon-actions'; -import { - // ActionMenu, - // MenuItem, - Content, - Heading, - Text, - IllustratedMessage, - Link, - TreeView, - TreeViewItem} from '../src'; +import {categorizeArgTypes} from './utils'; +import FileTxt from '../s2wf-icons/S2_Icon_FileText_20_N.svg'; // import Add from '../s2wf-icons/S2_Icon_Add_20_N.svg'; // 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'; import Folder from '../s2wf-icons/S2_Icon_Folder_20_N.svg'; +import type {Meta} from '@storybook/react'; import React from 'react'; +import { + // ActionMenu, + // MenuItem, + // Content, + // Heading, + Text, + // IllustratedMessage, + // Link, + TreeView, + TreeViewItem +} from '../src'; let onActionFunc = action('onAction'); let noOnAction = null; const onActionOptions = {onActionFunc, noOnAction}; -import {categorizeArgTypes} from './utils'; -import type {Meta, StoryObj} from '@storybook/react'; const meta: Meta = { component: TreeView, @@ -65,7 +66,6 @@ const meta: Meta = { }; export default meta; -type Story = StoryObj; const TreeExampleStatic = (args) => (
From 4f693940d68a06a54218ca7ff91aba8dfa5b2878 Mon Sep 17 00:00:00 2001 From: GitHub Date: Tue, 12 Nov 2024 16:43:17 +1100 Subject: [PATCH 04/31] fix install --- yarn.lock | 2 ++ 1 file changed, 2 insertions(+) diff --git a/yarn.lock b/yarn.lock index 707146cbede..55b1d109ea5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7653,10 +7653,12 @@ __metadata: dependencies: "@adobe/spectrum-tokens": "npm:^13.0.0-beta.48" "@parcel/macros": "npm:2.12.1-canary.3165" + "@react-aria/button": "npm:^3.10.1" "@react-aria/collections": "npm:3.0.0-alpha.5" "@react-aria/i18n": "npm:^3.12.3" "@react-aria/interactions": "npm:^3.22.4" "@react-aria/test-utils": "npm:1.0.0-alpha.1" + "@react-aria/tree": "npm:3.0.0-beta.1" "@react-aria/utils": "npm:^3.25.3" "@react-spectrum/utils": "npm:^3.11.11" "@react-stately/layout": "npm:^4.0.3" From 98654db0483f45b184367f853fee0be5578dde93 Mon Sep 17 00:00:00 2001 From: GitHub Date: Tue, 12 Nov 2024 17:38:01 +1100 Subject: [PATCH 05/31] fix lint, add preliminary test --- packages/@react-spectrum/s2/src/TreeView.tsx | 69 ++++++------ packages/@react-spectrum/s2/src/index.ts | 2 +- .../@react-spectrum/s2/test/Tree.test.tsx | 45 ++++++++ .../test/AriaTree.test-util.tsx | 104 ++++++++++++++++++ .../react-aria-components/test/Tree.test.tsx | 19 ++-- 5 files changed, 197 insertions(+), 42 deletions(-) create mode 100644 packages/@react-spectrum/s2/test/Tree.test.tsx create mode 100644 packages/react-aria-components/test/AriaTree.test-util.tsx diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 0d3939c77a6..58052b048ae 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -10,12 +10,12 @@ * governing permissions and limitations under the License. */ -import {AriaTreeGridListProps} from '@react-aria/tree'; import { ButtonContext, Collection, Provider, - TreeItemProps, + TreeItemProps as RACTreeItemProps, + TreeProps as RACTreeProps, TreeItemRenderProps, TreeRenderProps, UNSTABLE_Tree, @@ -25,31 +25,26 @@ import { } from 'react-aria-components'; import {Checkbox, IconContext, Text, TextContext} from '@react-spectrum/s2'; import Chevron from '../ui-icons/Chevron'; -import {DOMRef, Expandable, Key, SelectionBehavior, SpectrumSelectionProps, StyleProps} from '@react-types/shared'; +import {DOMRef, Key} from '@react-types/shared'; import {focusRing, style} from '../style' with {type: 'macro'}; import {isAndroid} from '@react-aria/utils'; -import React, {createContext, isValidElement, JSX, JSXElementConstructor, ReactElement, ReactNode, useContext, useRef} from 'react'; +import React, {createContext, isValidElement, JSXElementConstructor, ReactElement, useContext, useRef} from 'react'; +import {StylesPropWithHeight, UnsafeStyles} from './style-utils'; import {useButton} from '@react-aria/button'; import {useDOMRef} from '@react-spectrum/utils'; import {useLocale} from '@react-aria/i18n'; -export interface SpectrumTreeViewProps extends Omit, 'children'>, StyleProps, SpectrumSelectionProps, Expandable { - /** Provides content to display when there are no items in the tree. */ - renderEmptyState?: () => JSX.Element, - /** - * Handler that is called when a user performs an action on an item. The exact user event depends on - * the collection's `selectionStyle` prop and the interaction modality. - */ - onAction?: (key: Key) => void, - /** - * The contents of the tree. - */ - children?: ReactNode | ((item: T) => ReactNode) +interface S2TreeProps { + isDetached?: boolean, + onAction?: (key: Key) => void } -export interface SpectrumTreeViewItemProps extends Omit { - /** Rendered contents of the tree item or child items. */ - children: ReactNode, +export interface TreeViewProps extends Omit, 'style' | 'disabledBehavior' | 'className' | 'onRowAction' | 'selectionBehavior' | 'onScroll' | 'onCellAction' | 'dragAndDropHooks'>, UnsafeStyles, S2TreeProps { + /** Spectrum-defined styles, returned by the `style()` macro. */ + styles?: StylesPropWithHeight +} + +export interface TreeViewItemProps extends Omit { /** 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. */ @@ -65,6 +60,10 @@ function useTreeRendererContext(): TreeRendererContextValue { return useContext(TreeRendererContext)!; } + +let InternalTreeContext = createContext({}); + + // 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 @@ -96,8 +95,8 @@ const tree = style>({ } }); -function TreeView(props: SpectrumTreeViewProps, ref: DOMRef) { - let {children, selectionStyle} = props; +function TreeView(props: TreeViewProps, ref: DOMRef) { + let {children, isDetached} = props; let renderer; if (typeof children === 'function') { @@ -105,13 +104,14 @@ function TreeView(props: SpectrumTreeViewProps, ref: DOMRef } let domRef = useDOMRef(ref); - let selectionBehavior = selectionStyle === 'highlight' ? 'replace' : 'toggle'; return ( - tree({isEmpty})} selectionBehavior={selectionBehavior as SelectionBehavior} ref={domRef}> - {props.children} - + + tree({isEmpty})} selectionBehavior="toggle" ref={domRef}> + {props.children} + + ); } @@ -125,6 +125,7 @@ const treeRow = style({ display: 'flex', height: 40, width: 'full', + minWidth: 240, // do we really want this restriction? boxSizing: 'border-box', font: 'ui', color: 'body', @@ -187,22 +188,22 @@ const treeRowOutline = style({ content: '', display: 'block', position: 'absolute', - insetStart: 0, - insetEnd: 0, + insetStart: '[8px]', + insetEnd: '[8px]', top: { - default: 0, - isFocusVisible: '[-2px]', + default: '[8px]', + isFocusVisible: '[8px]', isSelected: { - default: '[-1px]', - isFocusVisible: '[-2px]' + default: '[1px]', + isFocusVisible: '[8px]' } }, - bottom: 0, + bottom: '[8px]', pointerEvents: 'none', forcedColorAdjust: 'none' }); -export const TreeViewItem = (props: SpectrumTreeViewItemProps) => { +export const TreeViewItem = (props: TreeViewItemProps) => { let { children, childItems, @@ -344,5 +345,5 @@ function ExpandableRowChevron(props: ExpandableRowChevronProps) { /** * A tree view provides users with a way to navigate nested hierarchical information. */ -const _TreeView = React.forwardRef(TreeView) as (props: SpectrumTreeViewProps & {ref?: DOMRef}) => ReactElement; +const _TreeView = React.forwardRef(TreeView) as (props: TreeViewProps & {ref?: DOMRef}) => ReactElement; export {_TreeView as TreeView}; diff --git a/packages/@react-spectrum/s2/src/index.ts b/packages/@react-spectrum/s2/src/index.ts index ff236395fb2..521db51aafc 100644 --- a/packages/@react-spectrum/s2/src/index.ts +++ b/packages/@react-spectrum/s2/src/index.ts @@ -135,5 +135,5 @@ export type {TextFieldProps, TextAreaProps} from './TextField'; export type {ToggleButtonProps} from './ToggleButton'; export type {ToggleButtonGroupProps} from './ToggleButtonGroup'; export type {TooltipProps} from './Tooltip'; -export type {SpectrumTreeViewProps, SpectrumTreeViewItemProps} from './TreeView'; +export type {TreeViewProps, TreeViewItemProps} from './TreeView'; export type {FileTriggerProps, TooltipTriggerComponentProps as TooltipTriggerProps} from 'react-aria-components'; diff --git a/packages/@react-spectrum/s2/test/Tree.test.tsx b/packages/@react-spectrum/s2/test/Tree.test.tsx new file mode 100644 index 00000000000..b51f865481b --- /dev/null +++ b/packages/@react-spectrum/s2/test/Tree.test.tsx @@ -0,0 +1,45 @@ + +import {AriaTreeTests} from '../../../react-aria-components/test/AriaTree.test-util'; +import FileTxt from '../s2wf-icons/S2_Icon_FileText_20_N.svg'; +import Folder from '../s2wf-icons/S2_Icon_Folder_20_N.svg'; +import React from 'react'; +import {render} from '@react-spectrum/test-utils-internal'; +import { + Text, + TreeView, + TreeViewItem +} from '../src'; + +AriaTreeTests({ + prefix: 'spectrum2-static', + renderers: { + standard: () => render( + + + Photos + + + + Projects + + + Projects-1 + + + Projects-1A + + + + + Projects-2 + + + + Projects-3 + + + + + ) + } +}); diff --git a/packages/react-aria-components/test/AriaTree.test-util.tsx b/packages/react-aria-components/test/AriaTree.test-util.tsx new file mode 100644 index 00000000000..57d444834e9 --- /dev/null +++ b/packages/react-aria-components/test/AriaTree.test-util.tsx @@ -0,0 +1,104 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {act, render} from '@testing-library/react'; +import {User} from '@react-aria/test-utils'; + +let describeInteractions = ((name, tests) => describe.each` + interactionType + ${'mouse'} + ${'keyboard'} + ${'touch'} +`(`${name} - $interactionType`, tests)); + +// @ts-ignore +describeInteractions.only = ((name, tests) => describe.only.each` + interactionType + ${'mouse'} + ${'keyboard'} + ${'touch'} +`(`${name} - $interactionType`, tests)); + +// @ts-ignore +describeInteractions.skip = ((name, tests) => describe.skip.each` + interactionType + ${'mouse'} + ${'keyboard'} + ${'touch'} +`(`${name} - $interactionType`, tests)); + +interface AriaBaseTestProps { + setup?: () => void, + prefix?: string +} +interface AriaTreeTestProps extends AriaBaseTestProps { + renderers: { + // must have label "test tree" + standard: (props?: {name: string}) => ReturnType + } +} +export const AriaTreeTests = ({renderers, setup, prefix}: AriaTreeTestProps) => { + describe(prefix ? prefix + 'AriaTree' : 'AriaTree', function () { + setup?.(); + + beforeAll(function () { + jest.useFakeTimers(); + }); + + afterEach(() => { + act(() => jest.runAllTimers()); + }); + + it('should have the base set of aria and data attributes', () => { + let {getByRole, getAllByRole} = (renderers.standard!)(); + let tree = getByRole('treegrid'); + expect(tree).toHaveAttribute('aria-label', 'test tree'); + + for (let row of getAllByRole('row')) { + expect(row).toHaveAttribute('aria-level'); + expect(row).toHaveAttribute('aria-posinset'); + expect(row).toHaveAttribute('aria-setsize'); + } + }); + + // if (renderers.singleSelection) { + // describe('single selection', function () { + // it('selects an option via mouse', async function () { + // let tree = (renderers.singleSelection!)(); + // let menuTester = testUtilUser.createTester('Menu', {user, root: tree.container}); + // let triggerButton = menuTester.trigger!; + + // await menuTester.open(); + // act(() => {jest.runAllTimers();}); + + // let menu = menuTester.menu; + // expect(menu).toBeTruthy(); + // expect(menu).toHaveAttribute('aria-labelledby', triggerButton.id); + + // let options = menuTester.options; + + // await menuTester.selectOption({option: options[1], menuSelectionMode: 'single'}); + + // act(() => {jest.runAllTimers();}); + // expect(menu).not.toBeInTheDocument(); + + // await menuTester.open(); + // act(() => {jest.runAllTimers();}); + + // options = menuTester.options; + // expect(options[0]).toHaveAttribute('aria-checked', 'false'); + // expect(options[1]).toHaveAttribute('aria-checked', 'true'); + // }); + // }); + // } + }); +}; diff --git a/packages/react-aria-components/test/Tree.test.tsx b/packages/react-aria-components/test/Tree.test.tsx index 67d82e362c7..d86feaeb24d 100644 --- a/packages/react-aria-components/test/Tree.test.tsx +++ b/packages/react-aria-components/test/Tree.test.tsx @@ -11,6 +11,7 @@ */ import {act, fireEvent, mockClickDefault, pointerMap, render, within} from '@react-spectrum/test-utils-internal'; +import {AriaTreeTests} from './AriaTree.test-util'; import {Button, Checkbox, Collection, UNSTABLE_ListLayout as ListLayout, Text, UNSTABLE_Tree, UNSTABLE_TreeItem, UNSTABLE_TreeItemContent, UNSTABLE_Virtualizer as Virtualizer} from '../'; import {composeStories} from '@storybook/react'; import React from 'react'; @@ -187,20 +188,14 @@ describe('Tree', () => { expect(tree).toHaveAttribute('style', expect.stringContaining('width: 200px')); }); - it('should have the base set of aria and data attributes', () => { + it('should have the base set of data attributes', () => { let {getByRole, getAllByRole} = render(); let tree = getByRole('treegrid'); - expect(tree).toHaveAttribute('data-rac'); - expect(tree).toHaveAttribute('aria-label', 'test tree'); expect(tree).not.toHaveAttribute('data-empty'); expect(tree).not.toHaveAttribute('data-focused'); expect(tree).not.toHaveAttribute('data-focus-visible'); for (let row of getAllByRole('row')) { - expect(row).toHaveAttribute('aria-level'); - expect(row).toHaveAttribute('data-level'); - expect(row).toHaveAttribute('aria-posinset'); - expect(row).toHaveAttribute('aria-setsize'); expect(row).toHaveAttribute('data-rac'); expect(row).not.toHaveAttribute('data-selected'); expect(row).not.toHaveAttribute('data-disabled'); @@ -1326,3 +1321,13 @@ describe('Tree', () => { }); }); }); + + +AriaTreeTests({ + prefix: 'rac-static', + renderers: { + standard: () => render( + + ) + } +}); From dec417abc13437077ad1b92e2353a75f80b9fb42 Mon Sep 17 00:00:00 2001 From: GitHub Date: Tue, 12 Nov 2024 17:38:42 +1100 Subject: [PATCH 06/31] fix lint --- packages/react-aria-components/test/AriaTree.test-util.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-aria-components/test/AriaTree.test-util.tsx b/packages/react-aria-components/test/AriaTree.test-util.tsx index 57d444834e9..0520dc5105b 100644 --- a/packages/react-aria-components/test/AriaTree.test-util.tsx +++ b/packages/react-aria-components/test/AriaTree.test-util.tsx @@ -11,7 +11,6 @@ */ import {act, render} from '@testing-library/react'; -import {User} from '@react-aria/test-utils'; let describeInteractions = ((name, tests) => describe.each` interactionType From 6b8f9abd0673a007e7f1dc79fc4991de13caff14 Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 13 Nov 2024 17:38:09 +1100 Subject: [PATCH 07/31] Add tests and test util --- packages/@react-aria/test-utils/src/events.ts | 4 + packages/@react-aria/test-utils/src/tree.ts | 270 +++++++++++++++ packages/@react-aria/test-utils/src/user.ts | 12 +- packages/@react-spectrum/s2/src/TreeView.tsx | 9 +- .../@react-spectrum/s2/test/Tree.test.tsx | 117 +++++++ .../stories/Tree.stories.tsx | 31 +- .../test/AriaTree.test-util.tsx | 194 +++++++++-- .../react-aria-components/test/Tree.test.tsx | 313 +++++++++--------- 8 files changed, 748 insertions(+), 202 deletions(-) create mode 100644 packages/@react-aria/test-utils/src/tree.ts diff --git a/packages/@react-aria/test-utils/src/events.ts b/packages/@react-aria/test-utils/src/events.ts index ae1e16dfcf7..47183e630b8 100644 --- a/packages/@react-aria/test-utils/src/events.ts +++ b/packages/@react-aria/test-utils/src/events.ts @@ -41,6 +41,10 @@ export async function pressElement(user, element: HTMLElement, interactionType: // stick to simulting an actual user's keyboard operations as closely as possible // There are problems when using this approach though, actions like trying to trigger the select all checkbox and stuff behave oddly. act(() => element.focus()); + // TODO: would be better throw? + if (document.activeElement !== element) { + return; + } await user.keyboard('[Space]'); } else if (interactionType === 'touch') { await user.pointer({target: element, keys: '[TouchA]'}); diff --git a/packages/@react-aria/test-utils/src/tree.ts b/packages/@react-aria/test-utils/src/tree.ts new file mode 100644 index 00000000000..ba328efe7f8 --- /dev/null +++ b/packages/@react-aria/test-utils/src/tree.ts @@ -0,0 +1,270 @@ +/* + * Copyright 2024 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {act, fireEvent, within} from '@testing-library/react'; +import {BaseTesterOpts, UserOpts} from './user'; +import {pressElement, triggerLongPress} from './events'; +export interface TreeOptions extends UserOpts, BaseTesterOpts { + user?: any +} + +// TODO: Previously used logic like https://github.com/testing-library/react-testing-library/blame/c63b873072d62c858959c2a19e68f8e2cc0b11be/src/pure.js#L16 +// but https://github.com/testing-library/dom-testing-library/issues/987#issuecomment-891901804 indicates that it may falsely indicate that fake timers are enabled +// when they aren't +export class TreeTester { + private user; + private _interactionType: UserOpts['interactionType']; + private _advanceTimer: UserOpts['advanceTimer']; + private _tree: HTMLElement; + + constructor(opts: TreeOptions) { + let {root, user, interactionType, advanceTimer} = opts; + this.user = user; + this._interactionType = interactionType || 'mouse'; + this._advanceTimer = advanceTimer; + this._tree = within(root).getByRole('treegrid'); + } + + setInteractionType = (type: UserOpts['interactionType']) => { + this._interactionType = type; + }; + + toggleRowSelection = async (opts: { + index?: number, + text?: string, + needsLongPress?: boolean, + // if false, will use the row to select instead of the checkbox directly + checkboxSelection?: boolean, + interactionType?: UserOpts['interactionType'] + } = {}) => { + let { + index, + text, + needsLongPress, + checkboxSelection = true, + interactionType = this._interactionType + } = opts; + + // this would be better than the check to do nothing in events.ts + // also, it'd be good to be able to trigger selection on the row instead of having to go to the checkbox directly + if (interactionType === 'keyboard' && !checkboxSelection) { + await this.keyboardNavigateToRow({row: this.findRow({index, text})}); + await this.user.keyboard('[Space]'); + return; + } + let row = this.findRow({index, text}); + let rowCheckbox = within(row).queryByRole('checkbox'); + if (rowCheckbox) { + await pressElement(this.user, rowCheckbox, interactionType); + } else { + let cell = within(row).getAllByRole('gridcell')[0]; + if (needsLongPress && interactionType === 'touch') { + if (this._advanceTimer == null) { + throw new Error('No advanceTimers provided for long press.'); + } + + // Note that long press interactions with rows is strictly touch only for grid rows + await triggerLongPress({element: cell, advanceTimer: this._advanceTimer, pointerOpts: {pointerType: 'touch'}}); + // TODO: interestingly enough, we need to do a followup click otherwise future row selections may not fire properly? + // To reproduce, try removing this, forcing toggleRowSelection to hit "needsLongPress ? await triggerLongPress(cell) : await action(cell);" and + // run Table.test's "should support long press to enter selection mode on touch" test to see what happens + await fireEvent.click(cell); + } else { + await pressElement(this.user, cell, interactionType); + } + } + }; + + expandItem = async (opts: {index?: number, text?: string, interactionType?: UserOpts['interactionType']} = {}) => { + let { + index, + text, + interactionType = this._interactionType + } = opts; + if (!this.tree.contains(document.activeElement)) { + await act(async () => { + this.tree.focus(); + }); + } + + let row = this.findRow({index, text}); + + if (row.getAttribute('aria-expanded') === 'true') { + return; + } + + if (interactionType === 'mouse' || interactionType === 'touch') { + let rowExpander = within(row).getAllByRole('button')[0]; // what happens if the button is not first? how can we differentiate? + await pressElement(this.user, rowExpander, interactionType); + } else if (interactionType === 'keyboard') { + await this.keyboardNavigateToRow({row}); + await this.user.keyboard('[ArrowRight]'); + } + }; + + collapseItem = async (opts: {index?: number, text?: string, interactionType?: UserOpts['interactionType']} = {}) => { + let { + index, + text, + interactionType = this._interactionType + } = opts; + if (!this.tree.contains(document.activeElement)) { + await act(async () => { + this.tree.focus(); + }); + } + + let row = this.findRow({index, text}); + + if (row.getAttribute('aria-expanded') === 'false') { + return; + } + + if (interactionType === 'mouse' || interactionType === 'touch') { + let rowExpander = within(row).getAllByRole('button')[0]; // what happens if the button is not first? how can we differentiate? + await pressElement(this.user, rowExpander, interactionType); + } else if (interactionType === 'keyboard') { + await this.keyboardNavigateToRow({row}); + await this.user.keyboard('[ArrowLeft]'); + } + }; + + keyboardNavigateToRow = async (opts: {row: HTMLElement}) => { + let {row} = opts; + let rows = this.rows; + let targetIndex = rows.indexOf(row); + if (targetIndex === -1) { + throw new Error('Option provided is not in the menu'); + } + if (document.activeElement === this.tree) { + await this.user.keyboard('[ArrowDown]'); + } else if (document.activeElement!.getAttribute('role') !== 'row') { + do { + await this.user.keyboard('[ArrowLeft]'); + } while (document.activeElement!.getAttribute('role') !== 'row'); + } + let currIndex = rows.indexOf(document.activeElement as HTMLElement); + if (targetIndex === -1) { + throw new Error('ActiveElement is not in the menu'); + } + let direction = targetIndex > currIndex ? 'down' : 'up'; + + for (let i = 0; i < Math.abs(targetIndex - currIndex); i++) { + await this.user.keyboard(`[${direction === 'down' ? 'ArrowDown' : 'ArrowUp'}]`); + } + }; + + // TODO: should there be a util for triggering a row action? Perhaps there should be but it would rely on the user teling us the config of the + // table. Maybe we could rely on the user knowing to trigger a press/double click? We could have the user pass in "needsDoubleClick" + // It is also iffy if there is any row selected because then the table is in selectionMode and the below actions will simply toggle row selection + triggerRowAction = async (opts: {index?: number, text?: string, needsDoubleClick?: boolean, interactionType?: UserOpts['interactionType']} = {}) => { + let { + index, + text, + needsDoubleClick, + interactionType = this._interactionType + } = opts; + + let row = this.findRow({index, text}); + if (row) { + if (needsDoubleClick) { + await this.user.dblClick(row); + } else if (interactionType === 'keyboard') { + act(() => row.focus()); + await this.user.keyboard('[Enter]'); + } else { + await pressElement(this.user, row, interactionType); + } + } + }; + + // TODO: should there be utils for drag and drop and column resizing? For column resizing, I'm not entirely convinced that users will be doing that in their tests. + // For DnD, it might be tricky to do for keyboard DnD since we wouldn't know what valid drop zones there are... Similarly, for simulating mouse drag and drop the coordinates depend + // on the mocks the user sets up for their row height/etc. + // Additionally, should we also support keyboard navigation/typeahead? Those felt like they could be very easily replicated by the user via user.keyboard already and don't really + // add much value if we provide that to them + + toggleSelectAll = async (opts: {interactionType?: UserOpts['interactionType']} = {}) => { + let { + interactionType = this._interactionType + } = opts; + let checkbox = within(this.tree).getByLabelText('Select All'); + if (interactionType === 'keyboard') { + // TODO: using the .focus -> trigger keyboard Enter approach doesn't work for some reason, for now just trigger select all with click. + await this.user.click(checkbox); + } else { + await pressElement(this.user, checkbox, interactionType); + } + }; + + findRow = (opts: {index?: number, text?: string} = {}) => { + let { + index, + text + } = opts; + + let row; + let rows = this.rows; + let bodyRowGroup = this.rowGroups[1]; + if (index != null) { + row = rows[index]; + } else if (text != null) { + row = within(bodyRowGroup).getByText(text); + while (row && row.getAttribute('role') !== 'row') { + row = row.parentElement; + } + } + + return row; + }; + + findCell = (opts: {text: string}) => { + let { + text + } = opts; + + let cell = within(this.tree).getByText(text); + if (cell) { + while (cell && !/gridcell|rowheader|columnheader/.test(cell.getAttribute('role') || '')) { + if (cell.parentElement) { + cell = cell.parentElement; + } else { + break; + } + } + } + + return cell; + }; + + get tree() { + return this._tree; + } + + get rowGroups() { + let tree = this.tree; + return tree ? within(tree).queryAllByRole('rowgroup') : []; + } + + get columns() { + let headerRowGroup = this.rowGroups[0]; + return headerRowGroup ? within(headerRowGroup).queryAllByRole('columnheader') : []; + } + + get rows() { + return within(this.tree).queryAllByRole('row') ?? []; + } + + get selectedRows() { + return this.rows.filter(row => row.getAttribute('aria-selected') === 'true'); + } +} diff --git a/packages/@react-aria/test-utils/src/user.ts b/packages/@react-aria/test-utils/src/user.ts index abae908181a..b1b041c9046 100644 --- a/packages/@react-aria/test-utils/src/user.ts +++ b/packages/@react-aria/test-utils/src/user.ts @@ -16,6 +16,7 @@ import {MenuOptions, MenuTester} from './menu'; import {pointerMap} from './'; import {SelectOptions, SelectTester} from './select'; import {TableOptions, TableTester} from './table'; +import {TreeOptions, TreeTester} from './tree'; import userEvent from '@testing-library/user-event'; // https://github.com/testing-library/dom-testing-library/issues/939#issuecomment-830771708 is an interesting way of allowing users to configure the timers @@ -33,13 +34,21 @@ export interface BaseTesterOpts { root: HTMLElement } -let keyToUtil = {'Select': SelectTester, 'Table': TableTester, 'Menu': MenuTester, 'ComboBox': ComboBoxTester, 'GridList': GridListTester} as const; +let keyToUtil = { + 'Select': SelectTester, + 'Table': TableTester, + 'Tree': TreeTester, + 'Menu': MenuTester, + 'ComboBox': ComboBoxTester, + 'GridList': GridListTester +} as const; export type PatternNames = keyof typeof keyToUtil; // Conditional type: https://www.typescriptlang.org/docs/handbook/2/conditional-types.html type ObjectType = T extends 'Select' ? SelectTester : T extends 'Table' ? TableTester : + T extends 'Tree' ? TreeTester : T extends 'Menu' ? MenuTester : T extends 'ComboBox' ? ComboBoxTester : T extends 'GridList' ? GridListTester : @@ -48,6 +57,7 @@ type ObjectType = type ObjectOptionsTypes = T extends 'Select' ? SelectOptions : T extends 'Table' ? TableOptions : + T extends 'Tree' ? TreeOptions : T extends 'Menu' ? MenuOptions : T extends 'ComboBox' ? ComboBoxOptions : T extends 'GridList' ? GridListOptions : diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 58052b048ae..1cea30eeb81 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -28,7 +28,7 @@ import Chevron from '../ui-icons/Chevron'; import {DOMRef, Key} from '@react-types/shared'; import {focusRing, style} from '../style' with {type: 'macro'}; import {isAndroid} from '@react-aria/utils'; -import React, {createContext, isValidElement, JSXElementConstructor, ReactElement, useContext, useRef} from 'react'; +import React, {createContext, forwardRef, isValidElement, JSXElementConstructor, ReactElement, useContext, useRef} from 'react'; import {StylesPropWithHeight, UnsafeStyles} from './style-utils'; import {useButton} from '@react-aria/button'; import {useDOMRef} from '@react-spectrum/utils'; @@ -39,7 +39,8 @@ interface S2TreeProps { onAction?: (key: Key) => void } -export interface TreeViewProps extends Omit, 'style' | 'disabledBehavior' | 'className' | 'onRowAction' | 'selectionBehavior' | 'onScroll' | 'onCellAction' | 'dragAndDropHooks'>, UnsafeStyles, S2TreeProps { +// should we remove disabledBehavior? +export interface TreeViewProps extends Omit, 'style' | 'className' | 'onRowAction' | 'selectionBehavior' | 'onScroll' | 'onCellAction' | 'dragAndDropHooks'>, UnsafeStyles, S2TreeProps { /** Spectrum-defined styles, returned by the `style()` macro. */ styles?: StylesPropWithHeight } @@ -95,7 +96,7 @@ const tree = style>({ } }); -function TreeView(props: TreeViewProps, ref: DOMRef) { +function TreeView(props: TreeViewProps, ref: DOMRef) { let {children, isDetached} = props; let renderer; @@ -345,5 +346,5 @@ function ExpandableRowChevron(props: ExpandableRowChevronProps) { /** * A tree view provides users with a way to navigate nested hierarchical information. */ -const _TreeView = React.forwardRef(TreeView) as (props: TreeViewProps & {ref?: DOMRef}) => ReactElement; +const _TreeView = forwardRef(TreeView); export {_TreeView as TreeView}; diff --git a/packages/@react-spectrum/s2/test/Tree.test.tsx b/packages/@react-spectrum/s2/test/Tree.test.tsx index b51f865481b..ac6e468dd3b 100644 --- a/packages/@react-spectrum/s2/test/Tree.test.tsx +++ b/packages/@react-spectrum/s2/test/Tree.test.tsx @@ -13,6 +13,7 @@ import { AriaTreeTests({ prefix: 'spectrum2-static', renderers: { + // todo - we don't support isDisabled on TreeViewItems? standard: () => render( @@ -39,6 +40,122 @@ AriaTreeTests({ + + School + + + Homework-1 + + + Homework-1A + + + + + Homework-2 + + + + Homework-3 + + + + + ), + singleSelection: () => render( + + + Photos + + + + Projects + + + Projects-1 + + + Projects-1A + + + + + Projects-2 + + + + Projects-3 + + + + + School + + + Homework-1 + + + Homework-1A + + + + + Homework-2 + + + + Homework-3 + + + + + ), + allInteractionsDisabled: () => render( + + + Photos + + + + Projects + + + Projects-1 + + + Projects-1A + + + + + Projects-2 + + + + Projects-3 + + + + + School + + + Homework-1 + + + Homework-1A + + + + + Homework-2 + + + + Homework-3 + + + ) } diff --git a/packages/react-aria-components/stories/Tree.stories.tsx b/packages/react-aria-components/stories/Tree.stories.tsx index d29aff75a78..d1c881765eb 100644 --- a/packages/react-aria-components/stories/Tree.stories.tsx +++ b/packages/react-aria-components/stories/Tree.stories.tsx @@ -87,7 +87,7 @@ const StaticTreeItem = (props: StaticTreeItemProps) => { ); }; -export const TreeExampleStatic = (args) => ( +const TreeExampleStaticRender = (args) => ( Photos @@ -134,7 +134,8 @@ export const TreeExampleStatic = (args) => ( ); -TreeExampleStatic.story = { +export const TreeExampleStatic = { + render: TreeExampleStaticRender, args: { selectionMode: 'none', selectionBehavior: 'toggle', @@ -271,7 +272,7 @@ const DynamicTreeItem = (props: DynamicTreeItemProps) => { let defaultExpandedKeys = new Set(['projects', 'project-2', 'project-5', 'reports', 'reports-1', 'reports-1A', 'reports-1AB']); -export const TreeExampleDynamic = (args: TreeProps) => ( +const TreeExampleDynamicRender = (args: TreeProps) => ( {(item) => ( @@ -281,22 +282,22 @@ export const TreeExampleDynamic = (args: TreeProps) => ( ); -TreeExampleDynamic.story = { - ...TreeExampleStatic.story, +export const TreeExampleDynamic = { + ...TreeExampleStatic, + render: TreeExampleDynamicRender, parameters: null }; export const WithActions = { - render: TreeExampleDynamic, ...TreeExampleDynamic, args: { onAction: action('onAction'), - ...TreeExampleDynamic.story.args + ...TreeExampleDynamic.args }, name: 'UNSTABLE_Tree with actions' }; -export const WithLinks = (args: TreeProps) => ( +const WithLinksRender = (args: TreeProps) => ( {(item) => ( @@ -306,8 +307,9 @@ export const WithLinks = (args: TreeProps) => ( ); -WithLinks.story = { - ...TreeExampleDynamic.story, +export const WithLinks = { + ...TreeExampleDynamic, + render: WithLinksRender, name: 'UNSTABLE_Tree with links', parameters: { description: { @@ -486,7 +488,7 @@ export const ButtonLoadingIndicatorStory = { } } }; -export function VirtualizedTree(args) { +function VirtualizedTreeRender(args) { let layout = useMemo(() => { return new ListLayout({ rowHeight: 30 @@ -495,9 +497,12 @@ export function VirtualizedTree(args) { return ( - + ); } -VirtualizedTree.story = TreeExampleDynamic.story; +export const VirtualizedTree = { + ...TreeExampleDynamic, + render: VirtualizedTreeRender +}; diff --git a/packages/react-aria-components/test/AriaTree.test-util.tsx b/packages/react-aria-components/test/AriaTree.test-util.tsx index 0520dc5105b..282fc134207 100644 --- a/packages/react-aria-components/test/AriaTree.test-util.tsx +++ b/packages/react-aria-components/test/AriaTree.test-util.tsx @@ -10,7 +10,12 @@ * governing permissions and limitations under the License. */ -import {act, render} from '@testing-library/react'; +import {act, render, within} from '@testing-library/react'; +import { + pointerMap +} from '@react-spectrum/test-utils-internal'; +import {User} from '@react-aria/test-utils'; +import userEvent from '@testing-library/user-event'; let describeInteractions = ((name, tests) => describe.each` interactionType @@ -41,63 +46,180 @@ interface AriaBaseTestProps { } interface AriaTreeTestProps extends AriaBaseTestProps { renderers: { - // must have label "test tree" - standard: (props?: {name: string}) => ReturnType + // must have an aria-label + standard: (props?: {name: string}) => ReturnType, + // must have an aria-label + singleSelection?: (props?: {name: string}) => ReturnType, + // must have an aria-label + allInteractionsDisabled?: (props?: {name: string}) => ReturnType } } export const AriaTreeTests = ({renderers, setup, prefix}: AriaTreeTestProps) => { describe(prefix ? prefix + 'AriaTree' : 'AriaTree', function () { + let user; + let testUtilUser = new User(); setup?.(); beforeAll(function () { jest.useFakeTimers(); }); + beforeEach(function () { + user = userEvent.setup({delay: null, pointerMap}); + }); + afterEach(() => { act(() => jest.runAllTimers()); }); it('should have the base set of aria and data attributes', () => { - let {getByRole, getAllByRole} = (renderers.standard!)(); - let tree = getByRole('treegrid'); - expect(tree).toHaveAttribute('aria-label', 'test tree'); + let root = (renderers.standard!)(); + let treeTester = testUtilUser.createTester('Tree', {user, root: root.container}); + let tree = treeTester.tree; + expect(tree).toHaveAttribute('aria-label'); - for (let row of getAllByRole('row')) { + for (let row of treeTester.rows) { expect(row).toHaveAttribute('aria-level'); expect(row).toHaveAttribute('aria-posinset'); expect(row).toHaveAttribute('aria-setsize'); } + expect(treeTester.rows[0]).not.toHaveAttribute('aria-expanded'); + expect(treeTester.rows[1]).toHaveAttribute('aria-expanded', 'false'); }); - // if (renderers.singleSelection) { - // describe('single selection', function () { - // it('selects an option via mouse', async function () { - // let tree = (renderers.singleSelection!)(); - // let menuTester = testUtilUser.createTester('Menu', {user, root: tree.container}); - // let triggerButton = menuTester.trigger!; - - // await menuTester.open(); - // act(() => {jest.runAllTimers();}); - - // let menu = menuTester.menu; - // expect(menu).toBeTruthy(); - // expect(menu).toHaveAttribute('aria-labelledby', triggerButton.id); - - // let options = menuTester.options; - - // await menuTester.selectOption({option: options[1], menuSelectionMode: 'single'}); - - // act(() => {jest.runAllTimers();}); - // expect(menu).not.toBeInTheDocument(); - - // await menuTester.open(); - // act(() => {jest.runAllTimers();}); + describeInteractions('interaction', function ({interactionType}) { + it('should have the expected attributes on the rows', async () => { + let tree = (renderers.standard!)(); + let treeTester = testUtilUser.createTester('Tree', {user, root: tree.container, interactionType}); + await treeTester.expandItem({index: 1}); + await treeTester.expandItem({index: 2}); + + let rows = treeTester.rows; + let rowNoChild = rows[0]; + expect(rowNoChild).toHaveAttribute('aria-label'); + expect(rowNoChild).not.toHaveAttribute('aria-expanded'); + expect(rowNoChild).toHaveAttribute('aria-level', '1'); + expect(rowNoChild).toHaveAttribute('aria-posinset', '1'); + expect(rowNoChild).toHaveAttribute('aria-setsize', '3'); + + let rowWithChildren = rows[1]; + // Row has action since it is expandable but not selectable. + expect(rowWithChildren).toHaveAttribute('aria-expanded', 'true'); + expect(rowWithChildren).toHaveAttribute('aria-level', '1'); + expect(rowWithChildren).toHaveAttribute('aria-posinset', '2'); + expect(rowWithChildren).toHaveAttribute('aria-setsize', '3'); + + let level2ChildRow = rows[2]; + expect(level2ChildRow).toHaveAttribute('aria-expanded', 'true'); + expect(level2ChildRow).toHaveAttribute('data-expanded', 'true'); + expect(level2ChildRow).toHaveAttribute('aria-level', '2'); + expect(level2ChildRow).toHaveAttribute('aria-posinset', '1'); + expect(level2ChildRow).toHaveAttribute('aria-setsize', '3'); + + let level3ChildRow = rows[3]; + expect(level3ChildRow).not.toHaveAttribute('aria-expanded'); + expect(level3ChildRow).toHaveAttribute('aria-level', '3'); + expect(level3ChildRow).toHaveAttribute('aria-posinset', '1'); + expect(level3ChildRow).toHaveAttribute('aria-setsize', '1'); + + let level2ChildRow2 = rows[4]; + expect(level2ChildRow2).not.toHaveAttribute('aria-expanded'); + expect(level2ChildRow2).toHaveAttribute('aria-level', '2'); + expect(level2ChildRow2).toHaveAttribute('aria-posinset', '2'); + expect(level2ChildRow2).toHaveAttribute('aria-setsize', '3'); + + let level2ChildRow3 = rows[5]; + expect(level2ChildRow3).not.toHaveAttribute('aria-expanded'); + expect(level2ChildRow3).toHaveAttribute('aria-level', '2'); + expect(level2ChildRow3).toHaveAttribute('aria-posinset', '3'); + expect(level2ChildRow3).toHaveAttribute('aria-setsize', '3'); + + // Collapse the first row and make sure it's collpased and that the inner rows are gone + await treeTester.collapseItem({index: 1}); + expect(rowWithChildren).toHaveAttribute('aria-expanded', 'false'); + expect(level2ChildRow).not.toBeInTheDocument(); + }); + }); - // options = menuTester.options; - // expect(options[0]).toHaveAttribute('aria-checked', 'false'); - // expect(options[1]).toHaveAttribute('aria-checked', 'true'); - // }); - // }); - // } + if (renderers.singleSelection) { + describe('single selection', function () { + describeInteractions('interaction', function ({interactionType}) { + // todo add test for using Space on the row to select it + it('can select items', async () => { + let tree = (renderers.singleSelection!)(); + let treeTester = testUtilUser.createTester('Tree', {user, root: tree.container, interactionType}); + + let rows = treeTester.rows; + expect(rows[0]).toHaveAttribute('aria-selected', 'false'); + expect(rows[1]).toHaveAttribute('aria-selected', 'false'); + // disabled rows should not be selectable + expect(rows[2]).not.toHaveAttribute('aria-selected'); + expect(within(rows[2]).getByRole('checkbox')).toHaveAttribute('disabled'); + + await treeTester.toggleRowSelection({index: 0}); + expect(rows[0]).toHaveAttribute('aria-selected', 'true'); + expect(rows[1]).toHaveAttribute('aria-selected', 'false'); + + await treeTester.toggleRowSelection({index: 1}); + expect(rows[0]).toHaveAttribute('aria-selected', 'false'); + expect(rows[1]).toHaveAttribute('aria-selected', 'true'); + + await treeTester.toggleRowSelection({index: 2}); + expect(rows[0]).toHaveAttribute('aria-selected', 'false'); + expect(rows[1]).toHaveAttribute('aria-selected', 'true'); + expect(rows[2]).not.toHaveAttribute('aria-selected'); + + await treeTester.expandItem({index: 1}); + rows = treeTester.rows; + // row 2 is now the subrow of row 1 because we expanded it + expect(rows[2]).toHaveAttribute('aria-selected', 'false'); + + await treeTester.toggleRowSelection({index: 2}); + expect(rows[0]).toHaveAttribute('aria-selected', 'false'); + expect(rows[1]).toHaveAttribute('aria-selected', 'false'); + expect(rows[2]).toHaveAttribute('aria-selected', 'true'); + + // collapse and re-expand to make sure the selection persists + await treeTester.collapseItem({index: 1}); + await treeTester.expandItem({index: 1}); + rows = treeTester.rows; + expect(rows[2]).toHaveAttribute('aria-selected', 'true'); + + await treeTester.toggleRowSelection({index: 2}); + expect(rows[0]).toHaveAttribute('aria-selected', 'false'); + expect(rows[1]).toHaveAttribute('aria-selected', 'false'); + expect(rows[2]).toHaveAttribute('aria-selected', 'false'); + + await treeTester.collapseItem({index: 1}); + // items inside a disabled item can be selected + await treeTester.expandItem({index: 2}); + rows = treeTester.rows; + + await treeTester.toggleRowSelection({index: 3}); + expect(rows[3]).toHaveAttribute('aria-selected', 'true'); + }); + }); + }); + } + + if (renderers.allInteractionsDisabled) { + describe('all interactions disabled', function () { + describeInteractions('interaction', function ({interactionType}) { + it('should not be able to interact with the tree', async () => { + let tree = (renderers.allInteractionsDisabled!)(); + let treeTester = testUtilUser.createTester('Tree', {user, root: tree.container, interactionType}); + + let rows = treeTester.rows; + expect(rows[2]).toHaveAttribute('aria-expanded', 'false'); + + await treeTester.expandItem({index: 2}); + expect(rows[2]).toHaveAttribute('aria-expanded', 'false'); + + await treeTester.toggleRowSelection({index: 2}); + expect(rows[2]).not.toHaveAttribute('aria-selected'); + }); + }); + }); + } }); }; diff --git a/packages/react-aria-components/test/Tree.test.tsx b/packages/react-aria-components/test/Tree.test.tsx index d86feaeb24d..6066ffb34ee 100644 --- a/packages/react-aria-components/test/Tree.test.tsx +++ b/packages/react-aria-components/test/Tree.test.tsx @@ -215,66 +215,43 @@ describe('Tree', () => { expect(rowNoChild).toHaveAttribute('aria-label', 'Photos'); expect(rowNoChild).not.toHaveAttribute('aria-expanded'); expect(rowNoChild).not.toHaveAttribute('data-expanded'); - expect(rowNoChild).toHaveAttribute('aria-level', '1'); 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).toHaveAttribute('data-rac'); let rowWithChildren = rows[1]; // Row has action since it is expandable but not selectable. expect(rowWithChildren).toHaveAttribute('aria-label', 'Projects'); - expect(rowWithChildren).toHaveAttribute('aria-expanded', 'true'); expect(rowWithChildren).toHaveAttribute('data-expanded', 'true'); - expect(rowWithChildren).toHaveAttribute('aria-level', '1'); 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-rac'); let level2ChildRow = rows[2]; expect(level2ChildRow).toHaveAttribute('aria-label', 'Projects-1'); - expect(level2ChildRow).toHaveAttribute('aria-expanded', 'true'); expect(level2ChildRow).toHaveAttribute('data-expanded', 'true'); - expect(level2ChildRow).toHaveAttribute('aria-level', '2'); 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-rac'); let level3ChildRow = rows[3]; expect(level3ChildRow).toHaveAttribute('aria-label', 'Projects-1A'); - expect(level3ChildRow).not.toHaveAttribute('aria-expanded'); expect(level3ChildRow).not.toHaveAttribute('data-expanded'); - expect(level3ChildRow).toHaveAttribute('aria-level', '3'); 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).toHaveAttribute('data-rac'); let level2ChildRow2 = rows[4]; expect(level2ChildRow2).toHaveAttribute('aria-label', 'Projects-2'); - expect(level2ChildRow2).not.toHaveAttribute('aria-expanded'); expect(level2ChildRow2).not.toHaveAttribute('data-expanded'); - expect(level2ChildRow2).toHaveAttribute('aria-level', '2'); 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).toHaveAttribute('data-rac'); let level2ChildRow3 = rows[5]; expect(level2ChildRow3).toHaveAttribute('aria-label', 'Projects-3'); - expect(level2ChildRow3).not.toHaveAttribute('aria-expanded'); expect(level2ChildRow3).not.toHaveAttribute('data-expanded'); - expect(level2ChildRow3).toHaveAttribute('aria-level', '2'); 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).toHaveAttribute('data-rac'); }); @@ -283,9 +260,7 @@ describe('Tree', () => { let {getAllByRole} = render(); let rows = getAllByRole('row'); - expect(rows[1]).toHaveAttribute('aria-label', 'Projects'); expect(rows[1]).toHaveAttribute('data-has-child-rows', 'true'); - expect(rows[1]).toHaveAttribute('aria-selected', 'false'); }); it('should support dynamic trees', () => { @@ -866,22 +841,14 @@ describe('Tree', () => { await user.tab(); expect(document.activeElement).toBe(rows[0]); - 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(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]).toHaveAttribute('aria-expanded', 'false'); expect(rows[0]).not.toHaveAttribute('data-expanded'); - 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(onExpandedChange).toHaveBeenCalledTimes(1); // Note that the children of the parent row will still be in the "expanded" array @@ -891,11 +858,7 @@ describe('Tree', () => { await trigger(rows[0], 'Enter'); expect(document.activeElement).toBe(rows[0]); - 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(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'])); @@ -905,27 +868,15 @@ describe('Tree', () => { await user.keyboard('{ArrowDown}'); await user.keyboard('{ArrowDown}'); expect(document.activeElement).toBe(rows[2]); - expect(rows[2]).toHaveAttribute('aria-expanded', 'true'); expect(rows[2]).toHaveAttribute('data-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'); // 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]).toHaveAttribute('aria-expanded', 'false'); expect(rows[2]).not.toHaveAttribute('data-expanded'); - 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[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(onExpandedChange).toHaveBeenCalledTimes(3); expect(new Set(onExpandedChange.mock.calls[2][0])).toEqual(new Set(['projects', 'project-5', 'reports', 'reports-1', 'reports-1A', 'reports-1AB'])); @@ -951,75 +902,6 @@ describe('Tree', () => { expect(rows).toHaveLength(17); }); - it('should not expand/collapse if disabledBehavior is "all" and the row is disabled', async () => { - let {getAllByRole, rerender} = render(); - let rows = getAllByRole('row'); - expect(rows).toHaveLength(20); - - await user.tab(); - // Since first row is disabled, we can't keyboard focus it - expect(document.activeElement).toBe(rows[1]); - expect(rows[0]).toHaveAttribute('aria-expanded', 'true'); - expect(rows[0]).toHaveAttribute('data-expanded', 'true'); - expect(rows[0]).toHaveAttribute('aria-disabled', 'true'); - expect(rows[0]).toHaveAttribute('data-disabled', 'true'); - expect(onExpandedChange).toHaveBeenCalledTimes(0); - - // Try clicking on first row - await trigger(rows[0], 'Space'); - expect(document.activeElement).toBe(rows[1]); - expect(rows[0]).toHaveAttribute('aria-expanded', 'true'); - expect(rows[0]).toHaveAttribute('data-expanded', 'true'); - expect(onExpandedChange).toHaveBeenCalledTimes(0); - - rerender(); - await user.tab(); - rows = getAllByRole('row'); - expect(rows[0]).toHaveAttribute('aria-expanded', 'false'); - expect(rows[0]).not.toHaveAttribute('data-expanded'); - expect(rows[0]).toHaveAttribute('aria-disabled', 'true'); - expect(rows[0]).toHaveAttribute('data-disabled', 'true'); - expect(onExpandedChange).toHaveBeenCalledTimes(0); - - await trigger(rows[0], 'Space'); - expect(rows[0]).toHaveAttribute('aria-expanded', 'false'); - expect(rows[0]).not.toHaveAttribute('data-expanded'); - expect(onExpandedChange).toHaveBeenCalledTimes(0); - }); - - it('should expand/collapse if disabledBehavior is "selection" and the row is disabled', async () => { - let {getAllByRole} = render(); - let rows = getAllByRole('row'); - - await user.tab(); - expect(document.activeElement).toBe(rows[0]); - expect(rows[0]).toHaveAttribute('aria-expanded', 'true'); - expect(rows[0]).toHaveAttribute('data-expanded', 'true'); - expect(onExpandedChange).toHaveBeenCalledTimes(0); - expect(onSelectionChange).toHaveBeenCalledTimes(0); - - // Since selection is enabled, we need to click the chevron even for disabled rows since it is still regarded as the primary action - let chevron = within(rows[0]).getAllByRole('button')[0]; - await trigger(chevron, 'ArrowLeft'); - // TODO: reenable this when we make it so the chevron button isn't focusable via click - // expect(document.activeElement).toBe(rows[0]); - expect(rows[0]).toHaveAttribute('aria-expanded', 'false'); - expect(rows[0]).not.toHaveAttribute('data-expanded'); - expect(onExpandedChange).toHaveBeenCalledTimes(1); - expect(new Set(onExpandedChange.mock.calls[0][0])).toEqual(new Set(['project-2', 'project-5', 'reports', 'reports-1', 'reports-1A', 'reports-1AB'])); - expect(onSelectionChange).toHaveBeenCalledTimes(0); - - await trigger(chevron); - expect(rows[0]).toHaveAttribute('aria-expanded', 'true'); - expect(rows[0]).toHaveAttribute('data-expanded', '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'])); - expect(onSelectionChange).toHaveBeenCalledTimes(0); - - let disabledCheckbox = within(rows[0]).getByRole('checkbox'); - expect(disabledCheckbox).toHaveAttribute('disabled'); - }); - it('should not expand when clicking/using Enter on the row if the row is selectable', async () => { let {getAllByRole} = render(); let rows = getAllByRole('row'); @@ -1114,35 +996,6 @@ describe('Tree', () => { }); }); - it('should support controlled expansion', async () => { - function ControlledTree() { - let [expandedKeys, setExpandedKeys] = React.useState(new Set([])); - - return ( - - ); - } - - let {getAllByRole} = render(); - let rows = getAllByRole('row'); - expect(rows).toHaveLength(2); - - await user.tab(); - expect(document.activeElement).toBe(rows[0]); - expect(rows[0]).toHaveAttribute('aria-expanded', 'false'); - expect(rows[0]).not.toHaveAttribute('data-expanded'); - - await user.click(rows[0]); - rows = getAllByRole('row'); - expect(rows).toHaveLength(7); - - await user.click(rows[0]); - expect(rows[0]).toHaveAttribute('aria-expanded', 'false'); - expect(rows[0]).not.toHaveAttribute('data-expanded'); - rows = getAllByRole('row'); - expect(rows).toHaveLength(2); - }); - it('should apply the proper attributes to the chevron', async () => { let {getAllByRole} = render(); let rows = getAllByRole('row'); @@ -1327,7 +1180,171 @@ AriaTreeTests({ prefix: 'rac-static', renderers: { standard: () => render( - + + Photos + + + + Projects-1A + + + + Projects-2 + + + Projects-3 + + + + + + Homework-1A + + + + Homework-2 + + + Homework-3 + + + + ), + singleSelection: () => render( + + Photos + + + + Projects-1A + + + + Projects-2 + + + Projects-3 + + + + + + Homework-1A + + + + Homework-2 + + + Homework-3 + + + + ), + allInteractionsDisabled: () => render( + + Photos + + + + Projects-1A + + + + Projects-2 + + + Projects-3 + + + + + + Homework-1A + + + + Homework-2 + + + Homework-3 + + + + ) + } +}); + +let controlledRows = [ + {id: 'photos', name: 'Photos 1'}, + {id: 'projects', name: 'Projects', childItems: [ + {id: 'project-1', name: 'Project 1', childItems: [ + {id: 'project-1A', name: 'Project 1A'} + ]}, + {id: 'project-2', name: 'Project 2'}, + {id: 'project-3', name: 'Project 3'} + ]}, + {id: 'reports', name: 'Reports', childItems: [ + {id: 'reports-1', name: 'Reports 1', childItems: [ + {id: 'reports-1A', name: 'Reports 1A'} + ]}, + {id: 'reports-2', name: 'Reports 2'}, + {id: 'reports-3', name: 'Reports 3'} + ]} +]; + +let ControlledDynamicTreeItem = (props) => { + return ( + + + {({isExpanded, hasChildRows, selectionMode, selectionBehavior}) => ( + <> + {(selectionMode !== 'none' || props.href != null) && selectionBehavior === 'toggle' && ( + + )} + {hasChildRows && } + {props.title || props.children} + + + + )} + + + {(item: any) => ( + + {item.name} + + )} + + + ); +}; + +function ControlledDynamicTree(props) { + let [expanded, setExpanded] = React.useState(new Set([])); + + return ( + + {(item: any) => ( + + {item.name} + + )} + + ); +} + +AriaTreeTests({ + prefix: 'rac-controlled-dynamic', + renderers: { + standard: () => render( + + ), + singleSelection: () => render( + + ), + allInteractionsDisabled: () => render( + ) } }); From 8bdfb54cb162f4870b7219f3c9c73f544616bb82 Mon Sep 17 00:00:00 2001 From: GitHub Date: Thu, 21 Nov 2024 16:09:10 +1100 Subject: [PATCH 08/31] save point --- packages/@react-spectrum/s2/src/TreeView.tsx | 95 +++++++++++-------- .../s2/stories/TreeView.stories.tsx | 7 +- 2 files changed, 63 insertions(+), 39 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 1cea30eeb81..2e142e43fba 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -26,13 +26,14 @@ import { import {Checkbox, IconContext, Text, TextContext} from '@react-spectrum/s2'; import Chevron from '../ui-icons/Chevron'; import {DOMRef, Key} from '@react-types/shared'; -import {focusRing, style} from '../style' with {type: 'macro'}; +import {colorMix, focusRing, lightDark, style} from '../style' with {type: 'macro'}; import {isAndroid} from '@react-aria/utils'; import React, {createContext, forwardRef, isValidElement, JSXElementConstructor, ReactElement, useContext, useRef} from 'react'; import {StylesPropWithHeight, UnsafeStyles} from './style-utils'; import {useButton} from '@react-aria/button'; import {useDOMRef} from '@react-spectrum/utils'; import {useLocale} from '@react-aria/i18n'; +import { raw } from '../style/style-macro'; interface S2TreeProps { isDetached?: boolean, @@ -62,7 +63,7 @@ function useTreeRendererContext(): TreeRendererContextValue { } -let InternalTreeContext = createContext({}); +let InternalTreeContext = createContext<{isDetached?: boolean}>({}); // TODO: add animations for rows appearing and disappearing @@ -70,7 +71,7 @@ let InternalTreeContext = createContext({}); // 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 // keyboard focus ring. Perhaps find a different way of rendering the outlines since the top of the item doesn't // scroll into view due to how the ring is offset. Alternatively, have the tree render the top/bottom outline like it does in Listview -const tree = style>({ +const tree = style({ borderWidth: 2, boxSizing: 'border-box', borderXWidth: 0, @@ -91,8 +92,14 @@ const tree = style>({ height: { isEmpty: 'full' }, - display: { - isEmpty: 'flex' + display: 'flex', + flexDirection: 'column', + gap: { + isDetached: 2 + }, + '--indent': { + type: 'width', + value: 16 } }); @@ -109,7 +116,11 @@ function TreeView(props: TreeViewProps, ref: DOMRef) { return ( - tree({isEmpty})} selectionBehavior="toggle" ref={domRef}> + tree({isEmpty, isDetached})} + selectionBehavior="toggle" + ref={domRef}> {props.children} @@ -121,6 +132,28 @@ interface TreeRowRenderProps extends TreeItemRenderProps { isLink?: boolean } +const selectedBackground = lightDark(colorMix('gray-25', 'informative-900', 10), colorMix('gray-25', 'informative-700', 10)); +const selectedActiveBackground = lightDark(colorMix('gray-25', 'informative-900', 15), colorMix('gray-25', 'informative-700', 15)); + +const rowBackgroundColor = { + default: { + default: 'gray-25', + isQuiet: '--s2-container-bg' + }, + isFocusVisibleWithin: colorMix('gray-25', 'gray-900', 7), // table-row-hover-color + isHovered: colorMix('gray-25', 'gray-900', 7), // table-row-hover-color + isPressed: colorMix('gray-25', 'gray-900', 10), // table-row-hover-color + isSelected: { + default: selectedBackground, // table-selected-row-background-color, opacity /10 + isFocusVisibleWithin: selectedActiveBackground, // table-selected-row-background-color, opacity /15 + isHovered: selectedActiveBackground, // table-selected-row-background-color, opacity /15 + isPressed: selectedActiveBackground // table-selected-row-background-color, opacity /15 + }, + forcedColors: { + default: 'Background' + } +} as const; + const treeRow = style({ position: 'relative', display: 'flex', @@ -135,17 +168,23 @@ const treeRow = style({ default: 'default', isLink: 'pointer' }, - // TODO: not sure where to get the equivalent colors here, for instance isHovered is spectrum 600 with 10% opacity but I don't think that exists in the theme - backgroundColor: { - isHovered: '[var(--spectrum-table-row-background-color-hover)]', - isFocusVisibleWithin: '[var(--spectrum-table-row-background-color-hover)]', - isPressed: '[var(--spectrum-table-row-background-color-down)]', - isSelected: '[var(--spectrum-table-row-background-color-selected)]' + backgroundColor: '--rowBackgroundColor', + '--rowBackgroundColor': { + type: 'backgroundColor', + value: rowBackgroundColor }, - '--indent': { - type: 'width', - value: 16 - } + '--rowFocusIndicatorColor': { + type: 'outlineColor', + value: { + default: 'focus-ring', + forcedColors: 'Highlight' + } + }, + borderTopWidth: 0, + borderBottomWidth: 0, + borderStartWidth: 0, + borderEndWidth: 0, + borderStyle: 'none' }); const treeCellGrid = style({ @@ -184,26 +223,6 @@ const treeContent = style({ overflow: 'hidden' }); -const treeRowOutline = style({ - ...focusRing(), - content: '', - display: 'block', - position: 'absolute', - insetStart: '[8px]', - insetEnd: '[8px]', - top: { - default: '[8px]', - isFocusVisible: '[8px]', - isSelected: { - default: '[1px]', - isFocusVisible: '[8px]' - } - }, - bottom: '[8px]', - pointerEvents: 'none', - forcedColorAdjust: 'none' -}); - export const TreeViewItem = (props: TreeViewItemProps) => { let { children, @@ -215,6 +234,7 @@ export const TreeViewItem = (props: TreeViewItemProps) => { let content; let nestedRows; let {renderer} = useTreeRendererContext(); + let {isDetached} = useContext(InternalTreeContext); // 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 @@ -247,7 +267,7 @@ export const TreeViewItem = (props: TreeViewItemProps) => { className={renderProps => treeRow({ ...renderProps, isLink: !!href - })}> + }) + (renderProps.isFocusVisible && ' ' + raw('&:before { content: ""; display: inline-block; position: sticky; inset-inline-start: 0; width: 3px; height: 100%; margin-inline-end: -3px; margin-block-end: 1px; z-index: 3; background-color: var(--rowFocusIndicatorColor)'))}> {({isExpanded, hasChildRows, selectionMode, selectionBehavior, isDisabled, isSelected, isFocusVisible}) => (
@@ -273,7 +293,6 @@ export const TreeViewItem = (props: TreeViewItemProps) => { ]}> {content} -
)} diff --git a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx index 00c9a264ad6..e1aee8fbbfb 100644 --- a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx +++ b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx @@ -69,7 +69,12 @@ export default meta; const TreeExampleStatic = (args) => (
- + Photos From 10b40f4c27226d9e790b7dd53f6418d7940a6d12 Mon Sep 17 00:00:00 2001 From: GitHub Date: Thu, 21 Nov 2024 16:19:55 +1100 Subject: [PATCH 09/31] Add emptyState --- packages/@react-spectrum/s2/src/TreeView.tsx | 3 -- .../s2/stories/TreeView.stories.tsx | 31 ++++++++++++++++--- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 2e142e43fba..2a6bb8cce3f 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -65,9 +65,6 @@ function useTreeRendererContext(): TreeRendererContextValue { let InternalTreeContext = createContext<{isDetached?: boolean}>({}); - -// 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 // keyboard focus ring. Perhaps find a different way of rendering the outlines since the top of the item doesn't // scroll into view due to how the ring is offset. Alternatively, have the tree render the top/bottom outline like it does in Listview diff --git a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx index e1aee8fbbfb..fd7b377dfea 100644 --- a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx +++ b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx @@ -20,16 +20,17 @@ import Folder from '../s2wf-icons/S2_Icon_Folder_20_N.svg'; import type {Meta} from '@storybook/react'; import React from 'react'; import { + Content, + Heading, + IllustratedMessage, + Link, // ActionMenu, // MenuItem, - // Content, - // Heading, Text, - // IllustratedMessage, - // Link, TreeView, TreeViewItem } from '../src'; +import FolderOpen from '../spectrum-illustrations/linear/FolderOpen'; let onActionFunc = action('onAction'); let noOnAction = null; @@ -230,3 +231,25 @@ export const Dynamic = { disabledKeys: ['Foo 5'] } }; + +function renderEmptyState() { + return ( + + + + No results + + + No results found, press here for more info. + + + ); +} + +export const Empty = { + render: TreeExampleDynamic, + args: { + renderEmptyState, + items: [] + } +} From e9e4bf20d5c9fc075ab3c05b76736fe4d9ddb138 Mon Sep 17 00:00:00 2001 From: GitHub Date: Thu, 21 Nov 2024 16:26:47 +1100 Subject: [PATCH 10/31] fix lint --- packages/@react-spectrum/s2/src/TreeView.tsx | 9 ++++----- .../s2/stories/TreeView.stories.tsx | 16 ++++++++-------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 2a6bb8cce3f..e762e9447b8 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -17,7 +17,6 @@ import { TreeItemProps as RACTreeItemProps, TreeProps as RACTreeProps, TreeItemRenderProps, - TreeRenderProps, UNSTABLE_Tree, UNSTABLE_TreeItem, UNSTABLE_TreeItemContent, @@ -25,15 +24,15 @@ import { } from 'react-aria-components'; import {Checkbox, IconContext, Text, TextContext} from '@react-spectrum/s2'; import Chevron from '../ui-icons/Chevron'; +import {colorMix, lightDark, style} from '../style' with {type: 'macro'}; import {DOMRef, Key} from '@react-types/shared'; -import {colorMix, focusRing, lightDark, style} from '../style' with {type: 'macro'}; import {isAndroid} from '@react-aria/utils'; +import {raw} from '../style/style-macro'; import React, {createContext, forwardRef, isValidElement, JSXElementConstructor, ReactElement, useContext, useRef} from 'react'; import {StylesPropWithHeight, UnsafeStyles} from './style-utils'; import {useButton} from '@react-aria/button'; import {useDOMRef} from '@react-spectrum/utils'; import {useLocale} from '@react-aria/i18n'; -import { raw } from '../style/style-macro'; interface S2TreeProps { isDetached?: boolean, @@ -231,7 +230,7 @@ export const TreeViewItem = (props: TreeViewItemProps) => { let content; let nestedRows; let {renderer} = useTreeRendererContext(); - let {isDetached} = useContext(InternalTreeContext); + // let {isDetached} = useContext(InternalTreeContext); // 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 @@ -266,7 +265,7 @@ export const TreeViewItem = (props: TreeViewItemProps) => { isLink: !!href }) + (renderProps.isFocusVisible && ' ' + raw('&:before { content: ""; display: inline-block; position: sticky; inset-inline-start: 0; width: 3px; height: 100%; margin-inline-end: -3px; margin-block-end: 1px; z-index: 3; background-color: var(--rowFocusIndicatorColor)'))}> - {({isExpanded, hasChildRows, selectionMode, selectionBehavior, isDisabled, isSelected, isFocusVisible}) => ( + {({isExpanded, hasChildRows, selectionMode, selectionBehavior, isDisabled}) => (
{selectionMode !== 'none' && selectionBehavior === 'toggle' && ( // TODO: add transition? diff --git a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx index fd7b377dfea..5ee6e950cb7 100644 --- a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx +++ b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx @@ -12,13 +12,6 @@ import {action} from '@storybook/addon-actions'; import {categorizeArgTypes} from './utils'; -import FileTxt from '../s2wf-icons/S2_Icon_FileText_20_N.svg'; -// import Add from '../s2wf-icons/S2_Icon_Add_20_N.svg'; -// import Delete from '../s2wf-icons/S2_Icon_Delete_20_N.svg'; -// import Edit from '../s2wf-icons/S2_Icon_Edit_20_N.svg'; -import Folder from '../s2wf-icons/S2_Icon_Folder_20_N.svg'; -import type {Meta} from '@storybook/react'; -import React from 'react'; import { Content, Heading, @@ -30,7 +23,14 @@ import { TreeView, TreeViewItem } from '../src'; +// import Add from '../s2wf-icons/S2_Icon_Add_20_N.svg'; +// 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'; +import Folder from '../s2wf-icons/S2_Icon_Folder_20_N.svg'; import FolderOpen from '../spectrum-illustrations/linear/FolderOpen'; +import type {Meta} from '@storybook/react'; +import React from 'react'; let onActionFunc = action('onAction'); let noOnAction = null; @@ -252,4 +252,4 @@ export const Empty = { renderEmptyState, items: [] } -} +}; From 5fcfd47f8df7ba7d5abaafaceb9aaede0bdc304c Mon Sep 17 00:00:00 2001 From: GitHub Date: Thu, 21 Nov 2024 16:37:09 +1100 Subject: [PATCH 11/31] detached styles --- packages/@react-spectrum/s2/src/TreeView.tsx | 24 ++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index e762e9447b8..6e445bedbe6 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -150,7 +150,7 @@ const rowBackgroundColor = { } } as const; -const treeRow = style({ +const treeRow = style({ position: 'relative', display: 'flex', height: 40, @@ -159,6 +159,9 @@ const treeRow = style({ boxSizing: 'border-box', font: 'ui', color: 'body', + borderRadius: { // odd behaviour, if this is the last property, then bottom right isn't rounded + isDetached: 'default' + }, outlineStyle: 'none', cursor: { default: 'default', @@ -219,6 +222,17 @@ const treeContent = style({ overflow: 'hidden' }); +const cellFocus = { + outlineStyle: { + default: 'none', + isFocusVisible: 'solid' + }, + outlineOffset: -2, + outlineWidth: 2, + outlineColor: 'focus-ring', + borderRadius: '[6px]' +} as const; + export const TreeViewItem = (props: TreeViewItemProps) => { let { children, @@ -230,7 +244,7 @@ export const TreeViewItem = (props: TreeViewItemProps) => { let content; let nestedRows; let {renderer} = useTreeRendererContext(); - // let {isDetached} = useContext(InternalTreeContext); + let {isDetached} = useContext(InternalTreeContext); // 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 @@ -262,10 +276,11 @@ export const TreeViewItem = (props: TreeViewItemProps) => { {...props} className={renderProps => treeRow({ ...renderProps, + isDetached, isLink: !!href - }) + (renderProps.isFocusVisible && ' ' + raw('&:before { content: ""; display: inline-block; position: sticky; inset-inline-start: 0; width: 3px; height: 100%; margin-inline-end: -3px; margin-block-end: 1px; z-index: 3; background-color: var(--rowFocusIndicatorColor)'))}> + }) + (renderProps.isFocusVisible && !isDetached && ' ' + raw('&:before { content: ""; display: inline-block; position: sticky; inset-inline-start: 0; width: 3px; height: 100%; margin-inline-end: -3px; margin-block-end: 1px; z-index: 3; background-color: var(--rowFocusIndicatorColor)'))}> - {({isExpanded, hasChildRows, selectionMode, selectionBehavior, isDisabled}) => ( + {({isExpanded, hasChildRows, selectionMode, selectionBehavior, isDisabled, isFocusVisible}) => (
{selectionMode !== 'none' && selectionBehavior === 'toggle' && ( // TODO: add transition? @@ -289,6 +304,7 @@ export const TreeViewItem = (props: TreeViewItemProps) => { ]}> {content} + {isFocusVisible && isDetached &&
}
)} From 9699bc0a69f457c4739ba65c3d2f5d1bdca50f63 Mon Sep 17 00:00:00 2001 From: GitHub Date: Thu, 28 Nov 2024 10:32:38 +1100 Subject: [PATCH 12/31] detached styling with actions --- packages/@react-spectrum/s2/src/Menu.tsx | 2 + packages/@react-spectrum/s2/src/TreeView.tsx | 236 ++++++++++++------ .../s2/stories/TreeView.stories.tsx | 102 +++++--- .../s2/style/spectrum-theme.ts | 29 ++- .../@react-spectrum/s2/style/style-macro.ts | 2 +- packages/react-aria-components/src/Menu.tsx | 3 +- packages/react-aria-components/src/Tree.tsx | 12 +- 7 files changed, 262 insertions(+), 124 deletions(-) diff --git a/packages/@react-spectrum/s2/src/Menu.tsx b/packages/@react-spectrum/s2/src/Menu.tsx index dc9425565c6..885816ea251 100644 --- a/packages/@react-spectrum/s2/src/Menu.tsx +++ b/packages/@react-spectrum/s2/src/Menu.tsx @@ -22,6 +22,7 @@ import { SubmenuTrigger as AriaSubmenuTrigger, SubmenuTriggerProps as AriaSubmenuTriggerProps, ContextValue, + DEFAULT_SLOT, Provider, Separator, SeparatorProps @@ -478,6 +479,7 @@ export function MenuItem(props: MenuItemProps) { }], [TextContext, { slots: { + [DEFAULT_SLOT]: {styles: label({size})}, label: {styles: label({size})}, description: {styles: description({...renderProps, size})}, value: {styles: value} diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 6e445bedbe6..88c38a0026f 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -10,24 +10,23 @@ * governing permissions and limitations under the License. */ +import {ActionMenuContext, Checkbox, IconContext, Text, TextContext} from '@react-spectrum/s2'; import { ButtonContext, Collection, Provider, TreeItemProps as RACTreeItemProps, TreeProps as RACTreeProps, - TreeItemRenderProps, UNSTABLE_Tree, UNSTABLE_TreeItem, UNSTABLE_TreeItemContent, useContextProps } from 'react-aria-components'; -import {Checkbox, IconContext, Text, TextContext} from '@react-spectrum/s2'; import Chevron from '../ui-icons/Chevron'; import {colorMix, lightDark, style} from '../style' with {type: 'macro'}; import {DOMRef, Key} from '@react-types/shared'; import {isAndroid} from '@react-aria/utils'; -import {raw} from '../style/style-macro'; +import {raw} from '../style/style-macro' with {type: 'macro'}; import React, {createContext, forwardRef, isValidElement, JSXElementConstructor, ReactElement, useContext, useRef} from 'react'; import {StylesPropWithHeight, UnsafeStyles} from './style-utils'; import {useButton} from '@react-aria/button'; @@ -35,8 +34,11 @@ import {useDOMRef} from '@react-spectrum/utils'; import {useLocale} from '@react-aria/i18n'; interface S2TreeProps { + // Only detatched is supported right now with the current styles from Spectrum isDetached?: boolean, - onAction?: (key: Key) => void + onAction?: (key: Key) => void, + // not fully supported yet + isEmphasized?: boolean } // should we remove disabledBehavior? @@ -62,20 +64,13 @@ function useTreeRendererContext(): TreeRendererContextValue { } -let InternalTreeContext = createContext<{isDetached?: boolean}>({}); +let InternalTreeContext = createContext<{isDetached?: boolean, isEmphasized?: boolean}>({}); // 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 // keyboard focus ring. Perhaps find a different way of rendering the outlines since the top of the item doesn't // scroll into view due to how the ring is offset. Alternatively, have the tree render the top/bottom outline like it does in Listview const tree = style({ - borderWidth: 2, boxSizing: 'border-box', - borderXWidth: 0, - borderStyle: 'solid', - borderColor: { - default: 'transparent', - forcedColors: 'Background' - }, justifyContent: { isEmpty: 'center' }, @@ -100,7 +95,9 @@ const tree = style({ }); function TreeView(props: TreeViewProps, ref: DOMRef) { - let {children, isDetached} = props; + let {children, isDetached, isEmphasized} = props; + isDetached = true; + isEmphasized = false; let renderer; if (typeof children === 'function') { @@ -111,7 +108,7 @@ function TreeView(props: TreeViewProps, ref: DOMRef) { return ( - + tree({isEmpty, isDetached})} @@ -124,26 +121,29 @@ function TreeView(props: TreeViewProps, ref: DOMRef) { ); } -interface TreeRowRenderProps extends TreeItemRenderProps { - isLink?: boolean -} - const selectedBackground = lightDark(colorMix('gray-25', 'informative-900', 10), colorMix('gray-25', 'informative-700', 10)); const selectedActiveBackground = lightDark(colorMix('gray-25', 'informative-900', 15), colorMix('gray-25', 'informative-700', 15)); const rowBackgroundColor = { - default: { - default: 'gray-25', - isQuiet: '--s2-container-bg' - }, - isFocusVisibleWithin: colorMix('gray-25', 'gray-900', 7), // table-row-hover-color - isHovered: colorMix('gray-25', 'gray-900', 7), // table-row-hover-color - isPressed: colorMix('gray-25', 'gray-900', 10), // table-row-hover-color + default: '--s2-container-bg', + isFocusVisibleWithin: colorMix('gray-25', 'gray-900', 7), + isHovered: colorMix('gray-25', 'gray-900', 7), + isPressed: colorMix('gray-25', 'gray-900', 10), isSelected: { - default: selectedBackground, // table-selected-row-background-color, opacity /10 - isFocusVisibleWithin: selectedActiveBackground, // table-selected-row-background-color, opacity /15 - isHovered: selectedActiveBackground, // table-selected-row-background-color, opacity /15 - isPressed: selectedActiveBackground // table-selected-row-background-color, opacity /15 + default: colorMix('gray-25', 'gray-900', 7), + isEmphasized: selectedBackground, + isFocusVisibleWithin: { + default: colorMix('gray-25', 'gray-900', 10), + isEmphasized: selectedActiveBackground + }, + isHovered: { + default: colorMix('gray-25', 'gray-900', 10), + isEmphasized: selectedActiveBackground + }, + isPressed: { + default: colorMix('gray-25', 'gray-900', 10), + isEmphasized: selectedActiveBackground + } }, forcedColors: { default: 'Background' @@ -159,15 +159,11 @@ const treeRow = style({ boxSizing: 'border-box', font: 'ui', color: 'body', - borderRadius: { // odd behaviour, if this is the last property, then bottom right isn't rounded - isDetached: 'default' - }, outlineStyle: 'none', cursor: { default: 'default', isLink: 'pointer' }, - backgroundColor: '--rowBackgroundColor', '--rowBackgroundColor': { type: 'backgroundColor', value: rowBackgroundColor @@ -178,28 +174,88 @@ const treeRow = style({ default: 'focus-ring', forcedColors: 'Highlight' } - }, - borderTopWidth: 0, - borderBottomWidth: 0, - borderStartWidth: 0, - borderEndWidth: 0, - borderStyle: 'none' + } }); + const treeCellGrid = style({ display: 'grid', width: 'full', alignItems: 'center', - gridTemplateColumns: ['minmax(0, auto)', 'minmax(0, auto)', 'minmax(0, auto)', 40, 'minmax(0, auto)', '1fr'], + gridTemplateColumns: ['minmax(0, auto)', 'minmax(0, auto)', 'minmax(0, auto)', 40, 'minmax(0, auto)', '1fr', 'minmax(0, auto)'], gridTemplateRows: '1fr', gridTemplateAreas: [ - 'drag-handle checkbox level-padding expand-button icon content' + 'drag-handle checkbox level-padding expand-button icon content actions' ], + backgroundColor: '--rowBackgroundColor', color: { isDisabled: { default: 'gray-400', forcedColors: 'GrayText' } + }, + '--rowSelectedBorderColor': { + type: 'outlineColor', + value: { + default: 'gray-800', + isFocusVisible: 'focus-ring', + forcedColors: 'Highlight' + } + }, + '--rowForcedFocusBorderColor': { + type: 'outlineColor', + value: { + default: 'focus-ring', + forcedColors: 'Highlight' + } + }, + borderTopColor: { + default: 'transparent', + isSelected: { + isFirst: '--rowSelectedBorderColor' + }, + isDetached: { + default: 'transparent', + isSelected: '--rowSelectedBorderColor' + } + }, + borderInlineEndColor: { + default: 'transparent', + isSelected: '--rowSelectedBorderColor', + isDetached: { + default: 'transparent', + isSelected: '--rowSelectedBorderColor' + } + }, + borderBottomColor: { + default: 'transparent', + isSelected: '--rowSelectedBorderColor', + isNextSelected: '--rowSelectedBorderColor', + isNextFocused: '--rowForcedFocusBorderColor', + isDetached: { + default: 'transparent', + isSelected: '--rowSelectedBorderColor' + } + }, + borderInlineStartColor: { + default: 'transparent', + isSelected: '--rowSelectedBorderColor', + isDetached: { + default: 'transparent', + isSelected: '--rowSelectedBorderColor' + } + }, + borderTopWidth: { + default: 0, + isFirst: 1, + isDetached: 1 + }, + borderBottomWidth: 1, + borderStartWidth: 1, + borderEndWidth: 1, + borderStyle: 'solid', + borderRadius: { // odd behaviour, if this is the last property, then bottom right isn't rounded + isDetached: '[6px]' } }); @@ -222,6 +278,15 @@ const treeContent = style({ overflow: 'hidden' }); +const treeActions = style({ + gridArea: 'actions', + flexGrow: 0, + flexShrink: 0, + /* TODO: I made this one up, confirm desired behavior. These paddings are to make sure the action group has enough padding for the focus ring */ + marginStart: 2, + marginEnd: 4 +}); + const cellFocus = { outlineStyle: { default: 'none', @@ -233,6 +298,22 @@ const cellFocus = { borderRadius: '[6px]' } as const; +const treeRowFocusIndicator = raw(` + &:before { + content: ""; + display: inline-block; + position: sticky; + inset-inline-start: 0; + width: 3px; + height: 100%; + margin-inline-end: -3px; + margin-block-end: 1px; + z-index: 3; + background-color: var(--rowFocusIndicatorColor); + }` +); + + export const TreeViewItem = (props: TreeViewItemProps) => { let { children, @@ -244,7 +325,7 @@ export const TreeViewItem = (props: TreeViewItemProps) => { let content; let nestedRows; let {renderer} = useTreeRendererContext(); - let {isDetached} = useContext(InternalTreeContext); + let {isDetached, isEmphasized} = useContext(InternalTreeContext); // 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 @@ -274,39 +355,45 @@ export const TreeViewItem = (props: TreeViewItemProps) => { // 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, - isDetached, - isLink: !!href - }) + (renderProps.isFocusVisible && !isDetached && ' ' + raw('&:before { content: ""; display: inline-block; position: sticky; inset-inline-start: 0; width: 3px; height: 100%; margin-inline-end: -3px; margin-block-end: 1px; z-index: 3; background-color: var(--rowFocusIndicatorColor)'))}> + className={(renderProps) => treeRow({...renderProps, isLink: !!href, isEmphasized}) + (renderProps.isFocusVisible && !isDetached ? ' ' + treeRowFocusIndicator : '')}> - {({isExpanded, hasChildRows, selectionMode, selectionBehavior, isDisabled, 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) && } - - {content} - - {isFocusVisible && isDetached &&
} -
- )} + {({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} @@ -328,6 +415,7 @@ const expandButton = style({ alignContent: 'center', justifyContent: 'center', outlineStyle: 'none', + cursor: 'default', transform: { isExpanded: { default: 'rotate(90deg)', diff --git a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx index 5ee6e950cb7..1688dae489b 100644 --- a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx +++ b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx @@ -11,21 +11,20 @@ */ import {action} from '@storybook/addon-actions'; -import {categorizeArgTypes} from './utils'; import { + ActionMenu, Content, Heading, IllustratedMessage, Link, - // ActionMenu, - // MenuItem, + MenuItem, Text, TreeView, TreeViewItem } from '../src'; -// import Add from '../s2wf-icons/S2_Icon_Add_20_N.svg'; -// import Delete from '../s2wf-icons/S2_Icon_Delete_20_N.svg'; -// import Edit from '../s2wf-icons/S2_Icon_Edit_20_N.svg'; +import {categorizeArgTypes} from './utils'; +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'; import Folder from '../s2wf-icons/S2_Icon_Folder_20_N.svg'; import FolderOpen from '../spectrum-illustrations/linear/FolderOpen'; @@ -68,6 +67,7 @@ const meta: Meta = { export default meta; + const TreeExampleStatic = (args) => (
( Photos - {/* - + + Edit - + Delete - */} + Projects - {/* - + + Edit - + Delete - */} + Projects-1 - {/* - + + Edit - + Delete - */} + Projects-1A - {/* - + + Edit - + Delete - */} + Projects-2 - {/* - + + Edit - + Delete - */} + Projects-3 - {/* - + + Edit - + Delete - */} + @@ -208,16 +208,16 @@ const TreeExampleDynamic = (args) => ( {item.name} {item.icon} - {/* - + + Edit - + Delete - */} + )} @@ -253,3 +253,37 @@ export const Empty = { items: [] } }; + +const TreeExampleWithLinks = (args) => ( +
+ + {(item) => ( + + {item.name} + {item.icon} + + + + Edit + + + + Delete + + + + )} + +
+); + +export const WithLinks = { + ...Dynamic, + render: TreeExampleWithLinks, + name: 'Tree with links', + parameters: { + description: { + data: 'every tree item should link to adobe.com' + } + } +}; diff --git a/packages/@react-spectrum/s2/style/spectrum-theme.ts b/packages/@react-spectrum/s2/style/spectrum-theme.ts index 347120e82b3..c19dd4e4109 100644 --- a/packages/@react-spectrum/s2/style/spectrum-theme.ts +++ b/packages/@react-spectrum/s2/style/spectrum-theme.ts @@ -288,6 +288,19 @@ 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 @@ -550,18 +563,7 @@ 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': { @@ -635,6 +637,10 @@ 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', @@ -920,6 +926,7 @@ 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, diff --git a/packages/@react-spectrum/s2/style/style-macro.ts b/packages/@react-spectrum/s2/style/style-macro.ts index d7cde4447f8..b82d8d68cac 100644 --- a/packages/@react-spectrum/s2/style/style-macro.ts +++ b/packages/@react-spectrum/s2/style/style-macro.ts @@ -57,7 +57,7 @@ export function createSizingProperty(values: PropertyValueMa return [{[property]: value}, valueMap.get(value)!]; }); } - + if (typeof value === 'number') { let cssValue = value === 0 ? '0px' : fn(value); return {default: [{[property]: cssValue}, generateName(value + valueMap.size)]}; diff --git a/packages/react-aria-components/src/Menu.tsx b/packages/react-aria-components/src/Menu.tsx index d73339e506f..b38c8608538 100644 --- a/packages/react-aria-components/src/Menu.tsx +++ b/packages/react-aria-components/src/Menu.tsx @@ -15,7 +15,7 @@ import {AriaMenuProps, FocusScope, mergeProps, useFocusRing, useMenu, useMenuIte import {MenuTriggerProps as BaseMenuTriggerProps, Collection as ICollection, Node, TreeState, useMenuTriggerState, useTreeState} from 'react-stately'; import {Collection, CollectionBuilder, createBranchComponent, createLeafComponent} from '@react-aria/collections'; import {CollectionProps, CollectionRendererContext, ItemRenderProps, SectionContext, SectionProps, usePersistedKeys} from './Collection'; -import {ContextValue, Provider, RenderProps, ScrollableProps, SlotProps, StyleProps, useContextProps, useRenderProps, useSlot, useSlottedContext} from './utils'; +import {ContextValue, DEFAULT_SLOT, Provider, RenderProps, ScrollableProps, SlotProps, StyleProps, useContextProps, useRenderProps, useSlot, useSlottedContext} from './utils'; import {filterDOMProps, useObjectRef, useResizeObserver} from '@react-aria/utils'; import {FocusStrategy, forwardRefType, HoverEvents, Key, LinkDOMProps, MultipleSelection} from '@react-types/shared'; import {HeaderContext} from './Header'; @@ -378,6 +378,7 @@ export const MenuItem = /*#__PURE__*/ createLeafComponent('item', function MenuI values={[ [TextContext, { slots: { + [DEFAULT_SLOT]: labelProps, label: labelProps, description: descriptionProps } diff --git a/packages/react-aria-components/src/Tree.tsx b/packages/react-aria-components/src/Tree.tsx index 053dfaa0b95..b994fc015f2 100644 --- a/packages/react-aria-components/src/Tree.tsx +++ b/packages/react-aria-components/src/Tree.tsx @@ -276,7 +276,11 @@ export interface TreeItemContentRenderProps extends ItemRenderProps { // What level the tree item has within the tree. level: number, // Whether the tree item's children have keyboard focus. - isFocusVisibleWithin: boolean + isFocusVisibleWithin: boolean, + // The state of the tree. + state: TreeState, + // The unique id of the tree row. + id: Key } // The TreeItemContent is the one that accepts RenderProps because we would get much more complicated logic in TreeItem otherwise since we'd @@ -350,8 +354,10 @@ export const UNSTABLE_TreeItem = /*#__PURE__*/ createBranchComponent('item', Date: Mon, 2 Dec 2024 15:01:05 +1100 Subject: [PATCH 13/31] fix disabled, icon placement, and add examples --- .../s2/src/ActionButtonGroup.tsx | 2 +- packages/@react-spectrum/s2/src/TreeView.tsx | 27 ++++++++++++++----- .../s2/stories/TreeView.stories.tsx | 12 ++++----- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/packages/@react-spectrum/s2/src/ActionButtonGroup.tsx b/packages/@react-spectrum/s2/src/ActionButtonGroup.tsx index 3cdad768702..93e71d1bfab 100644 --- a/packages/@react-spectrum/s2/src/ActionButtonGroup.tsx +++ b/packages/@react-spectrum/s2/src/ActionButtonGroup.tsx @@ -71,7 +71,7 @@ export const actionGroupStyle = style({ }, getAllowedOverrides({height: true})); -export const ActionButtonGroupContext = createContext>(null); +export const ActionButtonGroupContext = createContext, HTMLDivElement>>(null); /** * An ActionButtonGroup is a grouping of related ActionButtons. diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 88c38a0026f..2f1f55658a2 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {ActionMenuContext, Checkbox, IconContext, Text, TextContext} from '@react-spectrum/s2'; +import {ActionButtonGroupContext, ActionMenuContext, Checkbox, IconContext, Text, TextContext} from '@react-spectrum/s2'; import { ButtonContext, Collection, @@ -23,7 +23,7 @@ import { useContextProps } from 'react-aria-components'; import Chevron from '../ui-icons/Chevron'; -import {colorMix, lightDark, style} from '../style' with {type: 'macro'}; +import {colorMix, fontRelative, lightDark, style} from '../style' with {type: 'macro'}; import {DOMRef, Key} from '@react-types/shared'; import {isAndroid} from '@react-aria/utils'; import {raw} from '../style/style-macro' with {type: 'macro'}; @@ -32,6 +32,7 @@ import {StylesPropWithHeight, UnsafeStyles} from './style-utils'; import {useButton} from '@react-aria/button'; import {useDOMRef} from '@react-spectrum/utils'; import {useLocale} from '@react-aria/i18n'; +import { centerBaseline } from './CenterBaseline'; interface S2TreeProps { // Only detatched is supported right now with the current styles from Spectrum @@ -182,10 +183,10 @@ const treeCellGrid = style({ display: 'grid', width: 'full', alignItems: 'center', - gridTemplateColumns: ['minmax(0, auto)', 'minmax(0, auto)', 'minmax(0, auto)', 40, 'minmax(0, auto)', '1fr', 'minmax(0, auto)'], + gridTemplateColumns: ['minmax(0, auto)', 'minmax(0, auto)', 'minmax(0, auto)', 40, 'minmax(0, auto)', '1fr', 'minmax(0, auto)', 'auto'], gridTemplateRows: '1fr', gridTemplateAreas: [ - 'drag-handle checkbox level-padding expand-button icon content actions' + 'drag-handle checkbox level-padding expand-button icon content actions actionmenu' ], backgroundColor: '--rowBackgroundColor', color: { @@ -268,7 +269,11 @@ const treeCheckbox = style({ const treeIcon = style({ gridArea: 'icon', - marginEnd: 'text-to-visual' + marginEnd: 'text-to-visual', + '--iconPrimary': { + type: 'fill', + value: 'currentColor' + } }); const treeContent = style({ @@ -287,6 +292,10 @@ const treeActions = style({ marginEnd: 4 }); +const treeActionMenu = style({ + gridArea: 'actionmenu' +}); + const cellFocus = { outlineStyle: { default: 'none', @@ -385,8 +394,12 @@ export const TreeViewItem = (props: TreeViewItemProps) => { {content} diff --git a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx index 1688dae489b..cf390f764c0 100644 --- a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx +++ b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx @@ -173,11 +173,11 @@ export const Example = { let rows = [ {id: 'projects', name: 'Projects', icon: , childItems: [ - {id: 'project-1', name: 'Project 1', icon: }, - {id: 'project-2', name: 'Project 2', icon: , childItems: [ - {id: 'project-2A', name: 'Project 2A', icon: }, - {id: 'project-2B', name: 'Project 2B', icon: }, - {id: 'project-2C', name: 'Project 2C', icon: } + {id: 'project-1', name: 'Project 1 Level 1', icon: }, + {id: 'project-2', name: 'Project 2 Level 1', icon: , childItems: [ + {id: 'project-2A', name: 'Project 2A Level 2', icon: }, + {id: 'project-2B', name: 'Project 2B Level 2', icon: }, + {id: 'project-2C', name: 'Project 2C Level 3', icon: } ]}, {id: 'project-3', name: 'Project 3', icon: }, {id: 'project-4', name: 'Project 4', icon: }, @@ -228,7 +228,7 @@ export const Dynamic = { render: TreeExampleDynamic, args: { ...Example.args, - disabledKeys: ['Foo 5'] + disabledKeys: ['project-2C', 'project-5'] } }; From 4c58a3c75493582046c8681734e4f5f2e0d974ff Mon Sep 17 00:00:00 2001 From: GitHub Date: Tue, 3 Dec 2024 08:34:26 +1100 Subject: [PATCH 14/31] fix lint --- 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 2f1f55658a2..4133683e231 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -22,6 +22,7 @@ import { UNSTABLE_TreeItemContent, useContextProps } from 'react-aria-components'; +import {centerBaseline} from './CenterBaseline'; import Chevron from '../ui-icons/Chevron'; import {colorMix, fontRelative, lightDark, style} from '../style' with {type: 'macro'}; import {DOMRef, Key} from '@react-types/shared'; @@ -32,7 +33,6 @@ import {StylesPropWithHeight, UnsafeStyles} from './style-utils'; import {useButton} from '@react-aria/button'; import {useDOMRef} from '@react-spectrum/utils'; import {useLocale} from '@react-aria/i18n'; -import { centerBaseline } from './CenterBaseline'; interface S2TreeProps { // Only detatched is supported right now with the current styles from Spectrum From 52d8a40c7e3a4f0dd50c31f00978e35c005ee1e9 Mon Sep 17 00:00:00 2001 From: GitHub Date: Tue, 3 Dec 2024 08:38:14 +1100 Subject: [PATCH 15/31] update snapshots --- packages/@react-aria/test-utils/src/events.ts | 5 +- .../s2/style/__tests__/style-macro.test.js | 66 +++++++++---------- 2 files changed, 34 insertions(+), 37 deletions(-) diff --git a/packages/@react-aria/test-utils/src/events.ts b/packages/@react-aria/test-utils/src/events.ts index 47183e630b8..364a6c071e7 100644 --- a/packages/@react-aria/test-utils/src/events.ts +++ b/packages/@react-aria/test-utils/src/events.ts @@ -41,10 +41,7 @@ export async function pressElement(user, element: HTMLElement, interactionType: // stick to simulting an actual user's keyboard operations as closely as possible // There are problems when using this approach though, actions like trying to trigger the select all checkbox and stuff behave oddly. act(() => element.focus()); - // TODO: would be better throw? - if (document.activeElement !== element) { - return; - } + await user.keyboard('[Space]'); } else if (interactionType === 'touch') { await user.pointer({target: element, keys: '[TouchA]'}); 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 e3541d10c02..06cac0eb36e 100644 --- a/packages/@react-spectrum/s2/style/__tests__/style-macro.test.js +++ b/packages/@react-spectrum/s2/style/__tests__/style-macro.test.js @@ -10,30 +10,30 @@ * governing permissions and limitations under the License. */ -import {style} from '../spectrum-theme'; +import { style } from "../spectrum-theme"; function testStyle(...args) { let css; let js = style.apply( { - addAsset({content}) { + addAsset({ content }) { css = content; - } + }, }, args ); - return {css, js}; + return { css, js }; } -describe('style-macro', () => { - it('should handle nested css conditions', () => { - let {css, js} = testStyle({ +describe("style-macro", () => { + it("should handle nested css conditions", () => { + let { css, js } = testStyle({ marginTop: { - ':first-child': { + ":first-child": { default: 4, - lg: 8 - } - } + lg: 8, + }, + }, }); expect(css).toMatchInlineSnapshot(` @@ -42,7 +42,7 @@ describe('style-macro', () => { @layer _.a, _.b, _.c, UNSAFE_overrides; @layer _.b { - .A-13alit4c { + .D-13alit4c { &:first-child { margin-top: 0.25rem; } @@ -51,7 +51,7 @@ describe('style-macro', () => { @layer _.c.e { @media (min-width: 1024px) { - .A-13alit4ed { + .D-13alit4ed { &:first-child { margin-top: 0.5rem; } @@ -61,14 +61,14 @@ describe('style-macro', () => { " `); - expect(js).toMatchInlineSnapshot('" . A-13alit4c A-13alit4ed"'); + expect(js).toMatchInlineSnapshot(`" . D-13alit4c D-13alit4ed"`); }); - it('should support self references', () => { - let {css} = testStyle({ + it("should support self references", () => { + let { css } = testStyle({ borderWidth: 2, - paddingX: 'edge-to-text', - width: '[calc(200px - self(borderStartWidth) - self(paddingStart))]' + paddingX: "edge-to-text", + width: "[calc(200px - self(borderStartWidth) - self(paddingStart))]", }); expect(css).toMatchInlineSnapshot(` @@ -77,48 +77,48 @@ describe('style-macro', () => { @layer _.a, _.b, UNSAFE_overrides; @layer _.a { - .uc { + .tc { border-top-width: 2px; } - .vc { + .uc { border-bottom-width: 2px; } - .s-375toy { - border-inline-start-width: var(--s); + .r-375tox { + border-inline-start-width: var(--r); } - .tc { + .sc { border-inline-end-width: 2px; } - .C-375tnm { - padding-inline-start: var(--C); + .F-375tnp { + padding-inline-start: var(--F); } - .DI { - padding-inline-end: calc(var(--k, var(--o)) * 3 / 8); + .GI { + padding-inline-end: calc(var(--j, var(--n)) * 3 / 8); } - .l-4s570k { - width: calc(200px - var(--s) - var(--C)); + .k-4s570k { + width: calc(200px - var(--r) - var(--F)); } - .-_375toy_s-c { - --s: 2px; + .-_375tox_r-c { + --r: 2px; } - .-_375tnm_C-I { - --C: calc(var(--k, var(--o)) * 3 / 8); + .-_375tnp_F-I { + --F: calc(var(--j, var(--n)) * 3 / 8); } } From 7f19c7a5665b28245b6a810becd86b61ac31e2cf Mon Sep 17 00:00:00 2001 From: GitHub Date: Tue, 3 Dec 2024 15:05:03 +1100 Subject: [PATCH 16/31] fix lint and tests --- packages/@react-aria/test-utils/src/events.ts | 1 - packages/@react-aria/test-utils/src/tree.ts | 12 ++++--- .../s2/style/__tests__/style-macro.test.js | 32 +++++++++---------- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/packages/@react-aria/test-utils/src/events.ts b/packages/@react-aria/test-utils/src/events.ts index 364a6c071e7..ae1e16dfcf7 100644 --- a/packages/@react-aria/test-utils/src/events.ts +++ b/packages/@react-aria/test-utils/src/events.ts @@ -41,7 +41,6 @@ export async function pressElement(user, element: HTMLElement, interactionType: // stick to simulting an actual user's keyboard operations as closely as possible // There are problems when using this approach though, actions like trying to trigger the select all checkbox and stuff behave oddly. act(() => element.focus()); - await user.keyboard('[Space]'); } else if (interactionType === 'touch') { await user.pointer({target: element, keys: '[TouchA]'}); diff --git a/packages/@react-aria/test-utils/src/tree.ts b/packages/@react-aria/test-utils/src/tree.ts index ba328efe7f8..b12b7d4182b 100644 --- a/packages/@react-aria/test-utils/src/tree.ts +++ b/packages/@react-aria/test-utils/src/tree.ts @@ -53,16 +53,20 @@ export class TreeTester { checkboxSelection = true, interactionType = this._interactionType } = opts; + let row = this.findRow({index, text}); + let rowCheckbox = within(row).queryByRole('checkbox'); + // Would be nice to get rid of this check + if (rowCheckbox?.getAttribute('disabled') === '') { + return; + } // this would be better than the check to do nothing in events.ts // also, it'd be good to be able to trigger selection on the row instead of having to go to the checkbox directly if (interactionType === 'keyboard' && !checkboxSelection) { - await this.keyboardNavigateToRow({row: this.findRow({index, text})}); - await this.user.keyboard('[Space]'); + await this.keyboardNavigateToRow({row}); + await this.user.keyboard('{Space}'); return; } - let row = this.findRow({index, text}); - let rowCheckbox = within(row).queryByRole('checkbox'); if (rowCheckbox) { await pressElement(this.user, rowCheckbox, interactionType); } else { 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 06cac0eb36e..fa0fee85b5f 100644 --- a/packages/@react-spectrum/s2/style/__tests__/style-macro.test.js +++ b/packages/@react-spectrum/s2/style/__tests__/style-macro.test.js @@ -10,30 +10,30 @@ * governing permissions and limitations under the License. */ -import { style } from "../spectrum-theme"; +import {style} from '../spectrum-theme'; function testStyle(...args) { let css; let js = style.apply( { - addAsset({ content }) { + addAsset({content}) { css = content; - }, + } }, args ); - return { css, js }; + return {css, js}; } -describe("style-macro", () => { - it("should handle nested css conditions", () => { - let { css, js } = testStyle({ +describe('style-macro', () => { + it('should handle nested css conditions', () => { + let {css, js} = testStyle({ marginTop: { - ":first-child": { + ':first-child': { default: 4, - lg: 8, - }, - }, + lg: 8 + } + } }); expect(css).toMatchInlineSnapshot(` @@ -61,14 +61,14 @@ describe("style-macro", () => { " `); - expect(js).toMatchInlineSnapshot(`" . D-13alit4c D-13alit4ed"`); + expect(js).toMatchInlineSnapshot('" . D-13alit4c D-13alit4ed"'); }); - it("should support self references", () => { - let { css } = testStyle({ + it('should support self references', () => { + let {css} = testStyle({ borderWidth: 2, - paddingX: "edge-to-text", - width: "[calc(200px - self(borderStartWidth) - self(paddingStart))]", + paddingX: 'edge-to-text', + width: '[calc(200px - self(borderStartWidth) - self(paddingStart))]' }); expect(css).toMatchInlineSnapshot(` From aa3b45ed7ac1be30797f8df410db0b0d9a0ef558 Mon Sep 17 00:00:00 2001 From: GitHub Date: Tue, 3 Dec 2024 16:00:15 +1100 Subject: [PATCH 17/31] fix imports --- packages/@react-spectrum/s2/src/TreeView.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 4133683e231..fde1fb650d8 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -10,7 +10,8 @@ * governing permissions and limitations under the License. */ -import {ActionButtonGroupContext, ActionMenuContext, Checkbox, IconContext, Text, TextContext} from '@react-spectrum/s2'; +import {ActionButtonGroupContext} from './ActionButtonGroup'; +import {ActionMenuContext} from './ActionMenu'; import { ButtonContext, Collection, @@ -23,13 +24,16 @@ import { useContextProps } from 'react-aria-components'; import {centerBaseline} from './CenterBaseline'; +import {Checkbox} from './Checkbox'; import Chevron from '../ui-icons/Chevron'; import {colorMix, fontRelative, lightDark, style} from '../style' with {type: 'macro'}; import {DOMRef, Key} from '@react-types/shared'; +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, useRef} from 'react'; import {StylesPropWithHeight, UnsafeStyles} from './style-utils'; +import {Text, TextContext} from './Content'; import {useButton} from '@react-aria/button'; import {useDOMRef} from '@react-spectrum/utils'; import {useLocale} from '@react-aria/i18n'; From 159948f8bd69b2ba8ccd7cd6610c9a6d633d71f5 Mon Sep 17 00:00:00 2001 From: GitHub Date: Tue, 3 Dec 2024 16:07:11 +1100 Subject: [PATCH 18/31] Match row selection to table --- packages/@react-spectrum/s2/src/TreeView.tsx | 30 +++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index fde1fb650d8..4b94f0f201b 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -101,8 +101,7 @@ const tree = style({ function TreeView(props: TreeViewProps, ref: DOMRef) { let {children, isDetached, isEmphasized} = props; - isDetached = true; - isEmphasized = false; + isEmphasized = isDetached ? false : true; let renderer; if (typeof children === 'function') { @@ -217,7 +216,7 @@ const treeCellGrid = style({ borderTopColor: { default: 'transparent', isSelected: { - isFirst: '--rowSelectedBorderColor' + isFirst: 'transparent' }, isDetached: { default: 'transparent', @@ -226,7 +225,7 @@ const treeCellGrid = style({ }, borderInlineEndColor: { default: 'transparent', - isSelected: '--rowSelectedBorderColor', + isSelected: 'transparent', isDetached: { default: 'transparent', isSelected: '--rowSelectedBorderColor' @@ -234,9 +233,9 @@ const treeCellGrid = style({ }, borderBottomColor: { default: 'transparent', - isSelected: '--rowSelectedBorderColor', - isNextSelected: '--rowSelectedBorderColor', - isNextFocused: '--rowForcedFocusBorderColor', + isSelected: 'transparent', + isNextSelected: 'transparent', + isNextFocused: 'transparent', isDetached: { default: 'transparent', isSelected: '--rowSelectedBorderColor' @@ -244,7 +243,7 @@ const treeCellGrid = style({ }, borderInlineStartColor: { default: 'transparent', - isSelected: '--rowSelectedBorderColor', + isSelected: 'transparent', isDetached: { default: 'transparent', isSelected: '--rowSelectedBorderColor' @@ -255,9 +254,18 @@ const treeCellGrid = style({ isFirst: 1, isDetached: 1 }, - borderBottomWidth: 1, - borderStartWidth: 1, - borderEndWidth: 1, + borderBottomWidth: { + default: 0, + isDetached: 1, + }, + borderStartWidth: { + default: 0, + isDetached: 1, + }, + borderEndWidth: { + default: 0, + isDetached: 1, + }, borderStyle: 'solid', borderRadius: { // odd behaviour, if this is the last property, then bottom right isn't rounded isDetached: '[6px]' From d97a79f4f5342b4a6d04630bc427dafd6617a3d7 Mon Sep 17 00:00:00 2001 From: GitHub Date: Tue, 3 Dec 2024 21:30:17 +1100 Subject: [PATCH 19/31] fix lint, again --- packages/@react-spectrum/s2/src/TreeView.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 4b94f0f201b..d9eaf2d0b05 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -101,7 +101,7 @@ const tree = style({ function TreeView(props: TreeViewProps, ref: DOMRef) { let {children, isDetached, isEmphasized} = props; - isEmphasized = isDetached ? false : true; + isEmphasized = !isDetached; let renderer; if (typeof children === 'function') { @@ -256,15 +256,15 @@ const treeCellGrid = style({ }, borderBottomWidth: { default: 0, - isDetached: 1, + isDetached: 1 }, borderStartWidth: { default: 0, - isDetached: 1, + isDetached: 1 }, borderEndWidth: { default: 0, - isDetached: 1, + isDetached: 1 }, borderStyle: 'solid', borderRadius: { // odd behaviour, if this is the last property, then bottom right isn't rounded From 31083d8f38f2493834ce5fd3048f108eba6613fe Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Tue, 14 Jan 2025 12:02:44 +1100 Subject: [PATCH 20/31] feat: S2 treeview virtualized (#7465) * feature: s2 treeview virtualization * support both detached and non in the layout * fix lint --- packages/@react-spectrum/s2/src/TreeView.tsx | 51 +++++++++++-------- .../s2/stories/TreeView.stories.tsx | 5 +- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index d9eaf2d0b05..f743cf61125 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -18,9 +18,11 @@ import { Provider, TreeItemProps as RACTreeItemProps, TreeProps as RACTreeProps, + UNSTABLE_ListLayout, UNSTABLE_Tree, UNSTABLE_TreeItem, UNSTABLE_TreeItemContent, + UNSTABLE_Virtualizer, useContextProps } from 'react-aria-components'; import {centerBaseline} from './CenterBaseline'; @@ -28,11 +30,11 @@ import {Checkbox} from './Checkbox'; import Chevron from '../ui-icons/Chevron'; import {colorMix, fontRelative, lightDark, style} from '../style' with {type: 'macro'}; import {DOMRef, Key} from '@react-types/shared'; +import {getAllowedOverrides, StylesPropWithHeight, UnsafeStyles} from './style-utils' with {type: 'macro'}; 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, useRef} from 'react'; -import {StylesPropWithHeight, UnsafeStyles} from './style-utils'; +import React, {createContext, forwardRef, isValidElement, JSXElementConstructor, ReactElement, useContext, useMemo, useRef} from 'react'; import {Text, TextContext} from './Content'; import {useButton} from '@react-aria/button'; import {useDOMRef} from '@react-spectrum/utils'; @@ -75,6 +77,11 @@ let InternalTreeContext = createContext<{isDetached?: boolean, isEmphasized?: bo // keyboard focus ring. Perhaps find a different way of rendering the outlines since the top of the item doesn't // scroll into view due to how the ring is offset. Alternatively, have the tree render the top/bottom outline like it does in Listview const tree = style({ + userSelect: 'none', + minHeight: 0, + minWidth: 0, + width: 'full', + overflow: 'auto', boxSizing: 'border-box', justifyContent: { isEmpty: 'center' @@ -82,22 +89,14 @@ const tree = style({ alignItems: { isEmpty: 'center' }, - width: { - isEmpty: 'full' - }, height: { isEmpty: 'full' }, - display: 'flex', - flexDirection: 'column', - gap: { - isDetached: 2 - }, '--indent': { type: 'width', value: 16 } -}); +}, getAllowedOverrides({height: true})); function TreeView(props: TreeViewProps, ref: DOMRef) { let {children, isDetached, isEmphasized} = props; @@ -110,18 +109,26 @@ function TreeView(props: TreeViewProps, ref: DOMRef) { let domRef = useDOMRef(ref); + let layout = useMemo(() => { + return new UNSTABLE_ListLayout({ + rowHeight: isDetached ? 42 : 40 + }); + }, [isDetached]); + return ( - - - tree({isEmpty, isDetached})} - selectionBehavior="toggle" - ref={domRef}> - {props.children} - - - + + + + tree({isEmpty, isDetached}, props.styles)} + selectionBehavior="toggle" + ref={domRef}> + {props.children} + + + + ); } diff --git a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx index cf390f764c0..eff6fbef7b0 100644 --- a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx +++ b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx @@ -197,12 +197,13 @@ let rows = [ {id: 'reports-1B', name: 'Reports 1B', icon: }, {id: 'reports-1C', name: 'Reports 1C', icon: } ]}, - {id: 'reports-2', name: 'Reports 2', icon: } + {id: 'reports-2', name: 'Reports 2', icon: }, + ...Array.from({length: 100}, (_, i) => ({id: `reports-repeat-${i}`, name: `Reports ${i}`, icon: })) ]} ]; const TreeExampleDynamic = (args) => ( -
+
{(item: any) => ( From 10419df69a5520284278aa9fbb74a73881fd5ff5 Mon Sep 17 00:00:00 2001 From: GitHub Date: Tue, 14 Jan 2025 12:28:42 +1100 Subject: [PATCH 21/31] fix height for docs --- packages/@react-spectrum/s2/stories/TreeView.stories.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx index eff6fbef7b0..326775797a6 100644 --- a/packages/@react-spectrum/s2/stories/TreeView.stories.tsx +++ b/packages/@react-spectrum/s2/stories/TreeView.stories.tsx @@ -69,7 +69,7 @@ export default meta; const TreeExampleStatic = (args) => ( -
+
( -
+
{(item: any) => ( @@ -256,7 +256,7 @@ export const Empty = { }; const TreeExampleWithLinks = (args) => ( -
+
{(item) => ( From 11ddb5dd869f51eb9ffa2e1997b6de9112e0277e Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 15 Jan 2025 13:50:46 +1100 Subject: [PATCH 22/31] default tree disabled behavior to all to match our other components --- packages/@react-spectrum/s2/src/TreeView.tsx | 1 - packages/react-aria-components/src/Tree.tsx | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index f743cf61125..0a816a46383 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -48,7 +48,6 @@ interface S2TreeProps { isEmphasized?: boolean } -// should we remove disabledBehavior? export interface TreeViewProps extends Omit, 'style' | 'className' | 'onRowAction' | 'selectionBehavior' | 'onScroll' | 'onCellAction' | 'dragAndDropHooks'>, UnsafeStyles, S2TreeProps { /** Spectrum-defined styles, returned by the `style()` macro. */ styles?: StylesPropWithHeight diff --git a/packages/react-aria-components/src/Tree.tsx b/packages/react-aria-components/src/Tree.tsx index b994fc015f2..072d041d489 100644 --- a/packages/react-aria-components/src/Tree.tsx +++ b/packages/react-aria-components/src/Tree.tsx @@ -127,7 +127,7 @@ export interface TreeProps extends Omit, 'children'> renderEmptyState?: (props: TreeEmptyStateRenderProps) => ReactNode, /** * Whether `disabledKeys` applies to all interactions, or only selection. - * @default 'selection' + * @default 'all' */ disabledBehavior?: DisabledBehavior } @@ -163,7 +163,7 @@ function TreeInner({props, collection, treeRef: ref}: TreeInne expandedKeys: propExpandedKeys, defaultExpandedKeys: propDefaultExpandedKeys, onExpandedChange, - disabledBehavior = 'selection' + disabledBehavior = 'all' } = props; let {CollectionRoot, isVirtualized, layoutDelegate} = useContext(CollectionRendererContext); From d5c2b1cb3cbaa2853340435773deda19646d90b6 Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 15 Jan 2025 17:18:43 +1100 Subject: [PATCH 23/31] fix tests --- packages/@react-spectrum/s2/test/Tree.test.tsx | 2 +- packages/@react-spectrum/tree/test/TreeView.test.tsx | 6 +++--- packages/react-aria-components/test/Tree.test.tsx | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/@react-spectrum/s2/test/Tree.test.tsx b/packages/@react-spectrum/s2/test/Tree.test.tsx index ac6e468dd3b..5b9f4aa1cff 100644 --- a/packages/@react-spectrum/s2/test/Tree.test.tsx +++ b/packages/@react-spectrum/s2/test/Tree.test.tsx @@ -63,7 +63,7 @@ AriaTreeTests({ ), singleSelection: () => render( - + Photos diff --git a/packages/@react-spectrum/tree/test/TreeView.test.tsx b/packages/@react-spectrum/tree/test/TreeView.test.tsx index 5af771e4069..f319033b4b6 100644 --- a/packages/@react-spectrum/tree/test/TreeView.test.tsx +++ b/packages/@react-spectrum/tree/test/TreeView.test.tsx @@ -571,7 +571,7 @@ describe('Tree', () => { await user.click(row); expect(row).toHaveAttribute('data-focused'); - rerender(tree, ); + rerender(tree, ); row = tree.getAllByRole('row')[0]; expect(row).not.toHaveAttribute('data-focus-visible'); @@ -1191,7 +1191,7 @@ describe('Tree', () => { it('should add a tab index to the chevron if the row isnt completely disabled', () => { let tree = render( - + Test @@ -1203,7 +1203,7 @@ describe('Tree', () => { rerender( tree, - + Test diff --git a/packages/react-aria-components/test/Tree.test.tsx b/packages/react-aria-components/test/Tree.test.tsx index b3c6df16214..4100ec51a6e 100644 --- a/packages/react-aria-components/test/Tree.test.tsx +++ b/packages/react-aria-components/test/Tree.test.tsx @@ -520,7 +520,7 @@ describe('Tree', () => { }); it('should not update the press state if the row is not interactive', () => { - let {getAllByRole, rerender} = render( isPressed ? 'pressed' : ''}} />); + let {getAllByRole, rerender} = render( isPressed ? 'pressed' : ''}} />); let row = getAllByRole('row')[0]; expect(row).not.toHaveAttribute('data-pressed'); @@ -557,7 +557,7 @@ describe('Tree', () => { }); it('should support focus', async () => { - let {getAllByRole, rerender} = render( isFocused ? 'focus' : ''}} />); + let {getAllByRole, rerender} = render( isFocused ? 'focus' : ''}} />); let row = getAllByRole('row')[0]; expect(row).not.toHaveAttribute('data-focused'); @@ -567,7 +567,7 @@ describe('Tree', () => { expect(row).toHaveAttribute('data-focused'); expect(row).toHaveClass('focus'); - rerender( isFocusVisible ? 'focus-visible' : ''}} />); + rerender( isFocusVisible ? 'focus-visible' : ''}} />); row = getAllByRole('row')[0]; expect(row).not.toHaveAttribute('data-focus-visible'); expect(row).not.toHaveClass('focus-visible'); @@ -1211,7 +1211,7 @@ AriaTreeTests({ ), singleSelection: () => render( - + Photos @@ -1341,7 +1341,7 @@ AriaTreeTests({ ), singleSelection: () => render( - + ), allInteractionsDisabled: () => render( From 75f8ade89806a852cd1499961b2dfcf651863c29 Mon Sep 17 00:00:00 2001 From: GitHub Date: Tue, 21 Jan 2025 08:53:15 +1100 Subject: [PATCH 24/31] update snapshots --- .../s2/style/__tests__/style-macro.test.js | 66 +++++++++---------- 1 file changed, 33 insertions(+), 33 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 3762768c1b6..e320f664ba8 100644 --- a/packages/@react-spectrum/s2/style/__tests__/style-macro.test.js +++ b/packages/@react-spectrum/s2/style/__tests__/style-macro.test.js @@ -10,37 +10,37 @@ * governing permissions and limitations under the License. */ -import {style} from '../spectrum-theme'; +import { style } from "../spectrum-theme"; function testStyle(...args) { let css; let js = style.apply( { - addAsset({content}) { + addAsset({ content }) { css = content; - } + }, }, args ); - return {css, js}; + return { css, js }; } -describe('style-macro', () => { - it('should handle nested css conditions', () => { - let {css, js} = testStyle({ +describe("style-macro", () => { + it("should handle nested css conditions", () => { + let { css, js } = testStyle({ marginTop: { - ':first-child': { + ":first-child": { default: 4, - lg: 8 - } - } + lg: 8, + }, + }, }); expect(css).toMatchInlineSnapshot(` "@layer _.a, _.b, _.c; @layer _.b { - .A-13alit4c { + .D-13alit4c { &:first-child { margin-top: 0.25rem; } @@ -49,7 +49,7 @@ describe('style-macro', () => { @layer _.c.e { @media (min-width: 1024px) { - .A-13alit4ed { + .D-13alit4ed { &:first-child { margin-top: 0.5rem; } @@ -59,62 +59,62 @@ describe('style-macro', () => { " `); - expect(js).toMatchInlineSnapshot('" A-13alit4c A-13alit4ed"'); + expect(js).toMatchInlineSnapshot(`" D-13alit4c D-13alit4ed"`); }); - it('should support self references', () => { - let {css} = testStyle({ + it("should support self references", () => { + let { css } = testStyle({ borderWidth: 2, - paddingX: 'edge-to-text', - width: '[calc(200px - self(borderStartWidth) - self(paddingStart))]' + paddingX: "edge-to-text", + width: "[calc(200px - self(borderStartWidth) - self(paddingStart))]", }); expect(css).toMatchInlineSnapshot(` "@layer _.a, _.b; @layer _.a { - .uc { + .tc { border-top-width: 2px; } - .vc { + .uc { border-bottom-width: 2px; } - .s-375toy { - border-inline-start-width: var(--s); + .r-375tox { + border-inline-start-width: var(--r); } - .tc { + .sc { border-inline-end-width: 2px; } - .C-375tnm { - padding-inline-start: var(--C); + .F-375tnp { + padding-inline-start: var(--F); } - .DI { - padding-inline-end: calc(var(--k, var(--o)) * 3 / 8); + .GI { + padding-inline-end: calc(var(--j, var(--n)) * 3 / 8); } - .l-4s570k { - width: calc(200px - var(--s) - var(--C)); + .k-4s570k { + width: calc(200px - var(--r) - var(--F)); } - .-_375toy_s-c { - --s: 2px; + .-_375tox_r-c { + --r: 2px; } - .-_375tnm_C-I { - --C: calc(var(--k, var(--o)) * 3 / 8); + .-_375tnp_F-I { + --F: calc(var(--j, var(--n)) * 3 / 8); } } From 783b78ab47578e5cefb6013f2ecc9f7f6a627ebd Mon Sep 17 00:00:00 2001 From: GitHub Date: Tue, 21 Jan 2025 08:55:53 +1100 Subject: [PATCH 25/31] fix lint --- .../s2/style/__tests__/style-macro.test.js | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 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 e320f664ba8..4e44c17866f 100644 --- a/packages/@react-spectrum/s2/style/__tests__/style-macro.test.js +++ b/packages/@react-spectrum/s2/style/__tests__/style-macro.test.js @@ -10,30 +10,30 @@ * governing permissions and limitations under the License. */ -import { style } from "../spectrum-theme"; +import {style} from '../spectrum-theme'; function testStyle(...args) { let css; let js = style.apply( { - addAsset({ content }) { + addAsset({content}) { css = content; - }, + } }, args ); - return { css, js }; + return {css, js}; } -describe("style-macro", () => { - it("should handle nested css conditions", () => { - let { css, js } = testStyle({ +describe('style-macro', () => { + it('should handle nested css conditions', () => { + let {css, js} = testStyle({ marginTop: { - ":first-child": { + ':first-child': { default: 4, - lg: 8, - }, - }, + lg: 8 + } + } }); expect(css).toMatchInlineSnapshot(` @@ -59,14 +59,14 @@ describe("style-macro", () => { " `); - expect(js).toMatchInlineSnapshot(`" D-13alit4c D-13alit4ed"`); + expect(js).toMatchInlineSnapshot('" D-13alit4c D-13alit4ed"'); }); - it("should support self references", () => { - let { css } = testStyle({ + it('should support self references', () => { + let {css} = testStyle({ borderWidth: 2, - paddingX: "edge-to-text", - width: "[calc(200px - self(borderStartWidth) - self(paddingStart))]", + paddingX: 'edge-to-text', + width: '[calc(200px - self(borderStartWidth) - self(paddingStart))]' }); expect(css).toMatchInlineSnapshot(` From bf7365dffa3b773a0153515bd5f5cffc84aad986 Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 22 Jan 2025 13:09:07 +1100 Subject: [PATCH 26/31] fix alignment and remove minwidth --- packages/@react-spectrum/s2/src/TreeView.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 0a816a46383..8baf7961ed0 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -165,7 +165,6 @@ const treeRow = style({ display: 'flex', height: 40, width: 'full', - minWidth: 240, // do we really want this restriction? boxSizing: 'border-box', font: 'ui', color: 'body', @@ -191,6 +190,7 @@ const treeRow = style({ const treeCellGrid = style({ display: 'grid', width: 'full', + alignContent: 'center', alignItems: 'center', gridTemplateColumns: ['minmax(0, auto)', 'minmax(0, auto)', 'minmax(0, auto)', 40, 'minmax(0, auto)', '1fr', 'minmax(0, auto)', 'auto'], gridTemplateRows: '1fr', @@ -198,6 +198,7 @@ const treeCellGrid = style({ 'drag-handle checkbox level-padding expand-button icon content actions actionmenu' ], backgroundColor: '--rowBackgroundColor', + paddingEnd: 4, // account for any focus rings on the last item in the cell color: { isDisabled: { default: 'gray-400', @@ -272,10 +273,10 @@ const treeCellGrid = style({ default: 0, isDetached: 1 }, - borderStyle: 'solid', - borderRadius: { // odd behaviour, if this is the last property, then bottom right isn't rounded + borderRadius: { isDetached: '[6px]' - } + }, + borderStyle: 'solid' }); const treeCheckbox = style({ From d814ccb57770191e52f5f4bd2e9e6d18d2b9a344 Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 22 Jan 2025 13:11:08 +1100 Subject: [PATCH 27/31] increase gap between detached items --- 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 8baf7961ed0..e859fc6258c 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -110,7 +110,7 @@ function TreeView(props: TreeViewProps, ref: DOMRef) { let layout = useMemo(() => { return new UNSTABLE_ListLayout({ - rowHeight: isDetached ? 42 : 40 + rowHeight: isDetached ? 44 : 40 }); }, [isDetached]); From 7c84032b1bb1f495d8bd39fecdff97588a8ad89e Mon Sep 17 00:00:00 2001 From: GitHub Date: Thu, 23 Jan 2025 12:57:24 +1100 Subject: [PATCH 28/31] remove restriction for isDetached and isEmphasized --- packages/@react-spectrum/s2/src/TreeView.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index e859fc6258c..f70ed5db931 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -99,7 +99,6 @@ const tree = style({ function TreeView(props: TreeViewProps, ref: DOMRef) { let {children, isDetached, isEmphasized} = props; - isEmphasized = !isDetached; let renderer; if (typeof children === 'function') { @@ -354,8 +353,6 @@ export const TreeViewItem = (props: TreeViewItemProps) => { let nestedRows; let {renderer} = useTreeRendererContext(); let {isDetached, isEmphasized} = useContext(InternalTreeContext); - // 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 if (typeof children === 'string') { content = {children}; @@ -380,7 +377,6 @@ 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 : '')}> From c76f76201f466b0229d0a2647ac36fc090247a41 Mon Sep 17 00:00:00 2001 From: GitHub Date: Thu, 23 Jan 2025 13:01:08 +1100 Subject: [PATCH 29/31] fix border radius --- 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 f70ed5db931..4861fca71e0 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -273,7 +273,7 @@ const treeCellGrid = style({ isDetached: 1 }, borderRadius: { - isDetached: '[6px]' + isDetached: 'default' }, borderStyle: 'solid' }); From 7806b6ebdf9ae8df4cb42a66e0188f354676c483 Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 29 Jan 2025 14:21:11 +1100 Subject: [PATCH 30/31] review comments --- packages/@react-spectrum/s2/package.json | 2 -- packages/@react-spectrum/s2/src/TreeView.tsx | 38 ++++++++++---------- yarn.lock | 2 -- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/packages/@react-spectrum/s2/package.json b/packages/@react-spectrum/s2/package.json index f2fad3dd0d4..0a1e395b3e2 100644 --- a/packages/@react-spectrum/s2/package.json +++ b/packages/@react-spectrum/s2/package.json @@ -129,9 +129,7 @@ "jest": "^29.5.0" }, "dependencies": { - "@react-aria/button": "^3.11.1", "@react-aria/collections": "3.0.0-alpha.7", - "@react-aria/i18n": "^3.12.5", "@react-aria/interactions": "^3.23.0", "@react-aria/live-announcer": "^3.4.1", "@react-aria/tree": "3.0.0-beta.3", diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 4861fca71e0..9ead35db425 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -13,6 +13,7 @@ import {ActionButtonGroupContext} from './ActionButtonGroup'; import {ActionMenuContext} from './ActionMenu'; import { + Button, ButtonContext, Collection, Provider, @@ -36,9 +37,8 @@ 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 {useButton} from '@react-aria/button'; +import {useLocale} from 'react-aria'; import {useDOMRef} from '@react-spectrum/utils'; -import {useLocale} from '@react-aria/i18n'; interface S2TreeProps { // Only detatched is supported right now with the current styles from Spectrum @@ -109,7 +109,7 @@ function TreeView(props: TreeViewProps, ref: DOMRef) { let layout = useMemo(() => { return new UNSTABLE_ListLayout({ - rowHeight: isDetached ? 44 : 40 + rowHeight: isDetached ? 42 : 40 }); }, [isDetached]); @@ -257,7 +257,10 @@ const treeCellGrid = style({ }, borderTopWidth: { default: 0, - isFirst: 1, + isFirst: { + default: 1, + forcedColors: 0 + }, isDetached: 1 }, borderBottomWidth: { @@ -414,7 +417,7 @@ export const TreeViewItem = (props: TreeViewItemProps) => { styles: style({size: fontRelative(20), flexShrink: 0}) }], [ActionButtonGroupContext, {styles: treeActions}], - [ActionMenuContext, {styles: treeActionMenu, isQuiet: true, 'aria-label': 'Actions'}] + [ActionMenuContext, {styles: treeActionMenu, isQuiet: true}] ]}> {content} @@ -450,29 +453,26 @@ const expandButton = style({ isRTL: 'rotate(-90deg)' } }, - transition: 'default' + transition: 'default', + backgroundColor: 'transparent', + borderStyle: 'none' }); function ExpandableRowChevron(props: ExpandableRowChevronProps) { - let expandButtonRef = useRef(null); - // @ts-ignore - check back on this + let expandButtonRef = useRef(null); let [fullProps, ref] = useContextProps({...props, slot: 'chevron'}, expandButtonRef, ButtonContext); let {isExpanded, isDisabled} = fullProps; let {direction} = useLocale(); - // Will need to keep the chevron as a button for iOS VO at all times since VO doesn't focus the cell. Also keep as button if cellAction is defined by the user in the future - let {buttonProps} = useButton({ - ...fullProps, - elementType: 'span' - }, ref); - return ( - + excludeFromTabOrder={isAndroid() && !isDisabled} + preventFocusOnPress + className={renderProps => expandButton({...renderProps, isExpanded, isRTL: direction === 'rtl'})}> - + ); } diff --git a/yarn.lock b/yarn.lock index 80fa3f3fb08..980699c8e8a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7894,9 +7894,7 @@ __metadata: dependencies: "@adobe/spectrum-tokens": "npm:^13.0.0-beta.53" "@parcel/macros": "npm:^2.13.0" - "@react-aria/button": "npm:^3.11.1" "@react-aria/collections": "npm:3.0.0-alpha.7" - "@react-aria/i18n": "npm:^3.12.5" "@react-aria/interactions": "npm:^3.23.0" "@react-aria/live-announcer": "npm:^3.4.1" "@react-aria/test-utils": "npm:1.0.0-alpha.3" From 685e7af617132e490088cd7257afbf8a4593cd7b Mon Sep 17 00:00:00 2001 From: GitHub Date: Wed, 29 Jan 2025 14:35:01 +1100 Subject: [PATCH 31/31] fix lint --- packages/@react-spectrum/s2/package.json | 1 + packages/@react-spectrum/s2/src/TreeView.tsx | 2 +- yarn.lock | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/@react-spectrum/s2/package.json b/packages/@react-spectrum/s2/package.json index 0a1e395b3e2..824aeb66789 100644 --- a/packages/@react-spectrum/s2/package.json +++ b/packages/@react-spectrum/s2/package.json @@ -130,6 +130,7 @@ }, "dependencies": { "@react-aria/collections": "3.0.0-alpha.7", + "@react-aria/i18n": "^3.12.5", "@react-aria/interactions": "^3.23.0", "@react-aria/live-announcer": "^3.4.1", "@react-aria/tree": "3.0.0-beta.3", diff --git a/packages/@react-spectrum/s2/src/TreeView.tsx b/packages/@react-spectrum/s2/src/TreeView.tsx index 9ead35db425..639678a7e0b 100644 --- a/packages/@react-spectrum/s2/src/TreeView.tsx +++ b/packages/@react-spectrum/s2/src/TreeView.tsx @@ -37,8 +37,8 @@ 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 {useLocale} from 'react-aria'; import {useDOMRef} from '@react-spectrum/utils'; +import {useLocale} from 'react-aria'; interface S2TreeProps { // Only detatched is supported right now with the current styles from Spectrum diff --git a/yarn.lock b/yarn.lock index 980699c8e8a..73de7b18d15 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7895,6 +7895,7 @@ __metadata: "@adobe/spectrum-tokens": "npm:^13.0.0-beta.53" "@parcel/macros": "npm:^2.13.0" "@react-aria/collections": "npm:3.0.0-alpha.7" + "@react-aria/i18n": "npm:^3.12.5" "@react-aria/interactions": "npm:^3.23.0" "@react-aria/live-announcer": "npm:^3.4.1" "@react-aria/test-utils": "npm:1.0.0-alpha.3"