-
Notifications
You must be signed in to change notification settings - Fork 645
TreeView: Support async behavior #2388
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
31d1248
eef409c
b948a2e
ee26ce8
cf81070
1da8a2f
bb27f05
a1afd9f
948dd2a
172987e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@primer/react": patch | ||
| --- | ||
|
|
||
| TreeView: Add `TreeView.LoadingItem` component |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,8 +8,9 @@ import {useSSRSafeId} from '@react-aria/ssr' | |
| import React from 'react' | ||
| import styled from 'styled-components' | ||
| import Box from '../Box' | ||
| import StyledOcticon from '../StyledOcticon' | ||
| import {useControllableState} from '../hooks/useControllableState' | ||
| import Spinner from '../Spinner' | ||
| import StyledOcticon from '../StyledOcticon' | ||
| import sx, {SxProp} from '../sx' | ||
| import Text from '../Text' | ||
| import {Theme} from '../ThemeProvider' | ||
|
|
@@ -282,7 +283,6 @@ const Item: React.FC<TreeViewItemProps> = ({ | |
| > | ||
| <Slots> | ||
| {slots => ( | ||
| // QUESTION: How should leading and trailing visuals impact the aria-label? | ||
| <> | ||
| {slots.LeadingVisual} | ||
| <Text | ||
|
|
@@ -362,6 +362,23 @@ const LinkItem: React.FC<TreeViewLinkItemProps> = ({href, onSelect, ...props}) = | |
| ) | ||
| } | ||
|
|
||
| // ---------------------------------------------------------------------------- | ||
| // TreeView.LoadingItem | ||
|
|
||
| const LoadingItem: React.FC = () => { | ||
| return ( | ||
| // TODO: What aria attributes do we need to add here? | ||
| <Item> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🙋 What aria props should we add to the LoadingItem? cc @ericwbailey
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm in the process of testing this out, but I believe we'll need a combination of |
||
| <LeadingVisual> | ||
| <Spinner size="small" /> | ||
| </LeadingVisual> | ||
| <Text sx={{color: 'fg.muted'}}>Loading...</Text> | ||
| </Item> | ||
| ) | ||
| } | ||
|
|
||
| LoadingItem.displayName = 'TreeView.LoadingItem' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this. I'm going to start doing this instead of keeping the name of the |
||
|
|
||
| // ---------------------------------------------------------------------------- | ||
| // TreeView.SubTree | ||
|
|
||
|
|
@@ -417,7 +434,7 @@ const LeadingVisual: React.FC<TreeViewVisualProps> = props => { | |
| const children = typeof props.children === 'function' ? props.children({isExpanded}) : props.children | ||
| return ( | ||
| <Slot name="LeadingVisual"> | ||
| <Box sx={{color: 'fg.muted'}}>{children}</Box> | ||
| <Box sx={{display: 'flex', color: 'fg.muted'}}>{children}</Box> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do we need flexbox for? Is it just a vertical alignment thing? Or are we actually laying out multiple children?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When you render an SVG (Octicon) in the leading visual slot, it adds unwanted extra space unless the container is a flex container.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I figured it was something like that. I've dealt with that before, and I don't understand why it works 🙈 I think it also works for |
||
| </Slot> | ||
| ) | ||
| } | ||
|
|
@@ -427,7 +444,7 @@ const TrailingVisual: React.FC<TreeViewVisualProps> = props => { | |
| const children = typeof props.children === 'function' ? props.children({isExpanded}) : props.children | ||
| return ( | ||
| <Slot name="TrailingVisual"> | ||
| <Box sx={{color: 'fg.muted'}}>{children}</Box> | ||
| <Box sx={{display: 'flex', color: 'fg.muted'}}>{children}</Box> | ||
| </Slot> | ||
| ) | ||
| } | ||
|
|
@@ -448,6 +465,7 @@ const DirectoryIcon = () => { | |
| export const TreeView = Object.assign(Root, { | ||
| Item, | ||
| LinkItem, | ||
| LoadingItem, | ||
| SubTree, | ||
| LeadingVisual, | ||
| TrailingVisual, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙋 Is ConfirmationDialog the right kind of dialog for error messages? cc @ericwbailey
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so as well.