feat(Tree): control activeItemIds through expanded prop of items (#2061)

* add expanded ids in activeItemIds * replace open with expanded * made test to work for expanded * fixed issue and added tests * changelog * fix changelog * fix changelog again * add test to behavior * fix expanded issue * add test to tree * add more tests to TreeItem * more tests to behavior * TreeTitle conformant * default size is actually 1
microsoft · Nov 18, 2019 · 3adb87d · 3adb87d
1 parent 54fafef
commit 3adb87d
Show file tree

Hide file tree

Showing 11 changed files with 198 additions and 49 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,8 @@
 This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### BREAKING CHANGES
+- Control Tree `activeItemIds` through `expanded` TreeItem prop @silviuavram ([#2061](https://github.com/stardust-ui/react/pull/2061))
 
 ### Fixes
 - Update Silver color scheme, changing `backgroundHover` and `backgroundPressed` for high-contrast theme @pompomon ([#2078](https://github.com/microsoft/fluent-ui-react/pull/2078))

diff --git a/packages/accessibility/src/behaviors/Tree/treeItemBehavior.ts b/packages/accessibility/src/behaviors/Tree/treeItemBehavior.ts
@@ -5,13 +5,15 @@ import { IS_FOCUSABLE_ATTRIBUTE } from '../../attributes'
 import treeTitleBehavior from './treeTitleBehavior'
 
 /**
- * @description
- * Adds role 'treeitem' to a non-leaf item and 'none' to a leaf item.
- * Adds 'aria-expanded' with a value based on the 'open' prop if item is not a leaf.
- * Adds 'tabIndex' as '-1' if the item is not a leaf.
- *
  * @specification
+ * Adds attribute 'aria-expanded=true' based on the property 'expanded' if the component has 'hasSubtree' property.
+ * Adds attribute 'tabIndex=-1' to 'root' slot if 'hasSubtree' property is true. Does not set the attribute otherwise.
+ * Adds attribute 'aria-setsize=3' based on the property 'treeSize' if the component has 'hasSubtree' property.
+ * Adds attribute 'aria-posinset=2' based on the property 'index' if the component has 'hasSubtree' property.
+ * Adds attribute 'aria-level=1' based on the property 'level' if the component has 'hasSubtree' property.
+ * Adds attribute 'role=treeitem' to 'root' slot if 'hasSubtree' property is true. Sets the attribute to 'none' otherwise.
  * Triggers 'performClick' action with 'Enter' or 'Spacebar' on 'root'.
+ * Triggers 'expandSiblings' action with '*' on 'root'.
  * Triggers 'focusParent' action with 'ArrowLeft' on 'root', when has a closed subtree.
  * Triggers 'collapse' action with 'ArrowLeft' on 'root', when has an opened subtree.
  * Triggers 'expand' action with 'ArrowRight' on 'root', when has a closed subtree.
@@ -22,7 +24,7 @@ const treeItemBehavior: Accessibility<TreeItemBehaviorProps> = props => ({
     root: {
       role: 'none',
       ...(props.hasSubtree && {
-        'aria-expanded': props.open,
+        'aria-expanded': props.expanded,
         tabIndex: -1,
         [IS_FOCUSABLE_ATTRIBUTE]: true,
         role: 'treeitem',
@@ -37,15 +39,15 @@ const treeItemBehavior: Accessibility<TreeItemBehaviorProps> = props => ({
       performClick: {
         keyCombinations: [{ keyCode: keyboardKey.Enter }, { keyCode: keyboardKey.Spacebar }],
       },
-      ...(isSubtreeOpen(props) && {
+      ...(isSubtreeExpanded(props) && {
         collapse: {
           keyCombinations: [{ keyCode: keyboardKey.ArrowLeft }],
         },
         focusFirstChild: {
           keyCombinations: [{ keyCode: keyboardKey.ArrowRight }],
         },
       }),
-      ...(!isSubtreeOpen(props) && {
+      ...(!isSubtreeExpanded(props) && {
         expand: {
           keyCombinations: [{ keyCode: keyboardKey.ArrowRight }],
         },
@@ -64,18 +66,18 @@ const treeItemBehavior: Accessibility<TreeItemBehaviorProps> = props => ({
 })
 
 export type TreeItemBehaviorProps = {
-  /** If item is a subtree, it indicates if it's open. */
-  open?: boolean
+  /** If item is a subtree, it indicates if it's expanded. */
+  expanded?: boolean
   level?: number
   index?: number
   hasSubtree?: boolean
   treeSize?: number
 }
 
-/** Checks if current tree item has a subtree and it is opened */
-const isSubtreeOpen = (props: TreeItemBehaviorProps): boolean => {
-  const { hasSubtree, open } = props
-  return !!(hasSubtree && open)
+/** Checks if current tree item has a subtree and it is expanded */
+const isSubtreeExpanded = (props: TreeItemBehaviorProps): boolean => {
+  const { hasSubtree, expanded } = props
+  return !!(hasSubtree && expanded)
 }
 
 export default treeItemBehavior
diff --git a/packages/accessibility/src/behaviors/Tree/treeTitleBehavior.ts b/packages/accessibility/src/behaviors/Tree/treeTitleBehavior.ts
@@ -4,11 +4,12 @@ import { IS_FOCUSABLE_ATTRIBUTE } from '../../attributes'
 import { Accessibility } from '../../types'
 
 /**
- * @description
- * Adds role 'treeitem' if the title is a leaf node inside the tree.
- * Adds 'tabIndex' as '-1' if the title is a leaf node inside the tree.
- *
  * @specification
+ * Adds attribute 'tabIndex=-1' to 'root' slot if 'hasSubtree' property is false or undefined. Does not set the attribute if true.
+ * Adds attribute 'role=treeitem' to 'root' slot if 'hasSubtree' property is false or undefined. Does not set the attribute if true.
+ * Adds attribute 'aria-setsize=3' based on the property 'treeSize' if the component has 'hasSubtree' property false or undefined. Does not set anything if true..
+ * Adds attribute 'aria-posinset=2' based on the property 'index' if the component has 'hasSubtree' property false or undefined. Does not set anything if true..
+ * Adds attribute 'aria-level=1' based on the property 'level' if the component has 'hasSubtree' property false or undefined. Does not set anything if true..
  * Triggers 'performClick' action with 'Enter' or 'Spacebar' on 'root'.
  */
 const treeTitleBehavior: Accessibility<TreeTitleBehavior> = props => ({

diff --git a/packages/accessibility/test/behaviors/testDefinitions.ts b/packages/accessibility/test/behaviors/testDefinitions.ts
@@ -167,11 +167,13 @@ function testMethodConditionallyAddAttribute(
   component,
   propertyDependsOn,
   valueOfProperty,
+  valueOfPropertyOtherwise,
   attributeToBeAdded,
   valueOfAttributeToBeAddedIfTrue,
   valueOfAttributeToBeAddedOtherwise,
 ) {
   const propertyWithAriaSelected = {}
+  propertyWithAriaSelected[propertyDependsOn] = valueOfPropertyOtherwise
   const expectedResultAttributeNotDefined = parameters.behavior(propertyWithAriaSelected)
     .attributes[component][attributeToBeAdded]
   expect(testHelper.convertToMatchingTypeIfApplicable(expectedResultAttributeNotDefined)).toEqual(
@@ -189,7 +191,7 @@ function testMethodConditionallyAddAttribute(
 
 // Example: Adds attribute 'aria-disabled=true' to 'trigger' slot if 'disabled' property is true. Does not set the attribute otherwise.
 definitions.push({
-  regexp: /Adds attribute '([\w-]+)=([\w\d]+)' to '([\w-]+)' slot if '([\w-]+)' property is true\. Does not set the attribute otherwise\./g,
+  regexp: /Adds attribute '([\w-]+)=([\w\d-]+)' to '([\w-]+)' slot if '([\w-]+)' property is true\. Does not set the attribute otherwise\./g,
   testMethod: (parameters: TestMethod) => {
     const [
       attributeToBeAdded,
@@ -203,6 +205,31 @@ definitions.push({
       component,
       propertyDependsOn,
       true,
+      undefined,
+      attributeToBeAdded,
+      valueOfAttributeToBeAdded,
+      undefined,
+    )
+  },
+})
+
+// Example: Adds attribute 'aria-disabled=true' to 'trigger' slot if 'disabled' property is false or undefined. Does not set the attribute if true.
+definitions.push({
+  regexp: /Adds attribute '([\w-]+)=([\w\d-]+)' to '([\w-]+)' slot if '([\w-]+)' property is false or undefined\. Does not set the attribute if true\./g,
+  testMethod: (parameters: TestMethod) => {
+    const [
+      attributeToBeAdded,
+      valueOfAttributeToBeAdded,
+      component,
+      propertyDependsOn,
+    ] = parameters.props
+
+    testMethodConditionallyAddAttribute(
+      parameters,
+      component,
+      propertyDependsOn,
+      undefined,
+      true,
       attributeToBeAdded,
       valueOfAttributeToBeAdded,
       undefined,
@@ -227,14 +254,15 @@ definitions.push({
       component,
       propertyDependsOn,
       true,
+      undefined,
       attributeToBeAdded,
       valueOfAttributeToBeAddedIfTrue,
       valueOfAttributeToBeAddedOtherwise,
     )
   },
 })
 
-// Adds attribute 'aria-haspopup=true' to 'root' slot if 'menu' menu property is set.
+// Adds attribute 'aria-haspopup=true' to 'root' slot if 'menu' property is set.
 definitions.push({
   regexp: /Adds attribute '([\w-]+)=([\w\d]+)' to '([\w-]+)' slot if '([\w-]+)' property is set\./g,
   testMethod: (parameters: TestMethod) => {
@@ -251,6 +279,7 @@ definitions.push({
       component,
       propertyDependsOn,
       'custom-value',
+      undefined,
       attributeToBeAdded,
       valueOfAttributeToBeAddedIfTrue,
       valueOfAttributeToBeAddedOtherwise,
@@ -317,6 +346,44 @@ definitions.push({
   },
 })
 
+// Example:  Adds attribute 'aria-expanded=true' based on the property 'open' if the component has 'hasSubtree' property false or undefined. Does not set anything if true.
+definitions.push({
+  regexp: /Adds attribute '([\w-]+)=(\w+)' based on the property '(\w+)' if the component has '(\w+)' property false or undefined. Does not set anything if true\./g,
+  testMethod: (parameters: TestMethod) => {
+    const [
+      attributeToBeAdded,
+      attributeExpectedValue,
+      propertyDependingOnFirst,
+      propertyDependingOnSecond,
+    ] = parameters.props
+
+    const property = {}
+
+    property[propertyDependingOnFirst] = attributeExpectedValue
+    property[propertyDependingOnSecond] = false
+    const actualResultIfFalse = parameters.behavior(property).attributes.root[attributeToBeAdded]
+    expect(testHelper.convertToMatchingTypeIfApplicable(actualResultIfFalse)).toEqual(
+      testHelper.convertToMatchingTypeIfApplicable(attributeExpectedValue),
+    )
+
+    property[propertyDependingOnSecond] = undefined
+    const actualResultIfUndefined = parameters.behavior(property).attributes.root[
+      attributeToBeAdded
+    ]
+    expect(testHelper.convertToMatchingTypeIfApplicable(actualResultIfUndefined)).toEqual(
+      testHelper.convertToMatchingTypeIfApplicable(attributeExpectedValue),
+    )
+
+    const propertyFirstPropUndefined = {}
+    propertyFirstPropUndefined[propertyDependingOnSecond] = true
+    const actualResultFirstPropertyNegateUndefined = parameters.behavior(propertyFirstPropUndefined)
+      .attributes.root[attributeToBeAdded]
+    expect(
+      testHelper.convertToMatchingTypeIfApplicable(actualResultFirstPropertyNegateUndefined),
+    ).toEqual(undefined)
+  },
+})
+
 // Example:  Adds attribute 'aria-expanded=true' based on the property 'open' if the component has 'hasSubtree' property.
 definitions.push({
   regexp: /Adds attribute '([\w-]+)=(\w+)' based on the property '(\w+)' if the component has '(\w+)' property\./g,
@@ -337,16 +404,18 @@ definitions.push({
       testHelper.convertToMatchingTypeIfApplicable(attributeExpectedValue),
     )
 
-    const propertyFirstPropNegate = {}
-    propertyFirstPropNegate[
-      propertyDependingOnFirst
-    ] = !testHelper.convertToMatchingTypeIfApplicable(attributeExpectedValue)
-    propertyFirstPropNegate[propertyDependingOnSecond] = true
-    const actualResultFirstPropertyNegate = parameters.behavior(propertyFirstPropNegate).attributes
-      .root[attributeToBeAdded]
-    expect(testHelper.convertToMatchingTypeIfApplicable(actualResultFirstPropertyNegate)).toEqual(
-      !testHelper.convertToMatchingTypeIfApplicable(attributeExpectedValue),
-    )
+    if (typeof testHelper.convertToMatchingTypeIfApplicable(attributeExpectedValue) === 'boolean') {
+      const propertyFirstPropNegate = {}
+      propertyFirstPropNegate[
+        propertyDependingOnFirst
+      ] = !testHelper.convertToMatchingTypeIfApplicable(attributeExpectedValue)
+      propertyFirstPropNegate[propertyDependingOnSecond] = true
+      const actualResultFirstPropertyNegate = parameters.behavior(propertyFirstPropNegate)
+        .attributes.root[attributeToBeAdded]
+      expect(testHelper.convertToMatchingTypeIfApplicable(actualResultFirstPropertyNegate)).toEqual(
+        !testHelper.convertToMatchingTypeIfApplicable(attributeExpectedValue),
+      )
+    }
 
     const propertyFirstPropUndefined = {}
     propertyFirstPropUndefined[propertyDependingOnFirst] = true
@@ -597,7 +666,13 @@ definitions.push({
   regexp: /Triggers '(\w+)' action with '(\w+)' on '([\w-]+)', when has an opened subtree\./g,
   testMethod: (parameters: TestMethod) => {
     const [action, key, elementToPerformAction] = [...parameters.props]
-    const propertyOpenedSubtree = { open: true, items: [{ a: 1 }], siblings: [], hasSubtree: true }
+    const propertyOpenedSubtree = {
+      open: true,
+      expanded: true,
+      items: [{ a: 1 }],
+      siblings: [],
+      hasSubtree: true,
+    }
     const expectedKeyNumberVertical = parameters.behavior(propertyOpenedSubtree).keyActions[
       elementToPerformAction
     ][action].keyCombinations[0].keyCode
@@ -610,7 +685,7 @@ definitions.push({
   regexp: /Triggers '(\w+)' action with '(\w+)' on '([\w-]+)', when has a closed subtree\./g,
   testMethod: (parameters: TestMethod) => {
     const [action, key, elementToPerformAction] = [...parameters.props]
-    const propertyClosedSubtree = { open: false, hasSubtree: false }
+    const propertyClosedSubtree = { open: false, expanded: false, hasSubtree: false }
     const expectedKeyNumberVertical = parameters.behavior(propertyClosedSubtree).keyActions[
       elementToPerformAction
     ][action].keyCombinations[0].keyCode

diff --git a/packages/accessibility/test/behaviors/treeItemBehavior-test.tsx b/packages/accessibility/test/behaviors/treeItemBehavior-test.tsx
@@ -20,12 +20,12 @@ describe('TreeItemBehavior', () => {
     })
 
     test(`is added with 'false' value to an item that is expandable but not open`, () => {
-      const expectedResult = treeItemBehavior({ hasSubtree: true, open: false })
+      const expectedResult = treeItemBehavior({ hasSubtree: true, expanded: false })
       expect(expectedResult.attributes.root['aria-expanded']).toEqual(false)
     })
 
     test(`is added with 'false' value to an item that is expandable and open`, () => {
-      const expectedResult = treeItemBehavior({ hasSubtree: true, open: true })
+      const expectedResult = treeItemBehavior({ hasSubtree: true, expanded: true })
       expect(expectedResult.attributes.root['aria-expanded']).toEqual(true)
     })
   })

diff --git a/packages/react/src/components/Tree/Tree.tsx b/packages/react/src/components/Tree/Tree.tsx
@@ -26,7 +26,7 @@ import {
   ShorthandValue,
 } from '../../types'
 import { hasSubtree, removeItemAtIndex } from './lib'
-import { TreeTitleProps } from './TreeTitle'
+import TreeTitle, { TreeTitleProps } from './TreeTitle'
 import { ReactAccessibilityBehavior } from '../../lib/accessibility/reactTypes'
 
 export interface TreeSlotClassNames {
@@ -37,13 +37,13 @@ export interface TreeProps extends UIComponentProps, ChildrenComponentProps {
   /** Accessibility behavior if overridden by the user. */
   accessibility?: Accessibility
 
-  /** Ids of opened items. */
+  /** Ids of expanded items. */
   activeItemIds?: string[]
 
   /** Initial activeItemIds value. */
   defaultActiveItemIds?: string[]
 
-  /** Only allow one subtree to be open at a time. */
+  /** Only allow one subtree to be expanded at a time. */
   exclusive?: boolean
 
   /** Shorthand array of props for Tree. */
@@ -112,6 +112,9 @@ class Tree extends AutoControlledComponent<WithAsProp<TreeProps>, TreeState> {
 
   static autoControlledProps = ['activeItemIds']
 
+  static Item = TreeItem
+  static Title = TreeTitle
+
   // memoize this function if performance issue occurs.
   static getItemsForRender = (itemsFromProps: ShorthandCollection<TreeItemProps>) => {
     const itemsForRenderGenerator = (
@@ -146,9 +149,35 @@ class Tree extends AutoControlledComponent<WithAsProp<TreeProps>, TreeState> {
   }
 
   static getAutoControlledStateFromProps(nextProps: TreeProps, prevState: TreeState) {
-    const itemsForRender = Tree.getItemsForRender(nextProps.items)
+    const { items } = nextProps
+    const itemsForRender = Tree.getItemsForRender(items)
+    let { activeItemIds } = nextProps
+
+    if (!activeItemIds && items) {
+      activeItemIds = prevState.activeItemIds
+
+      const expandedItemsGenerator = (items, acc = activeItemIds) =>
+        _.reduce(
+          items,
+          (acc, item) => {
+            if (item['expanded'] && acc.indexOf(item['id']) === -1) {
+              acc.push(item['id'])
+            }
+
+            if (item['items']) {
+              return expandedItemsGenerator(item['items'], acc)
+            }
+
+            return acc
+          },
+          acc,
+        )
+
+      expandedItemsGenerator(items)
+    }
 
     return {
+      activeItemIds,
       itemsForRender,
     }
   }
@@ -273,14 +302,14 @@ class Tree extends AutoControlledComponent<WithAsProp<TreeProps>, TreeState> {
           const itemForRender = itemsForRender[item['id']]
           const { elementRef, ...restItemForRender } = itemForRender
           const isSubtree = hasSubtree(item)
-          const isSubtreeOpen = isSubtree && this.isActiveItem(item['id'])
+          const isSubtreeExpanded = isSubtree && this.isActiveItem(item['id'])
           const renderedItem = TreeItem.create(item, {
             defaultProps: () => ({
               accessibility: accessibility.childBehaviors
                 ? accessibility.childBehaviors.item
                 : undefined,
               className: Tree.slotClassNames.item,
-              open: isSubtreeOpen,
+              expanded: isSubtreeExpanded,
               renderItemTitle,
               key: item['id'],
               contentRef: elementRef,
@@ -292,7 +321,7 @@ class Tree extends AutoControlledComponent<WithAsProp<TreeProps>, TreeState> {
           return [
             ...renderedItems,
             renderedItem,
-            ...(isSubtreeOpen ? renderItems(item['items']) : ([] as any)),
+            ...(isSubtreeExpanded ? renderItems(item['items']) : ([] as any)),
           ]
         },
         [],