feat: flat Tree component by silviuaavram · Pull Request #1779 · microsoft/fluent-ui-react

silviuaavram · 2019-08-09T10:11:16Z

Since now this tree may be virtualized, subtree open/closed state should be kept only on the main Tree, as other Items/Titles/Trees can be removed during the virtualisation process. Also removed sub-trees altogether and kept only the first Tree.

The Tree renders TreeItems which in turn render TreeTitles. open/close logic is kept on the main Tree state in a Map. Each item has an id (map key) and we keep its ref and open/close state as map object value.

At Tree constructor time, all items will receive props that should not be changed by the user, as they are used in aria attribute generation, keyboard navigation and collapsing/expansion. These are: level, siblings, index, parent. Each item will receive these props.

Each item will pass level, treeSize, index to the TreeTitle (for aria attribute generation in the behavior).

id is now required from the user for reach tree item.

docs/src/examples/components/Tree/Types/TreeTitleCustomizationExample.shorthand.tsx

docs/src/examples/components/Tree/Types/TreeExclusiveExample.shorthand.tsx

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

packages/react/src/lib/accessibility/Behaviors/Tree/treeBehavior.ts

packages/react/src/themes/teams-dark/components/Tree/treeTitleVariables.ts

layershifter · 2019-08-29T12:18:05Z

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

