Skip to content
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

feat: add keyboard navigation for resource tree #2591

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export const AttributionsList: React.FC<PackagesPanelChildrenProps> = ({
<List
renderItemContent={renderAttributionCard}
data={activeAttributionIds}
selected={selectedAttributionId}
selectedId={selectedAttributionId}
loading={loading}
sx={{ transition: TRANSITION, height: contentHeight }}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export const SignalsList: React.FC<PackagesPanelChildrenProps> = ({
return (
<GroupedList
grouped={groupedIds}
selected={selectedAttributionId}
selectedId={selectedAttributionId}
renderItemContent={renderAttributionCard}
renderGroupName={(sourceName) => (
<>
Expand Down
6 changes: 3 additions & 3 deletions src/Frontend/Components/GroupedList/GroupedList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export interface GroupedListProps {
datum: string,
props: GroupedListItemContentProps,
) => React.ReactNode;
selected?: string;
selectedId?: string;
sx?: SxProps;
testId?: string;
}
Expand All @@ -50,7 +50,7 @@ export function GroupedList({
loading,
renderGroupName,
renderItemContent,
selected,
selectedId,
sx,
testId,
...props
Expand Down Expand Up @@ -82,7 +82,7 @@ export function GroupedList({
selectedIndex,
} = useVirtuosoRefs<GroupedVirtuosoHandle>({
data: groups?.ids,
selected,
selectedId,
});

return (
Expand Down
6 changes: 3 additions & 3 deletions src/Frontend/Components/List/List.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export interface ListProps {
datum: string,
props: ListItemContentProps,
) => React.ReactNode;
selected?: string;
selectedId?: string;
sx?: SxProps;
testId?: string;
}
Expand All @@ -34,7 +34,7 @@ export function List({
data,
loading,
renderItemContent,
selected,
selectedId,
sx,
testId,
...props
Expand All @@ -47,7 +47,7 @@ export function List({
selectedIndex,
} = useVirtuosoRefs<VirtuosoHandle>({
data,
selected,
selectedId,
});

return (
Expand Down
5 changes: 3 additions & 2 deletions src/Frontend/Components/VirtualizedTree/VirtualizedTree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,19 @@ export function VirtualizedTree({
return (
<List
data={resourceIds.length ? Object.keys(treeNodes) : []}
renderItemContent={(nodeId, { selected }) => (
renderItemContent={(nodeId, { selected, focused }) => (
<VirtualizedTreeNode
TreeNodeLabel={TreeNodeLabel}
isExpandedNode={expandedIds.includes(nodeId)}
onToggle={onToggle}
onSelect={onSelect}
readOnly={readOnly}
selected={selected}
focused={focused}
{...treeNodes[nodeId]}
/>
)}
selected={selectedNodeId}
selectedId={selectedNodeId}
testId={testId}
sx={{
height: '100%',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import ChevronRightIcon from '@mui/icons-material/ChevronRight';
import ExpandMoreIcon from '@mui/icons-material/ExpandMore';
import MuiBox from '@mui/material/Box';
import { useEffect, useRef } from 'react';

import { Resources } from '../../../../shared/shared-types';
import { OpossumColors } from '../../../shared-styles';
Expand All @@ -26,6 +27,12 @@ const classes = {
'&:hover .tree-node-selected-indicator': {
display: 'block',
},
'&:focus .tree-node-selected-indicator': {
display: 'block',
},
'&:focus': {
outline: 'none',
},
},
clickableIcon: {
width: '16px',
Expand Down Expand Up @@ -74,6 +81,7 @@ interface VirtualizedTreeNodeProps extends TreeNode {
onToggle: (nodeIdsToExpand: Array<string>) => void;
readOnly?: boolean;
selected: boolean;
focused: boolean;
}

export function VirtualizedTreeNode({
Expand All @@ -86,6 +94,7 @@ export function VirtualizedTreeNode({
onToggle,
readOnly,
selected,
focused,
}: VirtualizedTreeNodeProps) {
const isExpandable = node !== 1 && Object.keys(node).length !== 0;
const marginRight =
Expand All @@ -96,6 +105,14 @@ export function VirtualizedTreeNode({
? SIMPLE_FOLDER_EXTRA_INDENT
: SIMPLE_NODE_EXTRA_INDENT);

const ref = useRef<HTMLDivElement>(null);

useEffect(() => {
if (focused) {
ref.current?.focus();
}
}, [focused]);

const handleClick = readOnly
? undefined
: () => {
Expand All @@ -106,7 +123,24 @@ export function VirtualizedTreeNode({
};

return (
<MuiBox sx={classes.listNode} onClick={handleClick}>
<MuiBox
sx={classes.listNode}
onClick={handleClick}
tabIndex={0}
ref={ref}
onKeyDown={(event) => {
if (['Enter', 'Space'].includes(event.code)) {
event.preventDefault();
handleClick?.();
} else if (event.code === 'ArrowRight' && !isExpandedNode) {
event.preventDefault();
onToggle?.([nodeId]);
} else if (event.code === 'ArrowLeft' && isExpandedNode) {
event.preventDefault();
onToggle?.([nodeId]);
}
}}
>
<MuiBox sx={classes.treeNodeSpacer} style={{ width: marginRight }} />
{renderExpandableNodeIcon()}
<MuiBox
Expand Down
28 changes: 18 additions & 10 deletions src/Frontend/util/use-virtuoso-refs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,33 +8,41 @@ import { VirtuosoHandle } from 'react-virtuoso';

export function useVirtuosoRefs<T extends VirtuosoHandle>({
data,
selected,
selectedId,
}: {
data: ReadonlyArray<string> | null | undefined;
selected: string | undefined;
selectedId: string | undefined;
}) {
const ref = useRef<T>(null);
const listRef = useRef<Window | HTMLElement>();
const [isVirtuosoFocused, setIsVirtuosoFocused] = useState(false);
const [focusedIndex, setFocusedIndex] = useState<number>();
const [focusedId, setFocusedId] = useState<string>();

const selectedIndex = useMemo(() => {
if (!data) {
return undefined;
}

return data.findIndex((datum) => datum === selected);
}, [data, selected]);
return data.findIndex((datum) => datum === selectedId);
}, [data, selectedId]);

const focusedIndex = useMemo(() => {
if (!data) {
return undefined;
}

return data.findIndex((datum) => datum === focusedId);
}, [data, focusedId]);

useEffect(() => {
if (isVirtuosoFocused) {
setFocusedIndex(selectedIndex);
setFocusedId(selectedId);
}

return () => {
setFocusedIndex(undefined);
setFocusedId(undefined);
};
}, [isVirtuosoFocused, selectedIndex]);
}, [isVirtuosoFocused, selectedId]);

useEffect(() => {
if (selectedIndex !== undefined && selectedIndex >= 0) {
Expand Down Expand Up @@ -68,12 +76,12 @@ export function useVirtuosoRefs<T extends VirtuosoHandle>({
index,
behavior: 'auto',
});
setFocusedIndex(index);
setFocusedId(data[index]);
event.preventDefault();
}
}
},
[data?.length, focusedIndex],
[data, focusedIndex],
);

const scrollerRef = useCallback(
Expand Down
85 changes: 85 additions & 0 deletions src/e2e-tests/__tests__/selecting-resources.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// SPDX-FileCopyrightText: Meta Platforms, Inc. and its affiliates
// SPDX-FileCopyrightText: TNG Technology Consulting GmbH <https://www.tngtech.com>
//
// SPDX-License-Identifier: Apache-2.0
import { faker, test } from '../utils';

const [resourceName1, resourceName2, resourceName3] =
faker.opossum.resourceNames({ count: 3 });
const [attributionId1, packageInfo1] = faker.opossum.rawAttribution({
packageName: 'a',
});
const [attributionId2, packageInfo2] = faker.opossum.rawAttribution({
packageName: 'b',
});
const [attributionId3, packageInfo3] = faker.opossum.rawAttribution({
packageName: 'c',
});

test.use({
data: {
inputData: faker.opossum.inputData({
resources: faker.opossum.resources({
[resourceName1]: {
[resourceName2]: 1,
},
[resourceName3]: 1,
}),
}),
outputData: faker.opossum.outputData({
manualAttributions: faker.opossum.rawAttributions({
[attributionId1]: packageInfo1,
[attributionId2]: packageInfo2,
[attributionId3]: packageInfo3,
}),
resourcesToAttributions: faker.opossum.resourcesToAttributions({
[faker.opossum.folderPath(resourceName1)]: [attributionId1],
[faker.opossum.filePath(resourceName2)]: [attributionId1],
[faker.opossum.filePath(resourceName3)]: [attributionId3],
}),
}),
},
});

test('allows navigating up and down the resource tree by keyboard', async ({
resourcesTree,
attributionDetails,
window,
}) => {
await resourcesTree.goto(resourceName1);
await resourcesTree.goto(resourceName2);
await attributionDetails.attributionForm.assert.matchesPackageInfo(
packageInfo1,
);

await window.keyboard.press('ArrowDown');
await window.keyboard.press('Enter');
await attributionDetails.attributionForm.assert.matchesPackageInfo(
packageInfo3,
);

await window.keyboard.press('ArrowUp');
await window.keyboard.press('ArrowUp');
await window.keyboard.press('Space');
await attributionDetails.attributionForm.assert.matchesPackageInfo(
packageInfo1,
);
});

test('allows expanding and collapsing folders in the resource tree by keyboard', async ({
resourcesTree,
window,
}) => {
await resourcesTree.goto(resourceName3);
await window.keyboard.press('ArrowUp');
await resourcesTree.assert.resourceIsHidden(resourceName2);

await window.keyboard.press('ArrowRight');
await resourcesTree.assert.resourceIsVisible(resourceName2);

await window.keyboard.press('ArrowLeft');
await resourcesTree.assert.resourceIsHidden(resourceName2);

await window.keyboard.press('Enter');
await resourcesTree.assert.resourceIsVisible(resourceName2);
});
Loading