Skip to content

Tree bug fixes introduced by keyboard navigation #1379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
May 18, 2018
Merged

Tree bug fixes introduced by keyboard navigation #1379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6ac616e
Tree bug fixes
garygong May 14, 2018
e29dbd5
prettier
garygong May 14, 2018
246119e
fix test, will figure out new test for this that works
garygong May 14, 2018
ee0788b
test
garygong May 14, 2018
79156b9
Tree: Clarify internal function names
interactivellama May 17, 2018
ccaf071
focus node and selected node fixes
garygong May 17, 2018
fe4aa23
Update Tree example to not have first item selected
interactivellama May 17, 2018
2c927d9
Tree: Refactor class methods
interactivellama May 17, 2018
89796f1
Tree: Update prop docs
interactivellama May 17, 2018
401255e
Tree: Use cloned nodes in Stories
interactivellama May 17, 2018
915401c
Tree lint:fix
interactivellama May 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 67 additions & 27 deletions components/component-docs.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6748,29 +6748,8 @@
},
"iconCategory": {
"type": {
"name": "enum",
"value": [
{
"value": "'action'",
"computed": false
},
{
"value": "'custom'",
"computed": false
},
{
"value": "'doctype'",
"computed": false
},
{
"value": "'standard'",
"computed": false
},
{
"value": "'utility'",
"computed": false
}
]
"name": "custom",
"raw": "requiredIf(\n PropTypes.oneOf(['action', 'custom', 'doctype', 'standard', 'utility']),\n (props) => !!props.iconName\n)"
},
"required": false,
"description": "Name of the icon category. Visit <a href=\"http://www.lightningdesignsystem.com/resources/icons\">Lightning Design System Icons</a> to reference icon categories."
Expand Down Expand Up @@ -7445,6 +7424,23 @@
"computed": false
}
},
"assistiveText": {
"type": {
"name": "shape",
"value": {
"dialogLabel": {
"name": "string",
"required": false
}
}
},
"required": false,
"description": "Assistive text for the modal.",
"defaultValue": {
"value": "{}",
"computed": false
}
},
"children": {
"type": {
"name": "node"
Expand Down Expand Up @@ -11110,7 +11106,51 @@
},
"tree": {
"description": "A tree is visualization of a structure hierarchy. A branch can be expanded or collapsed. This is a controlled component, since visual state is present in the `nodes` data.",
"methods": [],
"methods": [
{
"name": "handleSelect",
"docblock": null,
"modifiers": [],
"params": [
{
"name": "event",
"type": null
},
{
"name": "data",
"type": null
},
{
"name": "clearSelectedNodes",
"type": null
}
],
"returns": null
},
{
"name": "handleNodeBlur",
"docblock": null,
"modifiers": [],
"params": [],
"returns": null
},
{
"name": "handleExpand",
"docblock": null,
"modifiers": [],
"params": [
{
"name": "event",
"type": null
},
{
"name": "data",
"type": null
}
],
"returns": null
}
],
"props": {
"assistiveText": {
"type": {
Expand Down Expand Up @@ -11185,21 +11225,21 @@
"name": "array"
},
"required": false,
"description": "Array of items starting at the top of the tree. The required shape is: `{expanded: string, id: string, label: string, selected: string, type: string, nodes: array}`, but only `id` and `label` are required. Use `type: 'branch'` for folder and categories."
"description": "Array of items starting at the top of the tree. The shape each node in the array is:\n```\n{\n expanded: string,\n id: string,\n label: string or node,\n selected: string,\n type: string,\n nodes: array\n}\n```\n`assistiveText: string` is optional and helpful if the label is not a string. Only `id` and `label` are required. Use `type: 'branch'` for folder and categories."
},
"onClick": {
"type": {
"name": "func"
},
"required": true,
"description": "Function that will run whenever an item or branch is clicked."
"description": "Function that will run whenever an item or branch is selected due to click or keyboard navigation."
},
"onExpandClick": {
"type": {
"name": "func"
},
"required": true,
"description": "This function triggers when the expand or collapse icon is clicked."
"description": "This function triggers when the expand or collapse icon is clicked or due to keyboard navigation."
},
"onScroll": {
"type": {
Expand Down
7 changes: 4 additions & 3 deletions components/tree/__docs__/storybook-stories.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React from 'react';
import createReactClass from 'create-react-class';
import PropTypes from 'prop-types';
import { storiesOf, action } from '@storybook/react';
import cloneDeep from 'lodash.clonedeep';
import IconSettings from '../../icon-settings';

import { TREE } from '../../../utilities/constants';
Expand Down Expand Up @@ -38,7 +39,7 @@ const DemoTree = createReactClass({
? sampleNodes[this.props.exampleNodesIndex]
: sampleNodes.sampleNodesDefault;
return {
nodes: initalNodes,
nodes: cloneDeep(initalNodes),
selectedNode: undefined,
searchTerm: this.props.searchable ? 'fruit' : undefined,
};
Expand Down Expand Up @@ -74,15 +75,15 @@ const DemoTree = createReactClass({

return { selectedNode: data.node };
});
itemClicked('Node Clicked')(event, data);
itemClicked('Node Selected')(event, data);
} else if (
!this.props.noBranchSelection ||
(this.props.noBranchSelection && data.node.type !== 'branch')
) {
data.node.selected = data.select;
// trigger render
this.setState((prevState) => ({ ...prevState }));
itemClicked('Node Clicked')(event, data);
itemClicked('Node Selected')(event, data);
}
},

Expand Down
3 changes: 2 additions & 1 deletion components/tree/__examples__/default.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ const sampleNodes = {
expanded: true,
nodes: [
{
label: 'Ground Fruits',
assistiveText: 'Ground Fruits',
label: <span>Ground Fruits</span>,
type: 'branch',
id: 4,
nodes: [
Expand Down
8 changes: 5 additions & 3 deletions components/tree/__tests__/tree.browser-test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import createReactClass from 'create-react-class';
import PropTypes from 'prop-types';

import isFunction from 'lodash.isfunction';
import cloneDeep from 'lodash.clonedeep';

import chai, { expect } from 'chai';
import chaiEnzyme from 'chai-enzyme';
Expand Down Expand Up @@ -60,7 +61,7 @@ const DemoTree = createReactClass({
? sampleNodes[this.props.exampleNodesIndex]
: sampleNodes.sampleNodesDefault;
return {
nodes: initalNodes,
nodes: cloneDeep(initalNodes),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was seeing that nodes values were carrying over test-to-test. We should start fresh every time.

searchTerm: this.props.searchable ? 'fruit' : undefined,
};
},
Expand Down Expand Up @@ -312,9 +313,10 @@ describe('Tree: ', () => {

it('has initial selection', function () {
let selectedNode = this.wrapper
.find('#example-tree-1')
.find('#example-tree-2')
.find('.slds-is-selected');
expect(selectedNode).to.have.length(1);
// Fruits, Watermelon, Tree Fruits
expect(selectedNode).to.have.length(3);
selectedNode = this.wrapper
.find('#example-tree-5')
.find('.slds-is-selected');
Expand Down
Loading