From a6d4973260ff06e38a3705c5fd350779f65ae54a Mon Sep 17 00:00:00 2001
From: Jan Hassel <jan.hassel@ibm.com>
Date: Mon, 16 Mar 2020 17:21:13 +0100
Subject: [PATCH] fix(button): prevent icon and tooltip jumping for tertiary
 button (#5618)

* fix(button): prevent jump of icon in tertiary button

* fix(button): simplify button stylings

This addresses the following problems:
- jumping tooltip on icon-only tertiary button
- red border color for focused danger button when hovering

* fix(button): correct box-shadows

* fix: deprecate unused button-outline-offset and button-outline tokens

* fix(button): correct danger button hover and icon-only svg fill

Co-authored-by: TJ Egan <tw15egan@gmail.com>
Co-authored-by: Josh Black <josh@josh.black>
---
 packages/components/docs/sass.md              | 59 ++-----------------
 .../src/components/button/_button.scss        | 58 +++---------------
 .../src/components/button/_mixins.scss        |  7 ++-
 .../src/globals/scss/_theme-tokens.scss       |  4 +-
 4 files changed, 22 insertions(+), 106 deletions(-)

diff --git a/packages/components/docs/sass.md b/packages/components/docs/sass.md
index 8069f52d32bd..ca15f19c1afb 100644
--- a/packages/components/docs/sass.md
+++ b/packages/components/docs/sass.md
@@ -6203,7 +6203,6 @@ $ui-02: if(
 - **Type**: `{undefined}`
 - **Used by**:
   - [carbon--theme [mixin]](#carbon--theme-mixin)
-  - [button [mixin]](#button-mixin)
   - [button-theme [mixin]](#button-theme-mixin)
   - [snippet [mixin]](#snippet-mixin)
   - [loading [mixin]](#loading-mixin)
@@ -12964,8 +12963,7 @@ Button styles
       $interactive-03,
       $hover-tertiary,
       currentColor,
-      $active-tertiary,
-      1px
+      $active-tertiary
     );
 
     &:hover {
@@ -12993,19 +12991,6 @@ Button styles
     }
   }
 
-  .#{$prefix}--btn--tertiary,
-  .#{$prefix}--btn--tertiary.#{$prefix}--btn--field,
-  .#{$prefix}--btn--tertiary.#{$prefix}--btn--sm {
-    padding-right: rem(62px);
-    padding-left: rem(14px);
-
-    &:focus {
-      border-width: rem(3px);
-      padding-right: rem(60px);
-      padding-left: rem(12px);
-    }
-  }
-
   .#{$prefix}--btn--ghost {
     @include button-theme(
       transparent,
@@ -13058,7 +13043,6 @@ Button styles
 
   .#{$prefix}--btn.#{$prefix}--btn--icon-only.#{$prefix}--tooltip__trigger:focus {
     border-color: $focus;
-    box-shadow: inset 0 0 0 $button-outline-width $ui-02;
   }
 
   .#{$prefix}--btn.#{$prefix}--btn--icon-only.#{$prefix}--tooltip__trigger:focus
@@ -13076,8 +13060,8 @@ Button styles
   }
 
   .#{$prefix}--btn--icon-only {
-    padding-left: rem(13px);
-    padding-right: rem(13px);
+    padding-left: rem(15px);
+    padding-right: rem(15px);
 
     .#{$prefix}--btn__icon {
       position: static;
@@ -13113,44 +13097,14 @@ Button styles
     display: none;
   }
 
-  .#{$prefix}--btn--icon-only.#{$prefix}--btn--tertiary {
-    padding-left: rem(15px);
-    padding-right: rem(15px);
-
-    &:focus {
-      padding-left: rem(13px);
-      padding-right: rem(13px);
-    }
-  }
-
   .#{$prefix}--btn--field.#{$prefix}--btn--icon-only {
-    padding-left: rem(9px);
-    padding-right: rem(9px);
-  }
-
-  .#{$prefix}--btn--field.#{$prefix}--btn--icon-only.#{$prefix}--btn--tertiary {
     padding-left: rem(11px);
     padding-right: rem(11px);
