From f4ce73c6e75b6180ca6546583ff405d27874669f Mon Sep 17 00:00:00 2001 From: Hrusikesh Panda Date: Sun, 19 Jan 2020 17:41:14 -0500 Subject: [PATCH 1/2] fix: Backspace doesn't raise onTagRemove event --- src/index.js | 3 +-- src/tree-manager/keyboardNavigation.js | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/index.js b/src/index.js index 28a9c279..367d8210 100644 --- a/src/index.js +++ b/src/index.js @@ -256,8 +256,7 @@ class DropdownTreeSelect extends Component { } return } else if (e.key === 'Backspace' && tags.length && this.searchInput.value.length === 0) { - const lastTag = tags.pop() - this.onCheckboxChange(lastTag._id, false) + this.onTagRemove(tags[tags.length - 1]._id, true) } else { return } diff --git a/src/tree-manager/keyboardNavigation.js b/src/tree-manager/keyboardNavigation.js index a41d9c2d..8120f71c 100644 --- a/src/tree-manager/keyboardNavigation.js +++ b/src/tree-manager/keyboardNavigation.js @@ -1,5 +1,4 @@ import nodeVisitor from './nodeVisitor' -import { getTagId } from '../tag' const Keys = { Up: 'ArrowUp', @@ -150,8 +149,7 @@ const getNextFocusAfterTagDelete = (deletedId, prevTags, tags, fallback) => { if (index < 0 || !tags.length) return fallback index = tags.length > index ? index : tags.length - 1 - const newFocusId = tags[index]._id - const focusNode = document.getElementById(getTagId(newFocusId)) + const focusNode = document.getElementById(tags[index].getDOMId('tag')) if (focusNode) { return focusNode.firstElementChild || fallback } From a100034f7fef62d9b1163596a57af2627e511d5b Mon Sep 17 00:00:00 2001 From: Hrusikesh Panda Date: Sun, 19 Jan 2020 17:48:27 -0500 Subject: [PATCH 2/2] fix: Prefix instance id in case of custom node id Fixes #315 --- src/tag/index.js | 9 ++---- src/tags/index.js | 3 +- src/tree-manager/flatten-tree.js | 14 +++------- src/tree-manager/node.js | 48 ++++++++++++++++++++++++++++++++ src/trigger/index.js | 3 +- 5 files changed, 58 insertions(+), 19 deletions(-) create mode 100644 src/tree-manager/node.js diff --git a/src/tag/index.js b/src/tag/index.js index a0e9ce44..c5093a3e 100644 --- a/src/tag/index.js +++ b/src/tag/index.js @@ -3,8 +3,6 @@ import React, { PureComponent } from 'react' import './index.css' -export const getTagId = id => `${id}_tag` - class Tag extends PureComponent { static propTypes = { id: PropTypes.string.isRequired, @@ -37,10 +35,9 @@ class Tag extends PureComponent { } render() { - const { id, label, labelRemove = 'Remove', readOnly, disabled } = this.props - - const tagId = getTagId(id) - const buttonId = `${id}_button` + const { label, labelRemove = 'Remove', readOnly, disabled, getDOMId } = this.props + const tagId = getDOMId('tag') + const buttonId = getDOMId('button') const className = ['tag-remove', readOnly && 'readOnly', disabled && 'disabled'].filter(Boolean).join(' ') const isDisabled = readOnly || disabled diff --git a/src/tags/index.js b/src/tags/index.js index b467e67a..fb960261 100644 --- a/src/tags/index.js +++ b/src/tags/index.js @@ -7,7 +7,7 @@ import './index.css' const getTags = (tags = [], onDelete, readOnly, disabled, labelRemove) => tags.map(tag => { - const { _id, label, tagClassName, dataset } = tag + const { _id, label, tagClassName, dataset, getDOMId } = tag return (
  • readOnly={readOnly} disabled={disabled} labelRemove={labelRemove} + getDOMId={getDOMId} />
    • ) diff --git a/src/tree-manager/flatten-tree.js b/src/tree-manager/flatten-tree.js index b5c563d0..f579ada8 100644 --- a/src/tree-manager/flatten-tree.js +++ b/src/tree-manager/flatten-tree.js @@ -1,4 +1,5 @@ import getPartialState from './getPartialState' +import Node from './node' import { isEmpty } from '../utils' @@ -216,16 +217,8 @@ function walkNodes({ _rv = { list: new Map(), defaultValues: [], singleSelectedNode: null }, }) { const single = simple || radio - nodes.forEach((node, i) => { - node._depth = depth - - if (parent) { - node._id = node.id || `${parent._id}-${i}` - node._parent = parent._id - parent._children.push(node._id) - } else { - node._id = node.id || `${rootPrefixId ? `${rootPrefixId}-${i}` : i}` - } + nodes.forEach((n, i) => { + const node = new Node({ rootPrefixId, depth, parent, index: i, ...n }) if (single && node.checked) { if (_rv.singleSelectedNode) { @@ -261,6 +254,7 @@ function walkNodes({ radio, showPartialState, hierarchical, + rootPrefixId, _rv, }) diff --git a/src/tree-manager/node.js b/src/tree-manager/node.js new file mode 100644 index 00000000..826ab8c5 --- /dev/null +++ b/src/tree-manager/node.js @@ -0,0 +1,48 @@ +/** + * Represents a tree node (not DOM node) in the data tree. + * It can have the following properties: + * + * id {string|optional} User defined id, comes from `data` passed to the component. + * _id {string} Internal id, auto generated if `id` is not defined, otherwise set to `id`. + * rootPrefixId {string} The id of the component's instance. + * parent {Node} Parent node, if a child. + * label {string|required} Checkbox label + * value {string|required} Checkbox value + * children {array|optional} Array of child nodes + * checked {bool|optional} Initial state of checkbox. if true, checkbox is selected and corresponding pill is rendered. + * disabled {bool|optional} Selectable state of checkbox. if true, the checkbox is disabled and the node is not selectable. + * expanded {bool|optional} If true, the node is expanded (children of children nodes are not expanded by default unless children nodes also have expanded: true). + * className {string|optional} Additional css class for the node. This is helpful to style the nodes your way + * tagClassName {string|optional} Css class for the corresponding tag. Use this to add custom style the pill corresponding to the node. + * actions {array|optional} An array of extra action on the node (such as displaying an info icon or any custom icons/elements) + * dataset {object|optional} Allows data-* attributes to be set on the node and tag elements + * isDefaultValue {bool|optional} Indicate if a node is a default value. When true, the dropdown will automatically select the node(s) when there is no other selected node. Can be used on more than one node. + * + */ +export default class Node { + constructor({ depth, rootPrefixId, parent, index, ...dataProps }) { + // first copy all props coming from data + Object.assign(this, dataProps) + + // then assign basic ones + this._depth = depth + this.rootPrefixId = rootPrefixId + + if (parent) { + this._id = this.id || `${parent._id}-${index}` + this._parent = parent._id + parent._children.push(this._id) + } else { + this._id = this.id || `${rootPrefixId ? `${rootPrefixId}-${index}` : index}` + } + } + + /** + * Given an element, generate a DOM Id that's unique across instances + */ + getDOMId = element => { + // if user has defined id, then ensure it's unique across instances + if (this.id) return `${this.rootPrefixId}_${this.id}_${element}` + return `${this._id}_${element}` + } +} diff --git a/src/trigger/index.js b/src/trigger/index.js index ddead5c6..2d13deca 100644 --- a/src/trigger/index.js +++ b/src/trigger/index.js @@ -2,7 +2,6 @@ import React, { PureComponent } from 'react' import PropTypes from 'prop-types' import { getAriaLabel } from '../a11y' -import { getTagId } from '../tag' class Trigger extends PureComponent { static propTypes = { @@ -28,7 +27,7 @@ class Trigger extends PureComponent { labelledBy.push(triggerId) } tags.forEach(t => { - labelledBy.push(getTagId(t._id)) + labelledBy.push(t.getDOMId('tag')) }) labelAttributes = getAriaLabel(texts.label, labelledBy.join(' ')) }