Skip to content

Commit 4f3b73e

Browse files
committed
feat: call onFocusItem with first item if focused item is missing (#363)
1 parent 39572cb commit 4f3b73e

File tree

4 files changed

+80
-28
lines changed

4 files changed

+80
-28
lines changed

next-release-notes.md

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,4 @@
1-
<!--
2-
### Breaking Changes
3-
4-
### Features
5-
61
### Bug Fixes and Improvements
7-
8-
### Other Changes
9-
-->
2+
- If a tree environment renders without an item defined as focused in its `viewState` parameter, it will invoke the `onFocusItem`
3+
prop with the first item in the tree during its render. In the past, this was implicitly and silently set in the `viewState` prop,
4+
now this assignment is triggered explicitly with the handler call (#363)

packages/core/src/controlledEnvironment/ControlledTreeEnvironment.tsx

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as React from 'react';
2-
import { useContext } from 'react';
2+
import { useContext, useEffect } from 'react';
33
import {
44
ControlledTreeEnvironmentProps,
55
TreeEnvironmentContextProps,
@@ -21,26 +21,24 @@ export const ControlledTreeEnvironment = React.forwardRef<
2121
>((props, ref) => {
2222
const environmentContextProps = useControlledTreeEnvironmentProps(props);
2323

24-
const { viewState } = props;
24+
const { viewState, onFocusItem } = props;
2525

2626
// Make sure that every tree view state has a focused item
27-
for (const treeId of Object.keys(environmentContextProps.trees)) {
28-
// TODO if the focus item is dragged out of the tree and is not within the expanded items
29-
// TODO of that tree, the tree does not show any focus item anymore.
30-
// Fix: use linear items to see if focus item is visible, and reset if not. Only refresh that
31-
// information when the viewstate changes
32-
if (
33-
!viewState[treeId]?.focusedItem &&
34-
environmentContextProps.trees[treeId]
35-
) {
36-
viewState[treeId] = {
37-
...viewState[treeId],
38-
focusedItem:
39-
props.items[environmentContextProps.trees[treeId].rootItem]
40-
?.children?.[0],
41-
};
27+
useEffect(() => {
28+
for (const treeId of Object.keys(environmentContextProps.trees)) {
29+
const firstItemIndex =
30+
props.items[environmentContextProps.trees[treeId].rootItem]
31+
?.children?.[0];
32+
const firstItem = firstItemIndex && props.items[firstItemIndex];
33+
if (
34+
!viewState[treeId]?.focusedItem &&
35+
environmentContextProps.trees[treeId] &&
36+
firstItem
37+
) {
38+
onFocusItem?.(firstItem, treeId, false);
39+
}
4240
}
43-
}
41+
}, [environmentContextProps.trees, onFocusItem, props.items, viewState]);
4442

4543
return (
4644
<TreeEnvironmentContext.Provider value={environmentContextProps}>
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import { Meta } from '@storybook/react';
2+
import React, { useState } from 'react';
3+
import { longTree } from 'demodata';
4+
import { Tree } from '../tree/Tree';
5+
import { ControlledTreeEnvironment } from '../controlledEnvironment/ControlledTreeEnvironment';
6+
import { TreeItemIndex } from '../types';
7+
8+
export default {
9+
title: 'Core/Issue Report Reproduction',
10+
} as Meta;
11+
12+
export const Issue363 = () => {
13+
const [focusedItem, setFocusedItem] = useState<TreeItemIndex>();
14+
const [expandedItems, setExpandedItems] = useState<TreeItemIndex[]>([]);
15+
const [selectedItems, setSelectedItems] = useState<TreeItemIndex[]>([]);
16+
return (
17+
<ControlledTreeEnvironment<string>
18+
canDragAndDrop
19+
canDropOnFolder
20+
canReorderItems
21+
items={longTree.items}
22+
getItemTitle={item => item.data}
23+
onFocusItem={item => {
24+
setFocusedItem(item.index);
25+
console.log(`Focused item: ${item.index}`);
26+
}}
27+
onSelectItems={items => {
28+
setSelectedItems(items);
29+
}}
30+
onExpandItem={item => {
31+
setExpandedItems([...expandedItems, item.index]);
32+
}}
33+
onCollapseItem={item => {
34+
setExpandedItems(expandedItems.filter(i => i !== item.index));
35+
}}
36+
viewState={{
37+
'tree-1': {
38+
focusedItem,
39+
expandedItems,
40+
selectedItems,
41+
},
42+
}}
43+
>
44+
<button type="button">Focusable element</button>
45+
<Tree treeId="tree-1" rootItem="root" treeLabel="Tree Example" />
46+
<pre>
47+
{JSON.stringify(
48+
{
49+
focusedItem,
50+
expandedItems,
51+
selectedItems,
52+
},
53+
null,
54+
2
55+
)}
56+
</pre>
57+
</ControlledTreeEnvironment>
58+
);
59+
};

packages/core/test/dnd-restrictions.spec.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ describe('dnd restrictions', () => {
118118

119119
describe('canDrag', () => {
120120
it('respects disabled value', async () => {
121-
const canDrag = jest.fn(items => items[0].data !== 'aab');
121+
const canDrag = jest.fn(items => items[0]?.data !== 'aab');
122122
const test = await new TestUtil().renderOpenTree({
123123
canDrag,
124124
});
@@ -131,7 +131,7 @@ describe('dnd restrictions', () => {
131131
});
132132

133133
it('works for other item', async () => {
134-
const canDrag = jest.fn(items => items[0].data !== 'aab');
134+
const canDrag = jest.fn(items => items[0]?.data !== 'aab');
135135
const test = await new TestUtil().renderOpenTree({
136136
canDrag,
137137
});

0 commit comments

Comments
 (0)