Skip to content

Commit 63c5490

Browse files
author
Elliot Park
authored
Merge pull request #2354 from looker-open-source/elliot/tree-nested-item-role-none
fix(Tree): Added explicit role="none" to nested TreeItem in Tree
2 parents bec7a69 + 6d1ef31 commit 63c5490

File tree

5 files changed

+11
-2
lines changed

5 files changed

+11
-2
lines changed

packages/components/src/List/ListItemLabel.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ type ListItemLabelProps = CompatibleHTMLProps<HTMLElement> &
8282
export const ListItemLabel = styled(ListItemLabelLayout).withConfig({
8383
shouldForwardProp: (prop) => prop === 'itemRole' || shouldForwardProp(prop),
8484
})`
85-
${({ height, itemRole }) => itemRole === 'none' && `height: ${height}px;`}
85+
${({ height, itemRole }) => itemRole === 'none' && `min-height: ${height}px;`}
8686
${listItemBackgroundColor}
8787
8888
align-items: center;

packages/components/src/Tree/Tree.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ const TreeLayout: FC<TreeProps> = ({
128128
},
129129
}
130130

131+
const treeItemInnerRole = 'none'
131132
const label = (
132133
<TreeItemInner
133134
color={color}
@@ -136,6 +137,8 @@ const TreeLayout: FC<TreeProps> = ({
136137
disabled={disabled}
137138
icon={icon}
138139
truncate={truncate}
140+
itemRole={treeItemInnerRole}
141+
role={treeItemInnerRole}
139142
>
140143
{propsLabel}
141144
</TreeItemInner>

packages/components/src/Tree/TreeItem.test.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ describe('TreeItem', () => {
8282
})
8383

8484
test('Triggers onClickWhitespace when indent padding is clicked, itemRole = "none", and labelBackgroundOnly = true', () => {
85+
// eslint-disable-next-line no-console
86+
console.warn = jest.fn()
87+
8588
const onClickWhitespace = jest.fn()
8689

8790
// This onClick should be ignored within ListItem since the item has itemRole="none"
@@ -106,9 +109,12 @@ describe('TreeItem', () => {
106109
</Tree>
107110
)
108111

112+
// eslint-disable-next-line no-console
113+
expect(console.warn).toHaveBeenCalled()
114+
109115
// Expect indent padding click to trigger TreeItem onClickWhitespace (i.e. non-TreeItem child click)
110116
// Note: This selector may need to change once Tree ARIA roles are implemented
111-
fireEvent.click(screen.getAllByRole('none')[1])
117+
fireEvent.click(screen.getAllByRole('none')[2])
112118
expect(onClickWhitespace).toHaveBeenCalledTimes(1)
113119

114120
// Do not expect click on label to trigger click handlers

snapshots/Tree/Density-snap.png

-1.76 KB
Loading
-180 Bytes
Loading

0 commit comments

Comments
 (0)