+
+  static getDerivedStateFromProps(props: TreeItemProps) {
+    return {
+      hasSubtree: Tree.hasSubtree(props),


ERROR in packages/react/dist/es/components/Tree/Tree.js -> packages/react/dist/es/components/Tree/TreeItem.js -> packages/react/dist/es/components/Tree/Tree.js

This create a circular reference, I suggest to create a simple function as there are no context requirements and move it to Tree/lib/hasSubtree.ts:

Suggested change

hasSubtree: Tree.hasSubtree(props),

hasSubtree: hasSubtree(props),

layershifter · 2019-08-29T12:22:46Z

packages/react/src/themes/teams/components/Tree/treeTitleVariables.ts

+
+export default (siteVars: any): TreeTitleVariables => {
+  return {
+    defaultColor: siteVars.colors.grey[750],


https://stardust-ui.github.io/react/color-schemes
This color perfectly maps foreground from default, can we use it? And we will be to remove variable overrides in Dark and HC themes.

I also suggest to rename it to color to match our variable naming convention:

Suggested change

defaultColor: siteVars.colors.grey[750],

color: siteVars.colorScheme.default.foreground,

layershifter · 2019-08-29T12:23:07Z

packages/react/src/themes/teams-high-contrast/components/Tree/treeTitleVariables.ts

@@ -0,0 +1,7 @@
+import { TreeTitleVariables } from '../../../teams/components/Tree/treeTitleVariables'


Can be removed:
https://github.com/stardust-ui/react/pull/1779/files#r319038491

layershifter · 2019-08-29T12:23:15Z

packages/react/src/themes/teams-dark/components/Tree/treeTitleVariables.ts

+  return {
+    defaultColor: siteVars.colors.white,
+  }
+}


Can be removed:
https://github.com/stardust-ui/react/pull/1779/files#r319038491

layershifter · 2019-08-29T12:24:58Z

packages/react/src/themes/teams/components/Tree/treeTitleStyles.ts

+
+const treeTitleStyles = {
+  root: ({ variables, theme: { siteVariables } }): ICSSInJSStyle => ({
+    padding: `${pxToRem(1)} 0`,


Can we move this to variables?

Suggested change

padding: `${pxToRem(1)} 0`,

padding: v.padding,

layershifter · 2019-08-29T12:25:15Z

packages/react/src/themes/teams/components/Tree/treeTitleStyles.ts

+  root: ({ variables, theme: { siteVariables } }): ICSSInJSStyle => ({
+    padding: `${pxToRem(1)} 0`,
+    cursor: 'pointer',
+    color: variables.defaultColor,


Suggested change

color: variables.defaultColor,

color: v.color,

https://github.com/stardust-ui/react/pull/1779/files#r319038491

layershifter · 2019-08-29T12:25:33Z

packages/react/src/themes/teams/components/Tree/treeTitleStyles.ts

+import getBorderFocusStyles from '../../getBorderFocusStyles'
+
+const treeTitleStyles = {
+  root: ({ variables, theme: { siteVariables } }): ICSSInJSStyle => ({


Suggested change

root: ({ variables, theme: { siteVariables } }): ICSSInJSStyle => ({

root: ({ variables: v, theme: { siteVariables } }): ICSSInJSStyle => ({

Let's use v.* naming

layershifter · 2019-08-29T12:25:50Z

packages/react/src/themes/teams/components/Tree/treeTitleVariables.ts

@@ -0,0 +1,9 @@
+export interface TreeTitleVariables {
+  defaultColor: string


Suggested change

defaultColor: string

color: string

https://github.com/stardust-ui/react/pull/1779/files#r319038491

layershifter · 2019-08-29T12:26:22Z

packages/react/test/specs/behaviors/testDefinitions.ts

  testMethod: (parameters: TestMethod) => {
    const [action, key, elementToPerformAction] = [...parameters.props]
-    const propertyOpenedSubtree = { open: true, items: [{ a: 1 }] }
+    const propertyOpenedSubtree = { open: true, items: [{ a: 1 }], siblings: [], hasSubtree: true }


Suggested change

const propertyOpenedSubtree = { open: true, items: [{ a: 1 }], siblings: [], hasSubtree: true }

const propertyOpenedSubtree = { open: true, hasSubtree: true }

I think that is not required anymore

still required as it will be used for the Hierarchical tree tests.

layershifter · 2019-08-29T12:26:51Z

packages/react/src/themes/teams/components/Tree/treeItemStyles.ts

+import TreeTitle from '../../../../components/Tree/TreeTitle'
+
+const treeItemStyles: ComponentSlotStylesInput<TreeItemProps> = {
+  root: ({ theme: { siteVariables }, props: { level } }): ICSSInJSStyle => ({


Suggested change

root: ({ theme: { siteVariables }, props: { level } }): ICSSInJSStyle => ({

root: ({ theme: { siteVariables }, props: p } }): ICSSInJSStyle => ({

Please use p.* naming

…t-ui/react into feat/flat-tree-component

docs/src/examples/components/Tree/Usage/index.tsx

layershifter

Approved on my side, great job!

@miroslavstastny @mnajdova please also take a look

silviuaavram · 2019-08-30T08:48:56Z

Merging this. We can create issues to follow up, if any.

silviuaavram requested review from dvdzkwsk, kuzhelov, layershifter, levithomason, miroslavstastny and mnajdova as code owners August 9, 2019 10:11

vercel bot temporarily deployed to staging August 9, 2019 10:11 Inactive

vercel bot had a problem deploying to staging August 9, 2019 10:13 Failure

silviuaavram added help wanted Extra attention is needed 🚀 ready for review RFC 🚧 WIP labels Aug 9, 2019

vercel bot had a problem deploying to staging August 9, 2019 10:30 Failure

vercel bot had a problem deploying to staging August 9, 2019 12:39 Failure

lucivpav reviewed Aug 9, 2019

View reviewed changes

silviuaavram added 11 commits August 12, 2019 09:01

add Tree component

6503ccf

some progress on the handlers

399600b

fixed left-right navigation

d46d3b4

fixed more expand/collapse logic

bc3d663

add styles

8a15e9a

call onActiveItemsChange after state change

7338e35

remove react-virtualized

6f009f4

fixed posinset in title behavior

752647e

fix prop name in onActiveItemsChange call

3b81e15

behavior renames

d7311c0

refactor tree state

1b11bd4

silviuaavram force-pushed the feat/flat-tree-component branch from ae93151 to 1b11bd4 Compare August 12, 2019 13:23

vercel bot had a problem deploying to staging August 12, 2019 13:23 Failure

removed autocontrolled for now

8206641

vercel bot had a problem deploying to staging August 12, 2019 13:38 Failure

layershifter reviewed Aug 29, 2019

View reviewed changes

silviuaavram added 4 commits August 29, 2019 15:39

using lib methods

d15756a

more code review

d5304f2

move some examples to usage

2d2a1cd

Merge branch 'master' into feat/flat-tree-component

0af9a41

vercel bot temporarily deployed to staging August 29, 2019 14:24 Inactive

silviuaavram added 2 commits August 29, 2019 16:25

Merge branch 'feat/flat-tree-component' of https://github.com/stardus…

78a9d4f

…t-ui/react into feat/flat-tree-component

last round of review

84a2068

vercel bot deployed to staging August 29, 2019 14:27 View deployment

silviuaavram removed the 🚧 WIP label Aug 29, 2019

layershifter added ⚙️ enhancement New feature or request and removed RFC labels Aug 29, 2019

layershifter reviewed Aug 29, 2019

View reviewed changes

docs/src/examples/components/Tree/Usage/index.tsx Outdated Show resolved Hide resolved

Update docs/src/examples/components/Tree/Usage/index.tsx

2de17c2

vercel bot deployed to staging August 29, 2019 15:34 View deployment

layershifter approved these changes Aug 29, 2019

View reviewed changes

add changelog

8001b8c

vercel bot deployed to staging August 30, 2019 08:27 View deployment

silviuaavram merged commit 2d00948 into master Aug 30, 2019

delete-merged-branch bot deleted the feat/flat-tree-component branch August 30, 2019 08:49

	hasSubtree: Tree.hasSubtree(props),
	hasSubtree: hasSubtree(props),

	defaultColor: siteVars.colors.grey[750],
	color: siteVars.colorScheme.default.foreground,

		@@ -0,0 +1,7 @@
		import { TreeTitleVariables } from '../../../teams/components/Tree/treeTitleVariables'

	root: ({ variables, theme: { siteVariables } }): ICSSInJSStyle => ({
	root: ({ variables: v, theme: { siteVariables } }): ICSSInJSStyle => ({

		@@ -0,0 +1,9 @@
		export interface TreeTitleVariables {
		defaultColor: string

	const propertyOpenedSubtree = { open: true, items: [{ a: 1 }], siblings: [], hasSubtree: true }
	const propertyOpenedSubtree = { open: true, hasSubtree: true }

	root: ({ theme: { siteVariables }, props: { level } }): ICSSInJSStyle => ({
	root: ({ theme: { siteVariables }, props: p } }): ICSSInJSStyle => ({

Conversation

silviuaavram commented Aug 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

layershifter left a comment

Choose a reason for hiding this comment

Uh oh!

silviuaavram commented Aug 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

silviuaavram commented Aug 9, 2019 •

edited

Loading