-
-    &:focus {
-      padding-left: rem(9px);
-      padding-right: rem(9px);
-    }
   }
 
   .#{$prefix}--btn--sm.#{$prefix}--btn--icon-only {
-    padding-left: rem(5px);
-    padding-right: rem(5px);
-  }
-
-  .#{$prefix}--btn--sm.#{$prefix}--btn--icon-only.#{$prefix}--btn--tertiary {
     padding-left: rem(7px);
     padding-right: rem(7px);
-
-    &:focus {
-      padding-left: rem(5px);
-      padding-right: rem(5px);
-    }
   }
 
   .#{$prefix}--btn--danger {
@@ -13165,7 +13119,6 @@ Button styles
 
     &:hover {
       color: $text-04;
-      border: $button-border-width solid transparent;
     }
   }
 
@@ -13212,7 +13165,6 @@ Button styles
   - [carbon--spacing-03 [variable]](#carbon--spacing-03-variable)
   - [hover-primary-text [variable]](#hover-primary-text-variable)
   - [focus [variable]](#focus-variable)
-  - [ui-02 [variable]](#ui-02-variable)
   - [icon-01 [variable]](#icon-01-variable)
   - [disabled-02 [variable]](#disabled-02-variable)
   - [danger [variable]](#danger-variable)
@@ -13287,7 +13239,7 @@ Button variant styles
 ```scss
 @mixin button-theme() {
   background-color: $bg-color;
-  border-width: $border-width;
+  border-width: $button-outline-width;
   border-style: solid;
   border-color: $border-color;
   color: $font-color;
@@ -13298,7 +13250,8 @@ Button variant styles
 
   &:focus {
     border-color: $focus;
-    box-shadow: inset 0 0 0 $button-outline-width $ui-02;
+    box-shadow: inset 0 0 0 $button-border-width $focus, inset 0 0 0
+        ($button-border-width + $button-outline-width) $ui-02;
   }
 
   &:disabled:hover,
diff --git a/packages/components/src/components/button/_button.scss b/packages/components/src/components/button/_button.scss
index 8b35494ac567..e19c2c7d91ce 100644
--- a/packages/components/src/components/button/_button.scss
+++ b/packages/components/src/components/button/_button.scss
@@ -84,7 +84,6 @@
       $hover-tertiary,
       currentColor,
       $active-tertiary,
-      1px
     );
 
     &:hover {
@@ -112,19 +111,6 @@
     }
   }
 
-  .#{$prefix}--btn--tertiary,
-  .#{$prefix}--btn--tertiary.#{$prefix}--btn--field,
-  .#{$prefix}--btn--tertiary.#{$prefix}--btn--sm {
-    padding-right: rem(62px);
-    padding-left: rem(14px);
-
-    &:focus {
-      border-width: rem(3px);
-      padding-right: rem(60px);
-      padding-left: rem(12px);
-    }
-  }
-
   .#{$prefix}--btn--ghost {
     @include button-theme(
       transparent,
@@ -173,11 +159,16 @@
 
   .#{$prefix}--btn.#{$prefix}--btn--icon-only.#{$prefix}--tooltip__trigger {
     @include tooltip--trigger('icon', 'bottom');
+
+    svg,
+    &:hover svg,
+    &:focus svg {
+      fill: currentColor;
+    }
   }
 
   .#{$prefix}--btn.#{$prefix}--btn--icon-only.#{$prefix}--tooltip__trigger:focus {
     border-color: $focus;
-    box-shadow: inset 0 0 0 $button-outline-width $ui-02;
   }
 
   .#{$prefix}--btn.#{$prefix}--btn--icon-only.#{$prefix}--tooltip__trigger:focus
@@ -195,8 +186,8 @@
   }
 
   .#{$prefix}--btn--icon-only {
-    padding-left: rem(13px);
-    padding-right: rem(13px);
+    padding-left: rem(15px);
+    padding-right: rem(15px);
 
     .#{$prefix}--btn__icon {
       position: static;
@@ -232,50 +223,20 @@
     display: none;
   }
 
-  .#{$prefix}--btn--icon-only.#{$prefix}--btn--tertiary {
-    padding-left: rem(15px);
-    padding-right: rem(15px);
-
-    &:focus {
-      padding-left: rem(13px);
-      padding-right: rem(13px);
-    }
-  }
-
   .#{$prefix}--btn--field.#{$prefix}--btn--icon-only {
-    padding-left: rem(9px);
-    padding-right: rem(9px);
-  }
-
-  .#{$prefix}--btn--field.#{$prefix}--btn--icon-only.#{$prefix}--btn--tertiary {
     padding-left: rem(11px);
     padding-right: rem(11px);
-
-    &:focus {
-      padding-left: rem(9px);
-      padding-right: rem(9px);
-    }
   }
 
   .#{$prefix}--btn--sm.#{$prefix}--btn--icon-only {
-    padding-left: rem(5px);
-    padding-right: rem(5px);
-  }
-
-  .#{$prefix}--btn--sm.#{$prefix}--btn--icon-only.#{$prefix}--btn--tertiary {
     padding-left: rem(7px);
     padding-right: rem(7px);
-
-    &:focus {
-      padding-left: rem(5px);
-      padding-right: rem(5px);
-    }
   }
 
   .#{$prefix}--btn--danger {
     @include button-theme(
       $danger,
-      $danger,
+      transparent,
       $text-04,
       $hover-danger,
       currentColor,
@@ -284,7 +245,6 @@
 
     &:hover {
       color: $text-04;
-      border: $button-border-width solid transparent;
     }
   }
 
diff --git a/packages/components/src/components/button/_mixins.scss b/packages/components/src/components/button/_mixins.scss
index c97aea4c4cec..f6badb662162 100644
--- a/packages/components/src/components/button/_mixins.scss
+++ b/packages/components/src/components/button/_mixins.scss
@@ -59,10 +59,9 @@
   $hover-bg-color,
   $icon-color,
   $active-color,
-  $border-width: $button-border-width
 ) {
   background-color: $bg-color;
-  border-width: $border-width;
+  border-width: $button-outline-width;
   border-style: solid;
   border-color: $border-color;
   color: $font-color;
@@ -73,7 +72,9 @@
 
   &:focus {
     border-color: $focus;
-    box-shadow: inset 0 0 0 $button-outline-width $ui-02;
+    box-shadow:
+    inset 0 0 0 $button-border-width $focus,
+    inset 0 0 0 ($button-border-width + $button-outline-width) $ui-02;
   }
 
   &:disabled:hover,
diff --git a/packages/components/src/globals/scss/_theme-tokens.scss b/packages/components/src/globals/scss/_theme-tokens.scss
index c55d70a1db97..b4fd144515ab 100644
--- a/packages/components/src/globals/scss/_theme-tokens.scss
+++ b/packages/components/src/globals/scss/_theme-tokens.scss
@@ -131,7 +131,7 @@ $button-padding-ghost-sm: calc(0.375rem - 3px) 12px !default;
 /// @type Number
 /// @access public
 /// @group button
-$button-border-width: 3px !default;
+$button-border-width: 2px !default;
 
 /// @type Number
 /// @access public
@@ -141,11 +141,13 @@ $button-outline-width: 1px !default;
 /// @type Number
 /// @access public
 /// @group button
+/// @deprecated
 $button-outline-offset: -5px !default;
 
 /// @type Value
 /// @access public
 /// @group button
+/// @deprecated
 $button-outline: 1px solid $ibm-color__white-0 !default;
 
 // Accordion