fix(Tag): ignore click events when disabled (#4792)

* fix(Tag): ignore click events when disabled * test(Tag): remove warning * fix(tag): move aria-label to button * fix: add ellipses when width is constrained * docs: fix example markup * fix(tag): reintroduce padding due to recent markup change * fix(Tag): remove unneeded click handler logic * fix(tag): use disabled token Co-authored-by: Josh Black <[email protected]>
carbon-design-system · Jan 13, 2020 · 0188730 · 0188730
1 parent be3a798
commit 0188730
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 24 deletions.
diff --git a/packages/components/src/components/tag/_tag.scss b/packages/components/src/components/tag/_tag.scss
@@ -76,7 +76,8 @@
     @include tag-theme($ibm-color__warm-gray-20, $ibm-color__warm-gray-100);
   }
 
-  .#{$prefix}--tag--disabled {
+  .#{$prefix}--tag--disabled,
+  .#{$prefix}--tag--filter.#{$prefix}--tag--disabled {
     @include tag-theme($ibm-color__gray-10, $ibm-color__gray-30);
 
     &:hover {
@@ -97,31 +98,42 @@
 
     cursor: pointer;
     padding-right: rem(2px);
+
+    &:focus,
+    &:hover {
+      outline: none;
+    }
   }
 
   .#{$prefix}--tag--filter > svg {
-    fill: $inverse-01;
-    margin-left: rem(4px);
-    padding: rem(2px);
+    flex-shrink: 0;
     width: rem(20px);
     height: rem(20px);
-  }
-
-  .#{$prefix}--tag--filter > svg:hover {
+    margin: 0 0 0 rem(4px);
+    padding: rem(2px);
+    border: 0;
+    fill: $inverse-01;
+    background-color: transparent;
     border-radius: 50%;
-    background-color: $inverse-hover-ui;
-  }
 
-  .#{$prefix}--tag--filter:focus,
-  .#{$prefix}--tag--filter:hover {
-    outline: none;
+    &:hover {
+      background-color: $inverse-hover-ui;
+    }
   }
 
   .#{$prefix}--tag--filter:focus > svg {
     box-shadow: inset 0 0 0 2px $inverse-focus-ui;
     border-radius: 50%;
   }
 
+  .#{$prefix}--tag--filter.#{$prefix}--tag--disabled svg:hover {
+    background-color: transparent;
+  }
+
+  .#{$prefix}--tag--filter.#{$prefix}--tag--disabled svg {
+    fill: $disabled-02;
+  }
+
   // Skeleton state
   .#{$prefix}--tag.#{$prefix}--skeleton {
     @include skeleton;

diff --git a/packages/components/src/components/tag/tag.hbs b/packages/components/src/components/tag/tag.hbs
@@ -5,19 +5,19 @@
   LICENSE file in the root directory of this source tree.
 -->
 
-<div role="list">
+<div>
   {{#each tags}}
-  <span class="{{@root.prefix}}--tag {{@root.prefix}}--tag--{{type}}" role="listitem">
+  <button class="{{@root.prefix}}--tag {{@root.prefix}}--tag--{{type}}">
     <span class="{{@root.prefix}}--tag__label">{{label}}</span>
-  </span>
+  </button>
   {{/each}}
 </div>
 
-<div role="list">
+<div>
   {{#if filter}}
-  <span class="{{@root.prefix}}--tag {{@root.prefix}}--tag--filter" title="Clear filter" tabindex="0" role="listitem">
+  <button class="{{@root.prefix}}--tag {{@root.prefix}}--tag--filter" title="Clear filter">
     <span class="{{@root.prefix}}--tag__label">filter</span>
-    {{ carbon-icon 'Close16' aria-label='Clear filter' }}
-  </span>
+    {{ carbon-icon 'Close16' }}
+  </button>
   {{/if}}
 </div>
diff --git a/packages/react/src/components/Tag/Tag-test.js b/packages/react/src/components/Tag/Tag-test.js
@@ -49,19 +49,19 @@ describe('Tag', () => {
 
   describe('Renders as expected', () => {
     it('should render with the appropriate type', () => {
-      const tag = shallow(<Tag type="beta" />);
+      const tag = shallow(<Tag type="red" />);
       expect(tag.hasClass(`${prefix}--tag`)).toEqual(true);
-      expect(tag.hasClass(`${prefix}--tag--beta`)).toEqual(true);
+      expect(tag.hasClass(`${prefix}--tag--red`)).toEqual(true);
     });
   });
 
   it('should allow for a custom label', () => {
-    const tag = shallow(<Tag type="beta">New Version!</Tag>);
+    const tag = shallow(<Tag type="red">New Version!</Tag>);
     expect(tag.text()).toEqual('New Version!');
   });
 
   it('should support extra class names', () => {
-    const tag = shallow(<Tag type="beta" className="extra-class" />);
+    const tag = shallow(<Tag type="red" className="extra-class" />);
     expect(tag.hasClass('extra-class')).toEqual(true);
   });
 });

diff --git a/packages/react/src/components/Tag/Tag.js b/packages/react/src/components/Tag/Tag.js
@@ -48,8 +48,11 @@ const Tag = ({
           ? `${title} ${children}`
           : `Clear filter ${children}`
       }
+      disabled={disabled}
       {...other}>
-      {children !== null && children !== undefined ? children : TYPES[type]}
+      <span className={`${prefix}--tag__label`}>
+        {children !== null && children !== undefined ? children : TYPES[type]}
+      </span>
       <Close16 />
     </button>
   ) : (