feat: S2 treeview #7343

snowystinger · 2024-11-08T21:40:41Z

Closes

This only implements two of the combinations of isDetached/isEmphasized/Checkbox Selection.

isDetached + checkbox selection or no selection -> seemed mostly clear cut
isEmphasized + checkbox selection or no selection -> matches table
The others aren't well defined enough yet to implement.

What's left:

Selection states w/ detached+emphasized combos defined by Spectrum
Drag and Drop
thumbnail slot
skeleton/async loading
Sections / TreeView headers

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

# Conflicts: # packages/@react-spectrum/s2/package.json # yarn.lock

rspbot · 2024-12-03T05:10:34Z

Build successful! 🎉

rspbot · 2025-01-20T22:06:48Z

Build successful! 🎉

devongovett

The checkbox seems a bit off center vertically when detached. And I think the gap between items is supposed to be larger according to Figma.

packages/@react-spectrum/s2/src/TreeView.tsx

devongovett · 2025-01-22T00:59:15Z

packages/@react-spectrum/s2/src/TreeView.tsx

+  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


Is this not possible with RAC? What do we do in the RAC docs?

this got copied over from v3, @LFDanLu any idea what this is about?

if the chevron wasn't a button, iOS VO can't focus it and thus users can't toggle expansion of the row. If the tree row didn't have a row action, focusing the tree cell and double tapping would trigger expansion and thus this wouldn't be needed, but iOS doesn't focus grid cells in general, at least for iOS 17.

In RAC, it is up to the user to provide ALL the content of the TreeRow (including the chevron button) themselves hence why https://react-spectrum.adobe.com/react-aria/Tree.html#example has the a Button with the chevron slot. Here in S2/v3 we can afford to do that for the user thus we set it up here. If we want to provide a ChevronButton wrapper of sorts to expose from RAC we could go that route and ditch defining it at the S2/v3 level

But why are we using hooks here? Why is it a span instead of a button?

if I remember correctly, it was for the tabIndex handling below. I forget if we had all of the button props that we had before like excludeFromTabOrder and preventFocusOnPress

seems to work with the props @LFDanLu has suggested, but will need to verify on any Androids the team has

packages/@react-spectrum/s2/src/TreeView.tsx

rspbot · 2025-01-22T02:22:30Z

Build successful! 🎉

rspbot · 2025-01-22T02:23:12Z

Build successful! 🎉

rspbot · 2025-01-23T02:13:24Z

Build successful! 🎉

rspbot · 2025-01-23T02:13:44Z

Build successful! 🎉

rspbot · 2025-01-23T02:14:56Z

Build successful! 🎉

LFDanLu · 2025-01-24T23:34:21Z

packages/@react-spectrum/s2/src/TreeView.tsx

