-
Notifications
You must be signed in to change notification settings - Fork 510
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
Add TreeView.LeadingAction
sub-component
#4546
Changes from 16 commits
a5ccd2d
66c876d
c982c1d
61b2e50
bba15da
1794aa5
5d54533
069b72a
dd39abb
f5e3dde
a8a3069
1f63a8b
f2442b7
920ea88
770e321
f9b880d
3771156
c8b1086
b438d27
29348d6
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,6 @@ | ||
--- | ||
"@primer/react": minor | ||
"docs": patch | ||
--- | ||
|
||
TreeView: Add support for `TreeView.LeadingAction` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
import {GrabberIcon} from '@primer/octicons-react' | ||
import type {Meta, Story} from '@storybook/react' | ||
import React from 'react' | ||
import Box from '../Box' | ||
import {TreeView} from './TreeView' | ||
import {IconButton} from '../Button' | ||
|
||
const meta: Meta = { | ||
title: 'Components/TreeView/Examples', | ||
component: TreeView, | ||
decorators: [ | ||
Story => { | ||
return ( | ||
// Prevent TreeView from expanding to the full width of the screen | ||
<Box sx={{maxWidth: 400}}> | ||
<Story /> | ||
</Box> | ||
) | ||
}, | ||
], | ||
} | ||
|
||
export const DraggableListItem: Story = () => { | ||
return ( | ||
<Box | ||
sx={{ | ||
// using Box for css, this could be in a css file as well | ||
'.treeview-item': { | ||
'.treeview-leading-action': {visibility: 'hidden'}, | ||
'&:hover, &:focus': { | ||
'.treeview-leading-action': {visibility: 'visible'}, | ||
}, | ||
}, | ||
}} | ||
> | ||
<TreeView aria-label="Issues"> | ||
<ControlledDraggableItem id="item-1">Item 1</ControlledDraggableItem> | ||
<ControlledDraggableItem id="item-2"> | ||
Item 2 | ||
<TreeView.SubTree> | ||
<TreeView.Item id="item-2-sub-task-1">sub task 1</TreeView.Item> | ||
<TreeView.Item id="item-2-sub-task-2">sub task 2</TreeView.Item> | ||
</TreeView.SubTree> | ||
</ControlledDraggableItem> | ||
<ControlledDraggableItem id="item-3">Item 3</ControlledDraggableItem> | ||
</TreeView> | ||
</Box> | ||
) | ||
} | ||
|
||
const ControlledDraggableItem: React.FC<{id: string; children: React.ReactNode}> = ({id, children}) => { | ||
const [expanded, setExpanded] = React.useState(false) | ||
|
||
return ( | ||
<> | ||
<TreeView.Item id={id} className="treeview-item" expanded={expanded} onExpandedChange={setExpanded}> | ||
<TreeView.LeadingAction> | ||
<IconButton | ||
icon={GrabberIcon} | ||
variant="invisible" | ||
aria-label="Reorder item" | ||
className="treeview-leading-action" | ||
draggable="true" | ||
onDragStart={() => { | ||
setExpanded(false) | ||
// other drag logic to follow | ||
}} | ||
/> | ||
</TreeView.LeadingAction> | ||
{children} | ||
</TreeView.Item> | ||
</> | ||
) | ||
} | ||
|
||
export default meta |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,6 +45,7 @@ const ItemContext = React.createContext<{ | |
setIsExpanded: (isExpanded: boolean) => void | ||
leadingVisualId: string | ||
trailingVisualId: string | ||
leadingActionId: string | ||
}>({ | ||
itemId: '', | ||
level: 1, | ||
|
@@ -54,6 +55,7 @@ const ItemContext = React.createContext<{ | |
setIsExpanded: () => {}, | ||
leadingVisualId: '', | ||
trailingVisualId: '', | ||
leadingActionId: '', | ||
}) | ||
|
||
// ---------------------------------------------------------------------------- | ||
|
@@ -97,15 +99,20 @@ const UlBox = styled.ul<SxProp>` | |
outline-offset: -2; | ||
} | ||
} | ||
&[data-has-leading-action] { | ||
--has-leading-action: 1; | ||
} | ||
} | ||
|
||
.PRIVATE_TreeView-item-container { | ||
--level: 1; /* default level */ | ||
--toggle-width: 1rem; /* 16px */ | ||
position: relative; | ||
display: grid; | ||
grid-template-columns: calc(calc(var(--level) - 1) * (var(--toggle-width) / 2)) var(--toggle-width) 1fr; | ||
grid-template-areas: 'spacer toggle content'; | ||
--leading-action-width: calc(var(--has-leading-action, 0) * 1.5rem); | ||
--spacer-width: calc(calc(var(--level) - 1) * (var(--toggle-width) / 2)); | ||
grid-template-columns: var(--spacer-width) var(--leading-action-width) var(--toggle-width) 1fr; | ||
grid-template-areas: 'spacer leadingAction toggle content'; | ||
width: 100%; | ||
min-height: 2rem; /* 32px */ | ||
font-size: ${get('fontSizes.1')}; | ||
|
@@ -138,7 +145,7 @@ const UlBox = styled.ul<SxProp>` | |
} | ||
|
||
&[data-omit-spacer='true'] .PRIVATE_TreeView-item-container { | ||
grid-template-columns: 0 0 1fr; | ||
grid-template-columns: 0 0 0 1fr; | ||
} | ||
|
||
.PRIVATE_TreeView-item[aria-current='true'] > .PRIVATE_TreeView-item-container { | ||
|
@@ -202,6 +209,12 @@ const UlBox = styled.ul<SxProp>` | |
color: ${get('colors.fg.muted')}; | ||
} | ||
|
||
.PRIVATE_TreeView-item-leading-action { | ||
display: flex; | ||
color: ${get('colors.fg.muted')}; | ||
grid-area: leadingAction; | ||
} | ||
|
||
.PRIVATE_TreeView-item-level-line { | ||
width: 100%; | ||
height: 100%; | ||
|
@@ -354,11 +367,16 @@ const Item = React.forwardRef<HTMLElement, TreeViewItemProps>( | |
}, | ||
ref, | ||
) => { | ||
const [slots, rest] = useSlots(children, {leadingVisual: LeadingVisual, trailingVisual: TrailingVisual}) | ||
const [slots, rest] = useSlots(children, { | ||
leadingAction: LeadingAction, | ||
leadingVisual: LeadingVisual, | ||
trailingVisual: TrailingVisual, | ||
}) | ||
const {expandedStateCache} = React.useContext(RootContext) | ||
const labelId = useId() | ||
const leadingVisualId = useId() | ||
const trailingVisualId = useId() | ||
const leadingActionId = useId() | ||
const [isExpanded, setIsExpanded] = useControllableState({ | ||
name: itemId, | ||
// If the item was previously mounted, it's expanded state might be cached. | ||
|
@@ -434,6 +452,7 @@ const Item = React.forwardRef<HTMLElement, TreeViewItemProps>( | |
setIsExpanded: setIsExpandedWithCache, | ||
leadingVisualId, | ||
trailingVisualId, | ||
leadingActionId, | ||
}} | ||
> | ||
{/* @ts-ignore Box doesn't have type support for `ref` used in combination with `as` */} | ||
|
@@ -444,11 +463,12 @@ const Item = React.forwardRef<HTMLElement, TreeViewItemProps>( | |
id={itemId} | ||
role="treeitem" | ||
aria-labelledby={labelId} | ||
aria-describedby={`${leadingVisualId} ${trailingVisualId}`} | ||
aria-describedby={`${leadingActionId} ${leadingVisualId} ${trailingVisualId}`} | ||
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. Don't think the action should be used to describe the item, especially when the action itself is not accessible (aria-hidden + tab-index=-1) 🤔 cc @ericwbailey for confirmation 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.
We'll likely want to include a hint down the line in the 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. @ayy-bc Final thing for you to look at :) 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. @siddharthkp I was just following what 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 can remove it or you can do so, whatever you prefer (lmk). 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.
Yes please, I think actions have to be treated differently than visuals
Makes sense, but be careful about keyboard navigation. For example, the toggle element (picture below) is not accessible by keyboard and hence does not have a label and is aria-hidden 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. @siddharthkp pushed the changes |
||
aria-level={level} | ||
aria-expanded={isSubTreeEmpty ? undefined : isExpanded} | ||
aria-current={isCurrentItem ? 'true' : undefined} | ||
aria-selected={isFocused ? 'true' : 'false'} | ||
data-has-leading-action={slots.leadingAction ? true : undefined} | ||
onKeyDown={handleKeyDown} | ||
onFocus={event => { | ||
// Scroll the first child into view when the item receives focus | ||
|
@@ -488,6 +508,7 @@ const Item = React.forwardRef<HTMLElement, TreeViewItemProps>( | |
<div style={{gridArea: 'spacer', display: 'flex'}}> | ||
<LevelIndicatorLines level={level} /> | ||
</div> | ||
{slots.leadingAction} | ||
{hasSubTree ? ( | ||
// This lint rule is disabled due to the guidelines in the `TreeView` api docs. | ||
// https://github.com/github/primer/blob/main/apis/tree-view-api.md#the-expandcollapse-chevron-toggle | ||
|
@@ -829,6 +850,25 @@ const TrailingVisual: React.FC<TreeViewVisualProps> = props => { | |
|
||
TrailingVisual.displayName = 'TreeView.TrailingVisual' | ||
|
||
// ---------------------------------------------------------------------------- | ||
// TreeView.LeadingAction | ||
|
||
const LeadingAction: React.FC<TreeViewVisualProps> = props => { | ||
const {isExpanded, leadingActionId} = React.useContext(ItemContext) | ||
const children = typeof props.children === 'function' ? props.children({isExpanded}) : props.children | ||
return ( | ||
<> | ||
<div className="PRIVATE_VisuallyHidden" aria-hidden={true} id={leadingActionId}> | ||
{props.label} | ||
</div> | ||
<div className="PRIVATE_TreeView-item-leading-action" aria-hidden={true}> | ||
{children} | ||
</div> | ||
</> | ||
) | ||
} | ||
|
||
LeadingAction.displayName = 'TreeView.LeadingAction' | ||
// ---------------------------------------------------------------------------- | ||
// TreeView.DirectoryIcon | ||
|
||
|
@@ -898,6 +938,7 @@ ErrorDialog.displayName = 'TreeView.ErrorDialog' | |
export const TreeView = Object.assign(Root, { | ||
Item, | ||
SubTree, | ||
LeadingAction, | ||
LeadingVisual, | ||
TrailingVisual, | ||
DirectoryIcon, | ||
|
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.
@ayy-bc Don't think we need this anymore either, removed it :)