Skip to content

Commit 3346b09

Browse files
assuncaocharleslayershifter
assuncaocharles
and
layershifter
authored
fix(Tree): Match design spec to show/hide selectable indicator (#15133)
* Revert "fix(Tree): always show selectable indicator (#14916)" This reverts commit d6ed67d. * fix(Tree): contition to show selectable indicator matching the design spec * fix(Tree): always show indicator in expanded selectable parent * fix(TreeTitle): condition to show checkbox * Update Tree.tsx * fix(Tree): set all parents as selectable * fix(Tree): set all parents as selectable * fix(Tree): set all parents as selectable * fix(Tree): add basic example * fix(Tree): add event * fix(Tree): remove expanded condition * fix(Tree): all parent in a selectable tree should be selectable * fix(Tree): add changelg * fix(Tree): move hover to styles * fix(Tree): remove custom selection indicator * Update packages/fluentui/react-northstar/src/themes/teams/components/Tree/treeTitleStyles.ts Co-authored-by: Oleksandr Fediashov <[email protected]> * Update packages/fluentui/react-northstar/src/components/Tree/TreeItem.tsx * Update packages/fluentui/react-northstar/src/components/Tree/TreeItem.tsx * Update packages/fluentui/react-northstar/src/components/Tree/TreeItem.tsx * Update Site.module.scss * Update settings.json * Update settings.json * fix(Tree): add steps for basic selectable tree: Co-authored-by: Oleksandr Fediashov <[email protected]>
1 parent ad0cdab commit 3346b09

File tree

12 files changed

+216
-82
lines changed

12 files changed

+216
-82
lines changed

‎packages/fluentui/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
2323
- Close `Menu` at page scroll @assuncaocharles ([#14755](https://github.com/microsoft/fluentui/pull/14755))
2424
- Rename `indicatorBorderColorDefault` variable to `indicatorColorDefault` @assuncaocharles ([#14895](https://github.com/microsoft/fluentui/pull/14895))
2525
- Remove the `disabled` property from `Dialog` and `Popup` behaviors @rymeskar ([#14885](https://github.com/microsoft/fluentui/pull/14885))
26+
- Remove `selectableParent` prop in favor of `selectable` in `TreeItem` and make `selectionIndicator` visible only on focus or hover @assuncaocharles ([#15133](https://github.com/microsoft/fluentui/pull/15133))
2627

2728
### Fixes
2829
- Fix `treeAsListBehavior` to support multi-select `Tree` @yuanboxue-amber ([#15147](https://github.com/microsoft/fluentui/pull/15147))

‎packages/fluentui/docs/src/examples/components/Tree/Performance/TreeWith60ListItems.perf.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ const titleRenderer: ShorthandRenderFunction<TreeTitleProps & {
6161
index?: number;
6262
}> = (Component, { content, header, headerMedia, media, index, ...restProps }) => {
6363
// as providing all props to List.Item was showing console errors, therefore reducing props
64-
const { treeSize, expanded, hasSubtree, selectableParent, selectionIndicator, ...restReducedProps } = restProps;
64+
const { treeSize, expanded, hasSubtree, selectionIndicator, ...restReducedProps } = restProps;
6565
return header ? (
6666
<List.Item
6767
{...restReducedProps}

‎packages/fluentui/docs/src/examples/components/Tree/Usage/TreeMultiselectBasicExample.shorthand.steps.ts

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { treeItemClassName, treeTitleClassName, treeTitleSlotClassNames } from '@fluentui/react-northstar';
2+
import { ScreenerTestsConfig } from '@uifabric/build/screener';
3+
4+
const selectors = {
5+
treeTitle: (itemIndex: number) => `.${treeItemClassName}:nth-of-type(${itemIndex}) .${treeTitleClassName}`,
6+
treeItem: (itemIndex: number) => `.${treeItemClassName}:nth-of-type(${itemIndex})`,
7+
selectionIndicator: (itemIndex: number) =>
8+
`.${treeItemClassName}:nth-of-type(${itemIndex}) .${treeTitleSlotClassNames.indicator}`,
9+
};
10+
11+
const config: ScreenerTestsConfig = {
12+
themes: ['teams', 'teamsDark', 'teamsHighContrast'],
13+
steps: [
14+
(builder, keys) =>
15+
builder
16+
.hover(selectors.treeTitle(1))
17+
.snapshot('hover first, indicator visible for first item')
18+
.hover(selectors.treeTitle(2))
19+
.snapshot('hover second, indicator visible for second item')
20+
.click(selectors.treeTitle(2))
21+
.snapshot('second expanded, indicator always visible')
22+
.click(selectors.selectionIndicator(2))
23+
.snapshot('second group all selected')
24+
.click(selectors.treeTitle(2))
25+
.snapshot('second collapsed, indicator visible when has children item selected')
26+
.click(selectors.treeTitle(1))
27+
.click(selectors.treeTitle(2))
28+
.click(selectors.selectionIndicator(3))
29+
.snapshot('first group expanded and partially selected')
30+
.click(selectors.treeTitle(1))
31+
.snapshot('first group collapsed and partially selected'),
32+
],
33+
};
34+
35+
export default config;

‎packages/fluentui/docs/src/examples/components/Tree/Usage/TreeMultiselectBasicExample.shorthand.tsx

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import * as React from 'react';
2+
import { Tree } from '@fluentui/react-northstar';
3+
4+
const items = [
5+
{
6+
id: 'tree-item-1',
7+
title: 'House Lannister',
8+
items: [
9+
{
10+
id: 'tree-item-11',
11+
title: 'Tywin',
12+
items: [
13+
{
14+
id: '1',
15+
title: 'Jaime',
16+
items: [
17+
{
18+
id: '2',
19+
title: 'Jaime 2',
20+
},
21+
{
22+
id: '3',
23+
title: 'Jaime 3',
24+
},
25+
],
26+
},
27+
{
28+
id: '4',
29+
title: 'Cersei',
30+
},
31+
{
32+
id: '5',
33+
title: 'Tyrion',
34+
},
35+
],
36+
},
37+
{
38+
id: 'tree-item-12',
39+
title: 'Kevan',
40+
items: [
41+
{
42+
id: 'tree-item-121',
43+
title: 'Lancel',
44+
},
45+
{
46+
id: 'tree-item-122',
47+
title: 'Willem',
48+
},
49+
{
50+
id: 'tree-item-123',
51+
title: 'Martyn',
52+
},
53+
],
54+
},
55+
],
56+
},
57+
{
58+
id: 'tree-item-2',
59+
title: 'House Targaryen',
60+
items: [
61+
{
62+
id: 'tree-item-21',
63+
title: 'Aerys',
64+
items: [
65+
{
66+
id: 'tree-item-211',
67+
title: 'Rhaegar',
68+
},
69+
{
70+
id: 'tree-item-212',
71+
title: 'Viserys',
72+
},
73+
],
74+
},
75+
],
76+
},
77+
];
78+
79+
const TreeMultiselectExample = () => <Tree selectable aria-label="default" items={items} />;
80+
81+
export default TreeMultiselectExample;

‎packages/fluentui/docs/src/examples/components/Tree/Usage/TreeMultiselectExample.shorthand.tsx

+1-24
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as React from 'react';
2-
import { Tree, Text } from '@fluentui/react-northstar';
2+
import { Tree } from '@fluentui/react-northstar';
33

44
const items = [
55
{
@@ -9,7 +9,6 @@ const items = [
99
{
1010
id: 'tree-item-11',
1111
title: 'Tywin',
12-
selectableParent: true,
1312
items: [
1413
{
1514
id: '1',
@@ -28,21 +27,6 @@ const items = [
2827
{
2928
id: '4',
3029
title: 'Cersei',
31-
selectionIndicator: {
32-
children: (Component, { selected, onClick, ...props }) => {
33-
return (
34-
<Component {...props}>
35-
<input
36-
data-is-focusable={false}
37-
type="checkbox"
38-
checked={selected}
39-
onClick={onClick}
40-
onChange={() => {}}
41-
/>
42-
</Component>
43-
);
44-
},
45-
},
4630
},
4731
{
4832
id: '5',
@@ -73,12 +57,6 @@ const items = [
7357
{
7458
id: 'tree-item-2',
7559
title: 'House Targaryen',
76-
selectionIndicator: {
77-
children: (Component, { expanded, selected, ...props }) => {
78-
return <Text {...props} content={expanded && (selected ? 'unselect all' : 'select all')} />;
79-
},
80-
},
81-
selectableParent: true,
8260
items: [
8361
{
8462
id: 'tree-item-21',
@@ -104,7 +82,6 @@ const items = [
10482
{
10583
id: '100',
10684
title: 'House Skywalker',
107-
selectableParent: true,
10885
items: [
10986
{
11087
id: '102',

‎packages/fluentui/docs/src/examples/components/Tree/Usage/index.tsx

+5
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ const Usage = () => (
2020
description="A Tree with list accessibility roles."
2121
examplePath="components/Tree/Usage/TreeAsListExample"
2222
/>
23+
<ComponentExample
24+
title="Basic Multi Select"
25+
description="A basic multiselect Tree."
26+
examplePath="components/Tree/Usage/TreeMultiselectBasicExample"
27+
/>
2328
<ComponentExample
2429
title="Multi select"
2530
description="A multiselect Tree."

‎packages/fluentui/react-northstar/src/components/Tree/Tree.tsx

+5-6
Original file line numberDiff line numberDiff line change
@@ -312,9 +312,9 @@ export const Tree: ComponentWithAs<'div', TreeProps> &
312312
expandItems(e, treeItemProps);
313313
}
314314

315-
if (treeItemProps.selectable) {
315+
if (selectable) {
316316
// parent must be selectable and expanded in order to procced with selection, otherwise return
317-
if (treeItemHasSubtree && !(treeItemProps.selectableParent && treeItemProps.expanded)) {
317+
if (treeItemHasSubtree && !treeItemProps.selectable) {
318318
return;
319319
}
320320

@@ -326,7 +326,7 @@ export const Tree: ComponentWithAs<'div', TreeProps> &
326326
setSelectedItemIds(e, currSelectedItemIds => processItemsForSelection(treeItemProps, currSelectedItemIds));
327327
}
328328
},
329-
[expandItems, setSelectedItemIds],
329+
[expandItems, selectable, setSelectedItemIds],
330330
);
331331

332332
const onFocusFirstChild = React.useCallback(
@@ -383,7 +383,7 @@ export const Tree: ComponentWithAs<'div', TreeProps> &
383383
);
384384

385385
const isIndeterminate = (item: TreeItemProps) => {
386-
if (!item.selectableParent || !item.items) {
386+
if (!selectable || !hasSubtree(item)) {
387387
return false;
388388
}
389389

@@ -396,7 +396,7 @@ export const Tree: ComponentWithAs<'div', TreeProps> &
396396
};
397397

398398
const isSelectedItem = (item: TreeItemProps): boolean => {
399-
if (item.selectableParent && item.items) {
399+
if (selectable && hasSubtree(item)) {
400400
return isAllGroupChecked(item.items as TreeItemProps[], selectedItemIds);
401401
}
402402

@@ -434,7 +434,6 @@ export const Tree: ComponentWithAs<'div', TreeProps> &
434434
expanded: isSubtreeExpanded,
435435
selected: isSelectedItem(item),
436436
selectable,
437-
...(isSubtree && !item.selectableParent && { selectable: false }),
438437
renderItemTitle,
439438
id,
440439
key: id,

‎packages/fluentui/react-northstar/src/components/Tree/TreeItem.tsx

+11-4
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,9 @@ export interface TreeItemProps extends UIComponentProps, ChildrenComponentProps
110110
indeterminate?: boolean;
111111
}
112112

113-
export type TreeItemStylesProps = Required<Pick<TreeItemProps, 'level'>>;
113+
export type TreeItemStylesProps = Required<Pick<TreeItemProps, 'level'>> & {
114+
selectable?: boolean;
115+
};
114116
export const treeItemClassName = 'ui-tree__item';
115117

116118
/**
@@ -139,14 +141,14 @@ export const TreeItem: ComponentWithAs<'div', TreeItemProps> & FluentComponentSt
139141
variables,
140142
treeSize,
141143
selectionIndicator,
142-
selectableParent,
143144
selected,
144145
selectable,
145146
indeterminate,
146147
id,
147148
parent,
148149
} = props;
149150

151+
const selectableParent = hasSubtree && selectable;
150152
const hasSubtreeItem = hasSubtree(props);
151153

152154
const { onFocusParent, onSiblingsExpand, onFocusFirstChild, onTitleClick } = React.useContext(TreeContext);
@@ -210,10 +212,12 @@ export const TreeItem: ComponentWithAs<'div', TreeItemProps> & FluentComponentSt
210212
}),
211213
rtl: context.rtl,
212214
});
215+
213216
const { classes } = useStyles<TreeItemStylesProps>(TreeItem.displayName, {
214217
className: treeItemClassName,
215218
mapPropsToStyles: () => ({
216219
level,
220+
selectable,
217221
}),
218222
mapPropsToInlineStyles: () => ({ className, design, styles, variables }),
219223
rtl: context.rtl,
@@ -232,14 +236,17 @@ export const TreeItem: ComponentWithAs<'div', TreeItemProps> & FluentComponentSt
232236
_.invoke(props, 'onFocusFirstChild', e, props);
233237
onFocusFirstChild(props.id);
234238
};
239+
235240
const handleFocusParent = e => {
236241
_.invoke(props, 'onFocusParent', e, props);
237242
onFocusParent(parent);
238243
};
244+
239245
const handleSiblingsExpand = e => {
240246
_.invoke(props, 'onSiblingsExpand', e, props);
241247
onSiblingsExpand(e, props);
242248
};
249+
243250
const handleTitleOverrides = (predefinedProps: TreeTitleProps) => ({
244251
onClick: (e, titleProps) => {
245252
handleTitleClick(e);
@@ -283,9 +290,7 @@ export const TreeItem: ComponentWithAs<'div', TreeItemProps> & FluentComponentSt
283290
selected,
284291
selectable,
285292
parent,
286-
...(hasSubtreeItem && !selectableParent && { selectable: false }),
287293
...(selectableParent && { indeterminate }),
288-
selectableParent,
289294
selectionIndicator,
290295
}),
291296
render: renderItemTitle,
@@ -326,9 +331,11 @@ TreeItem.propTypes = {
326331
selectableParent: PropTypes.bool,
327332
indeterminate: PropTypes.bool,
328333
};
334+
329335
TreeItem.defaultProps = {
330336
accessibility: treeItemBehavior,
331337
};
338+
332339
TreeItem.handledProps = Object.keys(TreeItem.propTypes) as any;
333340

334341
TreeItem.create = createShorthandFactory({

0 commit comments

Comments
 (0)