[Tag] Improve outline behavior (#2428)

* remove focus outline * add tabIndex for focus on interactive tags * add custom outline styles for tag input tags * use focus outline mixin instead of custom box shadow * fix view source margin bottom by adding a heading * use tabIndex from props if given * more comments * address more comments
palantir · May 16, 2018 · 162d85b · 162d85b · blueprint-bot · May 16, 2018
1 parent f592a8c
commit 162d85b
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 20 deletions.
diff --git a/packages/core/src/common/_color-aliases.scss b/packages/core/src/common/_color-aliases.scss
@@ -9,6 +9,8 @@ $pt-intent-danger: $red3 !default;
 $pt-app-background-color: $light-gray5 !default;
 $pt-dark-app-background-color: $dark-gray3 !default;
 
+$pt-outline-color: rgba($blue3, 0.6);
+
 $pt-text-color: $dark-gray1 !default;
 $pt-text-color-muted: $gray1 !default;
 $pt-text-color-disabled: rgba($pt-text-color-muted, 0.5) !default;

diff --git a/packages/core/src/common/_mixins.scss b/packages/core/src/common/_mixins.scss
@@ -69,7 +69,7 @@ $pt-dark-intent-text-colors: (
 }
 
 @mixin focus-outline($offset: 2px) {
-  outline: rgba($blue3, 0.5) auto 2px;
+  outline: $pt-outline-color auto 2px;
   outline-offset: $offset;
   -moz-outline-radius: 6px;
 }

diff --git a/packages/core/src/components/tag-input/_tag-input.scss b/packages/core/src/components/tag-input/_tag-input.scss
@@ -5,10 +5,10 @@
 @import "../forms/common";
 @import "../tag/common";
 
-$ti-padding: ($pt-input-height - $tag-height) / 2;
+$tag-input-padding: ($pt-input-height - $tag-height) / 2 !default;
 
-$ti-icon-padding: ($pt-input-height - $pt-icon-size-standard) / 2;
-$ti-icon-padding-large: ($pt-input-height-large - $pt-icon-size-large) / 2;
+$tag-input-icon-padding: ($pt-input-height - $pt-icon-size-standard) / 2 !default;
+$tag-input-icon-padding-large: ($pt-input-height-large - $pt-icon-size-large) / 2 !default;
 
 .#{$ns}-tag-input {
   @include pt-flex-container(row, $fill: ".#{$ns}-tag-input-values");
@@ -17,34 +17,38 @@ $ti-icon-padding-large: ($pt-input-height-large - $pt-icon-size-large) / 2;
   height: auto;
   min-height: $pt-input-height;
   padding-right: 0;
-  padding-left: $ti-padding;
+  padding-left: $tag-input-padding;
 
   .#{$ns}-tag-input-icon {
     // margins to center icon in one-line input
-    margin-top: $ti-icon-padding;
-    margin-right: $ti-icon-padding;
-    margin-left: $ti-icon-padding - $ti-padding;
+    margin-top: $tag-input-icon-padding;
+    margin-right: $tag-input-icon-padding;
+    margin-left: $tag-input-icon-padding - $tag-input-padding;
     color: $pt-icon-color;
   }
 
   .#{$ns}-tag-input-values {
-    @include pt-flex-container(row, $ti-padding);
+    @include pt-flex-container(row, $tag-input-padding);
     flex-wrap: wrap;
     align-items: center;
     // fill vertical height
     align-self: stretch;
-    margin-top: $ti-padding;
-    margin-right: $ti-icon-padding;
+    margin-top: $tag-input-padding;
+    margin-right: $tag-input-icon-padding;
 
     > * {
-      margin-bottom: $ti-padding;
+      margin-bottom: $tag-input-padding;
     }
   }
 
   .#{$ns}-tag {
     // NOTE: in order to wrap long words, you must set explicit width on TagInput,
     // or use .#{$ns}-fill CSS class modifier.
     overflow-wrap: break-word;
+
+    &.#{$ns}-active {
+      @include focus-outline(0);
+    }
   }
 
   .#{$ns}-input-ghost {
@@ -71,13 +75,13 @@ $ti-icon-padding-large: ($pt-input-height-large - $pt-icon-size-large) / 2;
   }
 
   &.#{$ns}-large {
-    @include pt-flex-margin(row, $ti-icon-padding-large);
+    @include pt-flex-margin(row, $tag-input-icon-padding-large);
     height: auto;
     min-height: $pt-input-height-large;
 
     .#{$ns}-tag-input-icon {
-      margin-top: $ti-icon-padding-large;
-      margin-left: $ti-icon-padding-large - $ti-padding;
+      margin-top: $tag-input-icon-padding-large;
+      margin-left: $tag-input-icon-padding-large - $tag-input-padding;
     }
 
     .#{$ns}-input-ghost {

diff --git a/packages/core/src/components/tag/_common.scss b/packages/core/src/components/tag/_common.scss
@@ -27,6 +27,10 @@ $tag-padding-large: ($tag-height-large - $pt-icon-size-large) / 2 !default;
   color: $pt-dark-text-color;
   font-size: $pt-font-size-small;
 
+  &:focus {
+    @include focus-outline(0);
+  }
+
   &.#{$ns}-round {
     border-radius: $tag-height / 2;
   }
@@ -41,10 +45,6 @@ $tag-padding-large: ($tag-height-large - $pt-icon-size-large) / 2 !default;
   &.#{$ns}-tag-removable {
     padding-right: $tag-height;
   }
-
-  &.#{$ns}-active {
-    @include focus-outline(0);
-  }
 }
 
 @mixin pt-tag-large() {
@@ -106,6 +106,7 @@ $tag-padding-large: ($tag-height-large - $pt-icon-size-large) / 2 !default;
       background-color: rgba($background-color, $opacity - 0.15);
     }
 
+    &.#{$ns}-active,
     &:active {
       background-color: rgba($background-color, $opacity - 0.3);
     }
@@ -120,6 +121,7 @@ $tag-padding-large: ($tag-height-large - $pt-icon-size-large) / 2 !default;
       background-color: rgba($background-color, $opacity + 0.1);
     }
 
+    &.#{$ns}-active,
     &:active {
       background-color: rgba($background-color, $opacity + 0.2);
     }

diff --git a/packages/core/src/components/tag/tag.tsx b/packages/core/src/components/tag/tag.tsx
@@ -64,6 +64,7 @@ export class Tag extends React.PureComponent<ITagProps, {}> {
             minimal,
             onRemove,
             round,
+            tabIndex = 0,
             ...htmlProps
         } = this.props;
         const isRemovable = Utils.isFunction(onRemove);
@@ -88,7 +89,7 @@ export class Tag extends React.PureComponent<ITagProps, {}> {
         ) : null;
 
         return (
-            <span {...htmlProps} className={tagClasses}>
+            <span {...htmlProps} className={tagClasses} tabIndex={interactive ? tabIndex : undefined}>
                 {children}
                 {removeButton}
             </span>