+
+  let layout = useMemo(() => {
+    return new UNSTABLE_ListLayout({
+      rowHeight: isDetached ? 44 : 40


I'm seeing conflicting designs in Figma for the spacing between the detached rows, but the tokens file says that it should use tree-view-item-to-item-detached which is 2px whereas the design file has 8px between each row...

looks like it should be 2, i've moved it back to that. I'll raise with design after I have a build

LFDanLu

Behavior and design looks good to me, looking at the API update PR before commenting further on the implementation. Seems like it was rather straight forward which is good to see, were there any deviations from RSP TreeView's implementation that I may not have noticed?

LFDanLu · 2025-01-24T23:37:11Z

packages/@react-spectrum/s2/src/TreeView.tsx

+// 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({


is this comment still valid?

hmmm i assume, i copied it from v3 it looks like, i didn't think about it much though

packages/@react-spectrum/s2/src/TreeView.tsx

devongovett

fine if you want to merge and iterate

packages/@react-spectrum/s2/package.json

packages/@react-spectrum/s2/src/TreeView.tsx

devongovett · 2025-01-28T18:37:44Z

packages/@react-spectrum/s2/src/TreeView.tsx

+  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


But why are we using hooks here? Why is it a span instead of a button?

devongovett · 2025-01-28T18:39:21Z

packages/@react-spectrum/s2/style/spectrum-theme.ts

+    borderInlineStartColor: borderColor, // don't know why i can't use renamed
+    borderInlineEndColor: borderColor,
+    borderTopColor: borderColor,
+    borderBottomColor: borderColor,


Do we really need these? When do the borders have different colors? Adding these will increase the CSS size across all components so want to be sure it's actually needed. Looks like maybe you could set the width of one of the borders to zero instead of setting it to transparent?

using 0/1 instead of 1 always with transparent causes elements to shift their position internal to the tree item

devongovett · 2025-01-28T18:45:36Z

packages/@react-spectrum/s2/style/spectrum-theme.ts

@@ -670,6 +672,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,


If we are keeping these then they should be named like our other logical properties: borderStartColor, borderEndColor

yeah, as the comment says, i don't know why I can't use the createRenamedProperty like I could for borderEndWidth

rspbot · 2025-01-28T20:52:32Z

Build successful! 🎉

rspbot · 2025-01-29T03:47:23Z

Build successful! 🎉

rspbot · 2025-01-29T03:47:24Z

## API Changes

react-aria-components

/react-aria-components:UNSTABLE_Tree

 UNSTABLE_Tree <T extends {}> {
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   children?: ReactNode | ({}) => ReactNode
   className?: string | ((TreeRenderProps & {
     defaultClassName: string | undefined
 })) => string
   defaultExpandedKeys?: Iterable<Key>
   defaultSelectedKeys?: 'all' | Iterable<Key>
   dependencies?: Array<any>
-  disabledBehavior?: DisabledBehavior = 'selection'
+  disabledBehavior?: DisabledBehavior = 'all'
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
   expandedKeys?: Iterable<Key>
   id?: string
   onAction?: (Key) => void
   onExpandedChange?: (Set<Key>) => any
   onScroll?: (UIEvent<Element>) => void
   onSelectionChange?: (Selection) => void
   renderEmptyState?: (TreeEmptyStateRenderProps) => ReactNode
   selectedKeys?: 'all' | Iterable<Key>
   selectionBehavior?: SelectionBehavior
   selectionMode?: SelectionMode
   slot?: string | null
   style?: CSSProperties | ((TreeRenderProps & {
     defaultStyle: CSSProperties
 })) => CSSProperties | undefined
 }

/react-aria-components:TreeProps

 TreeProps <T> {
   aria-describedby?: string
   aria-details?: string
   aria-label?: string
   aria-labelledby?: string
   autoFocus?: boolean | FocusStrategy
   children?: ReactNode | (T) => ReactNode
   className?: string | ((TreeRenderProps & {
     defaultClassName: string | undefined
 })) => string
   defaultExpandedKeys?: Iterable<Key>
   defaultSelectedKeys?: 'all' | Iterable<Key>
   dependencies?: Array<any>
-  disabledBehavior?: DisabledBehavior = 'selection'
+  disabledBehavior?: DisabledBehavior = 'all'
   disabledKeys?: Iterable<Key>
   disallowEmptySelection?: boolean
   expandedKeys?: Iterable<Key>
   id?: string
   onAction?: (Key) => void
   onExpandedChange?: (Set<Key>) => any
   onScroll?: (UIEvent<Element>) => void
   onSelectionChange?: (Selection) => void
   renderEmptyState?: (TreeEmptyStateRenderProps) => ReactNode
   selectedKeys?: 'all' | Iterable<Key>
   selectionBehavior?: SelectionBehavior
   selectionMode?: SelectionMode
   slot?: string | null
   style?: CSSProperties | ((TreeRenderProps & {
     defaultStyle: CSSProperties
 })) => CSSProperties | undefined
 }

/react-aria-components:TreeItemContentRenderProps

 TreeItemContentRenderProps {
   allowsDragging?: boolean
   hasChildRows: boolean
+  id: Key
   isDisabled: boolean
   isDragging?: boolean
   isDropTarget?: boolean
   isExpanded: boolean
   isFocusVisible: boolean
   isFocusVisibleWithin: boolean
   isFocused: boolean
   isHovered: boolean
   isPressed: boolean
   isSelected: boolean
   level: number
   selectionBehavior: SelectionBehavior
   selectionMode: SelectionMode
+  state: TreeState<unknown>
 }

@react-spectrum/s2

/@react-spectrum/s2:TreeView

+TreeView {
+  UNSAFE_className?: string
+  UNSAFE_style?: CSSProperties
+  aria-describedby?: string
+  aria-details?: string
+  aria-label?: string
+  aria-labelledby?: string
+  autoFocus?: boolean | FocusStrategy
+  children?: ReactNode | (T) => ReactNode
+  defaultExpandedKeys?: Iterable<Key>
+  defaultSelectedKeys?: 'all' | Iterable<Key>
+  dependencies?: Array<any>
+  disabledBehavior?: DisabledBehavior = 'all'
+  disabledKeys?: Iterable<Key>
+  disallowEmptySelection?: boolean
+  expandedKeys?: Iterable<Key>
+  id?: string
+  isDetached?: boolean
+  isEmphasized?: boolean
+  items?: Iterable<T>
+  onAction?: (Key) => void
+  onExpandedChange?: (Set<Key>) => any
+  onSelectionChange?: (Selection) => void
+  renderEmptyState?: (TreeEmptyStateRenderProps) => ReactNode
+  selectedKeys?: 'all' | Iterable<Key>
+  selectionMode?: SelectionMode
+  slot?: string | null
+  styles?: StylesPropWithHeight
+}

/@react-spectrum/s2:TreeViewItem

+TreeViewItem <T extends {}> {
+  aria-label?: string
+  childItems?: Iterable<{}>
+  children: ReactNode
+  download?: boolean | string
+  hasChildItems?: boolean
+  href?: Href
+  hrefLang?: string
+  id?: Key
+  onHoverChange?: (boolean) => void
+  onHoverEnd?: (HoverEvent) => void
+  onHoverStart?: (HoverEvent) => void
+  ping?: string
+  referrerPolicy?: HTMLAttributeReferrerPolicy
+  rel?: string
+  routerOptions?: RouterOptions
+  target?: HTMLAttributeAnchorTarget
+  textValue: string
+  value?: T
+}

/@react-spectrum/s2:TreeViewProps

+TreeViewProps {
+  UNSAFE_className?: string
+  UNSAFE_style?: CSSProperties
+  aria-describedby?: string
+  aria-details?: string
+  aria-label?: string
+  aria-labelledby?: string
+  autoFocus?: boolean | FocusStrategy
+  children?: ReactNode | (T) => ReactNode
+  defaultExpandedKeys?: Iterable<Key>
+  defaultSelectedKeys?: 'all' | Iterable<Key>
+  dependencies?: Array<any>
+  disabledBehavior?: DisabledBehavior = 'all'
+  disabledKeys?: Iterable<Key>
+  disallowEmptySelection?: boolean
+  expandedKeys?: Iterable<Key>
+  id?: string
+  isDetached?: boolean
+  isEmphasized?: boolean
+  items?: Iterable<T>
+  onAction?: (Key) => void
+  onExpandedChange?: (Set<Key>) => any
+  onSelectionChange?: (Selection) => void
+  renderEmptyState?: (TreeEmptyStateRenderProps) => ReactNode
+  selectedKeys?: 'all' | Iterable<Key>
+  selectionMode?: SelectionMode
+  slot?: string | null
+  styles?: StylesPropWithHeight
+}

/@react-spectrum/s2:TreeViewItemProps

+TreeViewItemProps <T extends {} = {}> {
+  aria-label?: string
+  childItems?: Iterable<{}>
+  children: ReactNode
+  download?: boolean | string
+  hasChildItems?: boolean
+  href?: Href
+  hrefLang?: string
+  id?: Key
+  onHoverChange?: (boolean) => void
+  onHoverEnd?: (HoverEvent) => void
+  onHoverStart?: (HoverEvent) => void
+  ping?: string
+  referrerPolicy?: HTMLAttributeReferrerPolicy
+  rel?: string
+  routerOptions?: RouterOptions
+  target?: HTMLAttributeAnchorTarget
+  textValue: string
+  value?: T
+}

LFDanLu

Behavior for focusing the expand chevron seems to still work on iOS VO and Android talkback, and it is still properly skipped on keyboard. Only weird thing I saw in testing was that moving the talkback cursor caused the row to shift visibly but that seems to have been pre-exisiting since the virtualization commit. Not sure is causing that but happy for it to be followup since it is pretty low surface area

snowystinger and others added 24 commits November 5, 2024 16:38

Render a TreeView

81aa028

Correct styles

8dbce27

update styles to better fix s2

b297017

Merge branch 'main' into s2-treeview

f8b7353

fix install

4f69394

fix lint, add preliminary test

98654db

fix lint

dec417a

Add tests and test util

6b8f9ab

Merge branch 'main' into s2-treeview

1b72a1d

# Conflicts: # packages/@react-spectrum/s2/package.json # yarn.lock

save point

8bdfb54

Add emptyState

10b40f4

fix lint

e9e4bf2

detached styles

5fcfd47

Merge branch 'main' into s2-treeview

2e0a38e

Merge branch 'main' into s2-treeview

cada78a

detached styling with actions

9699bc0

fix disabled, icon placement, and add examples

3ea307f

Merge branch 'main' into s2-treeview

bc54a0d

Merge branch 'main' into s2-treeview

9197b9b

fix lint

4c58a3c

update snapshots

52d8a40

fix lint and tests

7f19c7a

fix imports

aa3b45e

Match row selection to table

159948f

snowystinger marked this pull request as ready for review December 3, 2024 05:09

Merge branch 'main' into s2-treeview

02d6af0

snowystinger changed the title ~~[WIP] - S2 treeview~~ S2 treeview Dec 3, 2024

fix lint, again

d97a79f

snowystinger changed the title ~~S2 treeview~~ feature: S2 treeview Dec 3, 2024

snowystinger added 2 commits January 21, 2025 08:53

update snapshots

75f8ade

fix lint

783b78a

devongovett reviewed Jan 22, 2025

View reviewed changes

snowystinger added 2 commits January 22, 2025 13:09

fix alignment and remove minwidth

bf7365d

increase gap between detached items

d814ccb

snowystinger and others added 3 commits January 23, 2025 12:57

remove restriction for isDetached and isEmphasized

7c84032

fix border radius

c76f762

Merge branch 'main' into s2-treeview

5d282b3

LFDanLu reviewed Jan 24, 2025

View reviewed changes

LFDanLu reviewed Jan 25, 2025

View reviewed changes

devongovett previously approved these changes Jan 28, 2025

View reviewed changes

Merge branch 'main' into s2-treeview

e495ef1

review comments

7806b6e

snowystinger dismissed devongovett’s stale review via 7806b6e January 29, 2025 03:21

fix lint

685e7af

LFDanLu approved these changes Jan 29, 2025

View reviewed changes

devongovett approved these changes Jan 29, 2025

View reviewed changes

snowystinger added this pull request to the merge queue Jan 29, 2025

Merged via the queue into main with commit e3ed3c7 Jan 29, 2025
30 checks passed

snowystinger deleted the s2-treeview branch January 29, 2025 23:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: S2 treeview #7343

feat: S2 treeview #7343

snowystinger commented Nov 8, 2024 •

edited

Loading

rspbot commented Dec 3, 2024

rspbot commented Jan 20, 2025

devongovett left a comment

devongovett Jan 22, 2025

snowystinger Jan 22, 2025

LFDanLu Jan 22, 2025

devongovett Jan 28, 2025

LFDanLu Jan 28, 2025

snowystinger Jan 29, 2025

rspbot commented Jan 22, 2025

rspbot commented Jan 22, 2025

rspbot commented Jan 23, 2025

rspbot commented Jan 23, 2025

rspbot commented Jan 23, 2025

LFDanLu Jan 24, 2025

snowystinger Jan 29, 2025

snowystinger Jan 29, 2025

LFDanLu left a comment

LFDanLu Jan 24, 2025

snowystinger Jan 29, 2025

devongovett left a comment

devongovett Jan 28, 2025

devongovett Jan 28, 2025

snowystinger Jan 29, 2025

devongovett Jan 28, 2025

snowystinger Jan 29, 2025

snowystinger Jan 29, 2025

rspbot commented Jan 28, 2025

rspbot commented Jan 29, 2025

rspbot commented Jan 29, 2025

LFDanLu left a comment

feat: S2 treeview #7343

feat: S2 treeview #7343

Conversation

snowystinger commented Nov 8, 2024 • edited Loading

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

rspbot commented Dec 3, 2024

rspbot commented Jan 20, 2025

devongovett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rspbot commented Jan 22, 2025

rspbot commented Jan 22, 2025

rspbot commented Jan 23, 2025

rspbot commented Jan 23, 2025

rspbot commented Jan 23, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LFDanLu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devongovett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rspbot commented Jan 28, 2025

rspbot commented Jan 29, 2025

rspbot commented Jan 29, 2025

react-aria-components

/react-aria-components:UNSTABLE_Tree

/react-aria-components:TreeProps

/react-aria-components:TreeItemContentRenderProps

@react-spectrum/s2

/@react-spectrum/s2:TreeView

/@react-spectrum/s2:TreeViewItem

/@react-spectrum/s2:TreeViewProps

/@react-spectrum/s2:TreeViewItemProps

LFDanLu left a comment

Choose a reason for hiding this comment

snowystinger commented Nov 8, 2024 •

edited

Loading