From 3c96df0d80e14c8bd8a23f4fbfa898138c4ea682 Mon Sep 17 00:00:00 2001
From: Zeus Courtois <zeusorion@yahoo.com>
Date: Tue, 26 Mar 2019 17:19:23 -0500
Subject: [PATCH] fix(tags): a11y, editable on hover (#75)

---
 src/components/TagList/TagList-story.js |  8 ++++-
 src/components/TagList/TagList-test.js  | 16 ---------
 src/components/TagList/TagList.js       | 47 ++++++-------------------
 3 files changed, 18 insertions(+), 53 deletions(-)

diff --git a/src/components/TagList/TagList-story.js b/src/components/TagList/TagList-story.js
index a590f1b..d7c5322 100644
--- a/src/components/TagList/TagList-story.js
+++ b/src/components/TagList/TagList-story.js
@@ -128,7 +128,13 @@ storiesOf('TagList', module)
     A TagList is used to manage multiple tags at once. The example below shows how the TagList component can be used to condense a list.
     Custom Tag properties have also been applied.
   `,
-    () => <TagList {...tagListEventsWithTagProps} numTagsDisplayed={2} />
+    () => (
+      <TagList
+        {...tagListEventsWithTagProps}
+        numTagsDisplayed={2}
+        isEditable="always"
+      />
+    )
   )
   .addWithInfo(
     'Default with Class Applied to all Tags',
diff --git a/src/components/TagList/TagList-test.js b/src/components/TagList/TagList-test.js
index 729a033..c73d642 100644
--- a/src/components/TagList/TagList-test.js
+++ b/src/components/TagList/TagList-test.js
@@ -68,22 +68,6 @@ describe('TagList', () => {
     expect(wrapper.find('.bx--tag-list-editable')).toHaveLength(1);
   });
 
-  it('should display edit state only on hover when isEditable is on-hover', () => {
-    mockProps = {
-      ...mockProps,
-      isEditable: 'on-hover',
-      onIconClick: onIconClickMock,
-    };
-
-    const wrapper = shallow(<TagList {...mockProps} />);
-    expect(wrapper.find(Icon)).toHaveLength(0);
-    wrapper.find('div').simulate('mouseenter');
-    expect(wrapper.find(Icon)).toHaveLength(1);
-    wrapper.find(Icon).simulate('click');
-    expect(onIconClickMock).toHaveBeenCalled;
-    expect(wrapper.find('.bx--tag-list-editable')).toHaveLength(1);
-  });
-
   it('should limit characters when passing maxCharacters through otherProps', () => {
     mockProps = {
       tags: [
diff --git a/src/components/TagList/TagList.js b/src/components/TagList/TagList.js
index 50a0ae4..79a49e8 100644
--- a/src/components/TagList/TagList.js
+++ b/src/components/TagList/TagList.js
@@ -6,10 +6,6 @@ import { Icon, Tooltip } from 'carbon-components-react';
 import Tag from '../Tag';
 
 export default class TagList extends Component {
-  state = {
-    showEditIcon: false,
-  };
-
   static propTypes = {
     numTagsDisplayed: PropTypes.number.isRequired,
     tags: PropTypes.arrayOf(
@@ -43,18 +39,6 @@ export default class TagList extends Component {
     maxTagsTooltip: 8,
   };
 
-  toggleEditIconShow = () => {
-    this.setState({
-      showEditIcon: true,
-    });
-  };
-
-  toggleEditIconHide = () => {
-    this.setState({
-      showEditIcon: false,
-    });
-  };
-
   handleOnIconClick = evt => {
     evt.stopPropagation();
     if (this.props.onIconClick) this.props.onIconClick();
@@ -167,21 +151,22 @@ export default class TagList extends Component {
       className
     );
 
+    const editButtonClasses = classNames({
+      'bx--tag-list--edit--button': true,
+      'always-editable': isEditable === 'always',
+      'never-editable': isEditable === 'never',
+    });
+
     return (
       <div
         className={tagListClassNames}
         onClick={this.handleOnIconClick}
-        {...rest}
-        onMouseEnter={
-          isEditable === 'on-hover' ? this.toggleEditIconShow : undefined
-        }
-        onMouseLeave={
-          isEditable === 'on-hover' ? this.toggleEditIconHide : undefined
-        }>
+        {...rest}>
         {displayList.map(tag => (
           <Tag
             key={tag.name}
             className="bx--tag-list--tag"
+            tabIndex="0"
             type={tag.type}
             title={tag.name}
             maxCharacters={maxCharacters}
@@ -190,8 +175,8 @@ export default class TagList extends Component {
           </Tag>
         ))}
         {this.overflowNode()}
-        {isEditable === 'always' && (
-          <button className="bx--tag-list--edit--button">
+        {
+          <button className={editButtonClasses}>
             <Icon
               name="edit--glyph"
               className="bx--tag-list--edit--icon"
@@ -199,17 +184,7 @@ export default class TagList extends Component {
               description="click to edit tags"
             />
           </button>
-        )}
-        {isEditable === 'on-hover' && this.state.showEditIcon && (
-          <button className="bx--tag-list--edit--button">
-            <Icon
-              name="edit--glyph"
-              className="bx--tag-list--edit--icon"
-              title="edit icon"
-              description="click to edit tags"
-            />
-          </button>
-        )}
+        }
       </div>
     );
   }