fix: Append clientId to tag, remove button to make them unique

Currently, having multiple component instances on same page renders same remove-button, tag ids.

Fixes #315

Author: mrchief

__snapshots__/src/tag/index.test.js.md

@@ -11,14 +11,14 @@ Generated by [AVA](https://ava.li).
     <span
       "aria-label"="hello"
       className="tag"
-      id="abc_tag"
+      id="rdts_abc_tag"
     >
       hello
       <button
         "aria-label"="Remove"
-        "aria-labelledby"="abc_button abc_tag"
+        "aria-labelledby"="rdts_abc_button rdts_abc_tag"
         className="tag-remove"
-        id="abc_button"
+        id="rdts_abc_button"
         onClick={Function {}}
         onKeyDown={Function {}}
         onKeyUp={Function {}}

__snapshots__/src/tag/index.test.js.snap

@@ -153,11 +153,15 @@ class DropdownTreeSelect extends Component {
   }
 
   onTagRemove = (id, isKeyboardEvent) => {
-    const { tags: prevTags } = this.state
+    const {
+      clientId,
+      searchInput,
+      state: { tags: prevTags },
+    } = this
     this.onCheckboxChange(id, false, tags => {
       if (!isKeyboardEvent) return
 
-      keyboardNavigation.getNextFocusAfterTagDelete(id, prevTags, tags, this.searchInput).focus()
+      keyboardNavigation.getNextFocusAfterTagDelete({ id, prevTags, tags, fallback: searchInput, clientId }).focus()
     })
   }
 

src/index.js

@@ -134,15 +134,15 @@ test('can select with enter on keyboardNavigation', t => {
 
 test('can delete tags with backspace/delete on keyboardNavigation', t => {
   const data = [{ ...node('a', 'a'), checked: true }, { ...node('b', 'b'), checked: true }]
-  const wrapper = mount(<DropdownTreeSelect data={data} />)
+  const wrapper = mount(<DropdownTreeSelect id="rdts" data={data} />)
   const event = {
     type: 'keydown',
     stopPropagation: spy(),
     nativeEvent: { stopImmediatePropagation: spy() },
   }
-  wrapper.find('#a_tag > .tag-remove').simulate('keyDown', { ...event, key: 'Backspace' })
+  wrapper.find('#rdts_a_tag > .tag-remove').simulate('keyDown', { ...event, key: 'Backspace' })
   t.deepEqual(wrapper.state().tags.length, 1)
-  wrapper.find('#b_tag > .tag-remove').simulate('keyUp', { ...event, key: 'Delete' })
+  wrapper.find('#rdts_b_tag > .tag-remove').simulate('keyUp', { ...event, key: 'Delete' })
   t.deepEqual(wrapper.state().tags.length, 0)
 })
 

src/index.keyboardNav.test.js

@@ -301,14 +301,14 @@ test('appends selected tags to aria-labelledby with provided aria-labelledby', t
   const { tree } = t.context
   tree[0].checked = true
   const wrapper = mount(<DropdownTreeSelect id="rdts" data={tree} texts={{ label: '#hello #world' }} />)
-  t.deepEqual(wrapper.find('.dropdown-trigger').prop('aria-labelledby'), 'hello world rdts-0_tag')
+  t.deepEqual(wrapper.find('.dropdown-trigger').prop('aria-labelledby'), 'hello world rdts_rdts-0_tag')
   t.deepEqual(wrapper.find('.dropdown-trigger').prop('aria-label'), undefined)
 })
 
 test('appends selected tags to aria-labelledby with text label', t => {
   const { tree } = t.context
   tree[0].checked = true
   const wrapper = mount(<DropdownTreeSelect id="rdts" data={tree} texts={{ label: 'hello world' }} />)
-  t.deepEqual(wrapper.find('.dropdown-trigger').prop('aria-labelledby'), 'rdts_trigger rdts-0_tag')
+  t.deepEqual(wrapper.find('.dropdown-trigger').prop('aria-labelledby'), 'rdts_trigger rdts_rdts-0_tag')
   t.deepEqual(wrapper.find('.dropdown-trigger').prop('aria-label'), 'hello world')
 })

src/index.test.js

@@ -3,11 +3,12 @@ import React, { PureComponent } from 'react'
 
 import './index.css'
 
-export const getTagId = id => `${id}_tag`
+export const getTagId = ({ id, clientId }) => `${clientId}_${id}_tag`
 
 class Tag extends PureComponent {
   static propTypes = {
     id: PropTypes.string.isRequired,
+    clientId: PropTypes.string.isRequired,
     label: PropTypes.string.isRequired,
     onDelete: PropTypes.func,
     readOnly: PropTypes.bool,
@@ -37,10 +38,10 @@ class Tag extends PureComponent {
   }
 
   render() {
-    const { id, label, labelRemove = 'Remove', readOnly, disabled } = this.props
+    const { id, label, labelRemove = 'Remove', readOnly, disabled, clientId } = this.props
 
-    const tagId = getTagId(id)
-    const buttonId = `${id}_button`
+    const tagId = getTagId({ id, clientId })
+    const buttonId = `${clientId}_${id}_button`
     const className = ['tag-remove', readOnly && 'readOnly', disabled && 'disabled'].filter(Boolean).join(' ')
     const isDisabled = readOnly || disabled
 

src/tag/index.js

@@ -9,13 +9,13 @@ import Tag from './index'
 const mockEvent = { nativeEvent: { stopImmediatePropagation: spy() } }
 
 test('renders label when passed in', t => {
-  const wrapper = toJson(shallow(<Tag label="hello" id="abc" />))
+  const wrapper = toJson(shallow(<Tag label="hello" clientId="rdts" id="abc" />))
   t.snapshot(wrapper)
 })
 
 test('call stopPropagation and stopImmediatePropagation when pill is closed', t => {
   const onDelete = spy()
-  const wrapper = mount(<Tag label="hello" id="abc" onDelete={onDelete} />)
+  const wrapper = mount(<Tag label="hello" clientId="rdts" id="abc" onDelete={onDelete} />)
   const event = {
     type: 'click',
     stopPropagation: spy(),
@@ -28,21 +28,21 @@ test('call stopPropagation and stopImmediatePropagation when pill is closed', t
 
 test('call onDelete handler when pill is closed', t => {
   const onDelete = spy()
-  const wrapper = mount(<Tag label="hello" id="abc" onDelete={onDelete} />)
+  const wrapper = mount(<Tag label="hello" clientId="rdts" id="abc" onDelete={onDelete} />)
   wrapper.find('.tag-remove').simulate('click', mockEvent)
   t.true(onDelete.calledWith('abc'))
 })
 
 test('should not call onDelete when readOnly', t => {
   const onDelete = spy()
-  const wrapper = mount(<Tag label="hello" id="abc" onDelete={onDelete} readOnly />)
+  const wrapper = mount(<Tag label="hello" clientId="rdts" id="abc" onDelete={onDelete} readOnly />)
   wrapper.find('.tag-remove').simulate('click', mockEvent)
   t.true(onDelete.notCalled)
 })
 
 test('should not call onDelete when disabled', t => {
   const onDelete = spy()
-  const wrapper = mount(<Tag label="hello" id="abc" onDelete={onDelete} disabled />)
+  const wrapper = mount(<Tag label="hello" clientId="rdts" id="abc" onDelete={onDelete} disabled />)
   wrapper.find('.tag-remove').simulate('click', mockEvent)
   t.true(onDelete.notCalled)
 })
@@ -52,7 +52,7 @@ test('should not cause form submit', t => {
   const onDelete = spy()
   const wrapper = mount(
     <form onSubmit={onSubmit}>
-      <Tag label="hello" id="abc" onDelete={onDelete} />
+      <Tag label="hello" clientId="rdts" id="abc" onDelete={onDelete} />
     </form>
   )
   wrapper.find('.tag-remove').simulate('click', mockEvent)

src/tag/index.test.js

@@ -5,7 +5,7 @@ import { getDataset } from '../utils'
 
 import './index.css'
 
-const getTags = (tags = [], onDelete, readOnly, disabled, labelRemove) =>
+const getTags = (tags = [], onDelete, readOnly, disabled, labelRemove, clientId) =>
   tags.map(tag => {
     const { _id, label, tagClassName, dataset } = tag
     return (
@@ -17,6 +17,7 @@ const getTags = (tags = [], onDelete, readOnly, disabled, labelRemove) =>
         <Tag
           label={label}
           id={_id}
+          clientId={clientId}
           onDelete={onDelete}
           readOnly={readOnly}
           disabled={disabled}
@@ -34,15 +35,16 @@ class Tags extends PureComponent {
     disabled: PropTypes.bool,
     texts: PropTypes.object,
     children: PropTypes.node,
+    clientId: PropTypes.string.isRequired,
   }
 
   render() {
-    const { tags, onTagRemove, texts = {}, disabled, readOnly, children } = this.props
+    const { tags, onTagRemove, texts = {}, disabled, readOnly, children, clientId } = this.props
     const lastItem = children || <span className="placeholder">{texts.placeholder || 'Choose...'}</span>
 
     return (
       <ul className="tag-list">
-        {getTags(tags, onTagRemove, readOnly, disabled, texts.labelRemove)}
+        {getTags(tags, onTagRemove, readOnly, disabled, texts.labelRemove, clientId)}
         <li className="tag-item">{lastItem}</li>
       </ul>
     )

src/tags/index.js

@@ -144,14 +144,14 @@ const getNextFocus = (tree, prevFocus, action, getNodeById, markSubTreeOnNonExpa
   return getRelativeFocus(nodes, prevFocus, action)
 }
 
-const getNextFocusAfterTagDelete = (deletedId, prevTags, tags, fallback) => {
+const getNextFocusAfterTagDelete = ({ id, prevTags, tags, fallback, clientId }) => {
   // Sets new focus to next tag or returns fallback
-  let index = prevTags && prevTags.findIndex(t => t._id === deletedId)
+  let index = prevTags && prevTags.findIndex(t => t._id === id)
   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(getTagId({ clientId, id: newFocusId }))
   if (focusNode) {
     return focusNode.firstElementChild || fallback
   }

src/tree-manager/keyboardNavigation.js

@@ -28,7 +28,7 @@ class Trigger extends PureComponent {
         labelledBy.push(triggerId)
       }
       tags.forEach(t => {
-        labelledBy.push(getTagId(t._id))
+        labelledBy.push(getTagId({ clientId, id: t._id }))
       })
       labelAttributes = getAriaLabel(texts.label, labelledBy.join(' '))
     }