fix: accessibility issues on Tree component, adjusting examples (#1703)

* fix: accessibility issues on Tree component, adjusting examples * replacing yarn.lock with master * cleaning code * cleaning code * fix: tree accesibility issues * fix: tree accesibility issues * fix: impriving code * fix: test * chore: fix yarn.lock * test: fix integration tests Co-authored-by: Tahimi <tahimileon@gmail.com> Co-authored-by: LeandroTorresSicilia <jtorressicilia@gmail.com>
nexxtway · Jul 16, 2020 · eb42f71 · eb42f71
1 parent 28c3626
commit eb42f71
Show file tree

Hide file tree

Showing 17 changed files with 311 additions and 79 deletions.
diff --git a/integration/specs/Tree/tree-1.spec.js b/integration/specs/Tree/tree-1.spec.js
@@ -25,13 +25,13 @@ describe('Tree basic', () => {
         node.click();
         expect(node.isExpanded()).toBe(false);
     });
-    it('should move focus to the next button icon when the first button icon is focused and press tab', () => {
+    it('should move focus outside the tree when the first button icon is focused and press tab', () => {
         const tree = new PageTree(TREE);
         const firstNode = tree.getNode(2);
         const secondNode = tree.getNode(3);
         firstNode.click();
         browser.keys(TAB_KEY);
-        expect(secondNode.hasFocus()).toBe(true);
+        expect(secondNode.hasFocus()).toBe(false);
     });
     it('should expand the node when its button icon is focused, press enter and the node was initially expanded', () => {
         const tree = new PageTree(TREE);

diff --git a/src/components/Tree/__test__/tree.spec.js b/src/components/Tree/__test__/tree.spec.js
@@ -39,26 +39,26 @@ const data = [
 ];
 
 describe('<Tree/>', () => {
-    it('should call onExpandCollapse with the right parameters when the button is clicked', () => {
+    it('should call onNodeExpand with the right parameters when the button is clicked', () => {
         const nodePath = [2, 1];
-        const onExpandCollapsekMock = jest.fn();
-        const component = mount(<Tree data={data} onExpandCollapse={onExpandCollapsekMock} />);
+        const onNodeExandMock = jest.fn();
+        const component = mount(<Tree data={data} onNodeExpand={onNodeExandMock} />);
         component
             .find('ButtonIcon')
             .at(1)
             .simulate('click');
-        expect(onExpandCollapsekMock).toHaveBeenCalledWith({ nodePath });
+        expect(onNodeExandMock).toHaveBeenCalledWith({ nodePath });
     });
-    it('should call onSelect with the right parameters when the node is selected', () => {
+    it('should call onNodeCheck with the right parameters when the node is selected', () => {
         const nodePath = [2];
-        const onSelectMock = jest.fn();
-        const component = mount(<Tree data={data} onSelect={onSelectMock} />);
+        const onNodeCheckMock = jest.fn();
+        const component = mount(<Tree data={data} onNodeCheck={onNodeCheckMock} />);
         component
             .find('PrimitiveCheckbox')
             .at(2)
             .find('input')
             .simulate('change');
-        expect(onSelectMock).toHaveBeenCalledWith({ nodePath });
+        expect(onNodeCheckMock).toHaveBeenCalledWith({ nodePath });
     });
     it('should render the correct number of children', () => {
         const component = mount(<Tree data={data} />);

diff --git a/src/components/Tree/child.js b/src/components/Tree/child.js
@@ -8,7 +8,9 @@ import NodeContainer from './styled/nodeContainer';
 import Label from './styled/label';
 import IconContainer from './styled/iconContainer';
 import InputCheckbox from './styled/inputCheckbox';
-import ChildrenContainer from './styled/childrenContainer';
+import ChildrenContainerUl from './styled/childrenContainer';
+import getNodeLevel from './helpers/getNodeLevel';
+import getTabIndex from './helpers/getTabIndex';
 
 export default function Child(props) {
     const {
@@ -19,26 +21,62 @@ export default function Child(props) {
         isChecked,
         icon,
         nodePath,
-        onExpandCollapse,
-        onSelect,
+        onNodeExpand,
+        onNodeCheck,
+        onNodeSelect,
+        isSelected,
+        name,
+        selectedNode,
+        isFirstNode,
     } = props;
     const hasChildren = Array.isArray(children);
     const hasCheckbox = typeof isChecked === 'boolean' || isChecked === 'indeterminate';
     const hasIcon = !!icon;
+    const ariaLevelValue = getNodeLevel({ name });
+    const ariaExpandedValue = hasChildren ? isExpanded : undefined;
+    const ariaSelectedValue = isSelected === true ? isSelected : undefined;
+    const tabIndex = getTabIndex({ selectedNode, isFirstNode, isSelected });
+
+    const handleNodeSelect = event => {
+        event.stopPropagation();
+        onNodeSelect({ name, nodePath });
+    };
+
+    const handleExpandCollapse = () => {
+        return onNodeExpand({ nodePath });
+    };
+
     return (
-        <ItemContainerLi hasChildren={hasChildren} icon={icon} data-id="node-element-li">
-            <NodeContainer data-id="node-element">
+        <ItemContainerLi
+            id={name}
+            data-id="node-element-li"
+            icon={icon}
+            hasChildren={hasChildren}
+            onClick={handleNodeSelect}
+            role="treeitem"
+            aria-level={ariaLevelValue}
+            aria-expanded={ariaExpandedValue}
+            aria-selected={ariaSelectedValue}
+            tabIndex={tabIndex}
+        >
+            <NodeContainer
+                data-id="node-element"
+                isSelected={isSelected}
+                ariaLevelValue={ariaLevelValue}
+                hasChildren={hasChildren}
+            >
                 <ExpandCollapseButton
                     hasChildren={hasChildren}
                     isExpanded={isExpanded === true}
                     isLoading={isLoading === true}
-                    onClick={() => onExpandCollapse({ nodePath })}
+                    onClick={handleExpandCollapse}
                 />
                 <RenderIf isTrue={hasCheckbox}>
                     <InputCheckbox
                         type="checkbox"
+                        label=""
                         checked={isChecked}
-                        onChange={() => onSelect({ nodePath })}
+                        onChange={() => onNodeCheck({ nodePath })}
                     />
                 </RenderIf>
                 <RenderIf isTrue={hasIcon}>
@@ -47,14 +85,17 @@ export default function Child(props) {
                 <Label icon={icon}>{label}</Label>
             </NodeContainer>
             <RenderIf isTrue={hasChildren && isExpanded}>
-                <ChildrenContainer icon={icon} isChecked={isChecked}>
+                <ChildrenContainerUl icon={icon} isChecked={isChecked} role="group">
                     <TreeChildren
                         data={children}
-                        onSelect={onSelect}
-                        onExpandCollapse={onExpandCollapse}
+                        onNodeCheck={onNodeCheck}
+                        onNodeExpand={onNodeExpand}
                         nodePath={nodePath}
+                        parentName={name}
+                        onNodeSelect={onNodeSelect}
+                        selectedNode={selectedNode}
                     />
-                </ChildrenContainer>
+                </ChildrenContainerUl>
             </RenderIf>
         </ItemContainerLi>
     );
@@ -67,9 +108,14 @@ Child.propTypes = {
     isLoading: PropTypes.bool,
     icon: PropTypes.node,
     children: PropTypes.array,
-    onExpandCollapse: PropTypes.func,
-    onSelect: PropTypes.func,
+    onNodeCheck: PropTypes.func,
+    onNodeExpand: PropTypes.func,
     nodePath: PropTypes.array,
+    onNodeSelect: PropTypes.func,
+    isSelected: PropTypes.bool,
+    name: PropTypes.string,
+    selectedNode: PropTypes.string,
+    isFirstNode: PropTypes.bool,
 };
 
 Child.defaultProps = {
@@ -79,7 +125,12 @@ Child.defaultProps = {
     isLoading: undefined,
     children: undefined,
     icon: null,
-    onExpandCollapse: () => {},
-    onSelect: () => {},
+    onNodeExpand: () => {},
+    onNodeCheck: () => {},
     nodePath: [],
+    onNodeSelect: undefined,
+    isSelected: undefined,
+    name: undefined,
+    selectedNode: undefined,
+    isFirstNode: undefined,
 };
diff --git a/src/components/Tree/expandCollapseButton.js b/src/components/Tree/expandCollapseButton.js
@@ -23,7 +23,7 @@ export default function ExpandCollapseButton(props) {
         );
     }
     if (hasChildren) {
-        return <Button size="x-small" icon={getIcon(isExpanded)} onClick={onClick} />;
+        return <Button size="x-small" icon={getIcon(isExpanded)} onClick={onClick} tabIndex={-1} />;
     }
     return null;
 }

diff --git a/src/components/Tree/helpers/getNodeLevel.js b/src/components/Tree/helpers/getNodeLevel.js
@@ -0,0 +1,9 @@
+const getNodeLevel = ({ name = '' }) => {
+    const levelMatch = name.match(/\./g);
+    if (levelMatch) {
+        return levelMatch.length + 1;
+    }
+    return 1;
+};
+
+export default getNodeLevel;
diff --git a/src/components/Tree/helpers/getNodeName.js b/src/components/Tree/helpers/getNodeName.js
@@ -0,0 +1,9 @@
+const getNodeName = ({ parentName, index }) => {
+    const nodeLevel = index + 1;
+    if (parentName) {
+        return `${parentName}.${nodeLevel}`;
+    }
+    return `node-${nodeLevel}`;
+};
+
+export default getNodeName;
diff --git a/src/components/Tree/helpers/getTabIndex.js b/src/components/Tree/helpers/getTabIndex.js
@@ -0,0 +1,11 @@
+const getTabIndex = ({ selectedNode, isSelected, isFirstNode }) => {
+    if (isSelected) {
+        return 0;
+    }
+    if (!selectedNode && isFirstNode) {
+        return 0;
+    }
+    return -1;
+};
+
+export default getTabIndex;
diff --git a/src/components/Tree/index.js b/src/components/Tree/index.js
@@ -9,15 +9,35 @@ import getNode from './helpers/getNode';
  * @category Layout
  */
 export default function Tree(props) {
-    const { data, onExpandCollapse, onSelect, className, style, id } = props;
+    const {
+        data,
+        onNodeExpand,
+        onNodeCheck,
+        onNodeSelect,
+        selectedNode,
+        className,
+        style,
+        id,
+        ariaLabel,
+        ariaLabelledBy,
+    } = props;
 
     return (
-        <TreeContainerUl className={className} style={style} id={id}>
+        <TreeContainerUl
+            className={className}
+            style={style}
+            id={id}
+            role="tree"
+            aria-labelledby={ariaLabelledBy}
+            aria-label={ariaLabel}
+        >
             <TreeChildren
                 data={data}
-                onExpandCollapse={onExpandCollapse}
-                onSelect={onSelect}
+                onNodeExpand={onNodeExpand}
+                onNodeCheck={onNodeCheck}
                 nodePath={[]}
+                selectedNode={selectedNode}
+                onNodeSelect={onNodeSelect}
             />
         </TreeContainerUl>
     );
@@ -37,24 +57,37 @@ Tree.propTypes = {
         }),
     ),
     /** The action triggered when the user clicks in the tree node expand or collapse button. */
-    onExpandCollapse: PropTypes.func,
+    onNodeExpand: PropTypes.func,
     /** The action triggered when the user clicks in the tree node checkbox. */
-    onSelect: PropTypes.func,
+    onNodeCheck: PropTypes.func,
+    /** The action triggered when the user clicks in the tree node label. */
+    onNodeSelect: PropTypes.func,
+    /** The tree node name. */
+    selectedNode: PropTypes.string,
     /** A CSS class for the outer element, in addition to the component's base classes. */
     className: PropTypes.string,
     /** An object with custom style applied for the outer element. */
     style: PropTypes.object,
     /** The id of the outer element. */
     id: PropTypes.string,
+    /** The id of the tree heading element. Set to the parent element of the tree who contains the tree nodes.
+     * No need to use "ariaLabel" atribute if this one apply */
+    ariaLabelledBy: PropTypes.string,
+    /** The label for the parent element of the tree who contains the tree nodes. Apply if no tree heading element present */
+    ariaLabel: PropTypes.string,
 };
 
 Tree.defaultProps = {
     data: [],
-    onExpandCollapse: () => {},
-    onSelect: () => {},
+    onNodeExpand: () => {},
+    onNodeCheck: () => {},
+    onNodeSelect: () => {},
+    selectedNode: undefined,
     className: undefined,
     style: undefined,
     id: undefined,
+    ariaLabelledBy: undefined,
+    ariaLabel: undefined,
 };
 
 /**