From e894d615a55016be63d7d899474987085df886f7 Mon Sep 17 00:00:00 2001
From: Chris Lewis <cmslewis@gmail.com>
Date: Mon, 3 Apr 2017 13:44:53 -0700
Subject: [PATCH] [Table] Refactor header menus to divorce click-target size
 from gradient size (#906)

---
 packages/table/src/common/classes.ts          |  2 +
 packages/table/src/headers/_headers.scss      | 80 ++++++++++++-------
 .../table/src/headers/columnHeaderCell.tsx    | 25 +++---
 3 files changed, 69 insertions(+), 38 deletions(-)

diff --git a/packages/table/src/common/classes.ts b/packages/table/src/common/classes.ts
index 247685df31..915e42a9a4 100644
--- a/packages/table/src/common/classes.ts
+++ b/packages/table/src/common/classes.ts
@@ -58,6 +58,8 @@ export const TABLE_ROW_NAME_TEXT = "bp-table-row-name-text";
 export const TABLE_SELECTION_ENABLED = "bp-table-selection-enabled";
 export const TABLE_SELECTION_REGION = "bp-table-selection-region";
 export const TABLE_TH_MENU = "bp-table-th-menu";
+export const TABLE_TH_MENU_CONTAINER = "bp-table-th-menu-container";
+export const TABLE_TH_MENU_CONTAINER_BACKGROUND = "bp-table-th-menu-container-background";
 export const TABLE_THEAD = "bp-table-thead";
 export const TABLE_TOP_CONTAINER = "bp-table-top-container";
 export const TABLE_TRUNCATED_CELL = "bp-table-truncated-cell";
diff --git a/packages/table/src/headers/_headers.scss b/packages/table/src/headers/_headers.scss
index 3c87795137..4e225ff877 100644
--- a/packages/table/src/headers/_headers.scss
+++ b/packages/table/src/headers/_headers.scss
@@ -51,7 +51,24 @@
   }
 }
 
-.bp-table-th-menu {
+.bp-table-th-menu-container {
+  flex-shrink: 0;
+  position: absolute;
+  right: $cell-border-width; // don't overlap box-shadow border
+  opacity: 0; // hide until header is hovered
+  text-align: right;
+
+  .bp-table-header:hover &,
+  .bp-table-header-active & {
+    opacity: 1;
+  }
+
+  .bp-table-interaction-bar & {
+    line-height: $interaction-bar-height;
+  }
+}
+
+.bp-table-th-menu-container-background {
   @function menu-background($color) {
     @return linear-gradient(90deg, rgba($color, 0) 0%, $color 50%);
   }
@@ -65,18 +82,39 @@
     mix($pt-intent-primary, $dark-header-hover-background-color, 10%)
   );
 
-  flex-shrink: 0;
   position: absolute;
   top: 0;
-  right: $cell-border-width; // don't overlap box-shadow border
-  opacity: 0; // hide until header is hovered
-  background: none;
+  right: 0;
+  width: $column-header-min-height + $pt-grid-size * 2;
+  height: $column-header-min-height;
+  pointer-events: none; // don't swallow clicks meant for the underlying header cell
+
+  .bp-table-header:hover &,
+  .bp-table-header-active & {
+    background-image: $menu-hover-background;
+
+    .pt-dark & {
+      background-image: $dark-menu-hover-background;
+    }
+  }
+
+  .bp-table-header.bp-table-header-selected & {
+    background-image: $menu-selected-background;
+
+    .pt-dark & {
+      background-image: $dark-menu-selected-background;
+    }
+  }
+
+  .bp-table-interaction-bar & {
+    height: $interaction-bar-height;
+  }
+}
+
+.bp-table-th-menu {
   cursor: $action-cursor;
-  width: $column-header-min-height + $pt-grid-size;
+  width: $column-header-min-height; // create a larger, more accessible click target
   height: $column-header-min-height;
-  padding-right: $pt-grid-size / 2 + $cell-border-width * 2; // icon has a border
-  text-align: right;
-  pointer-events: all;
 
   .bp-table-interaction-bar & {
     right: $cell-border-width;
@@ -94,12 +132,18 @@
   }
 
   .pt-icon-standard {
+    $icon-offset: ($column-header-min-height - $pt-icon-size-standard) / 2;
+
+    margin-top: $icon-offset;
+    margin-right: $icon-offset;
     border-radius: $pt-border-radius;
     box-shadow: inset 0 0 0 1px rgba($black, $pt-drop-shadow-opacity);
+    background-color: $header-hover-background-color;
     color: $pt-icon-color;
 
     .pt-dark & {
       box-shadow: inset 0 0 0 1px rgba($white, $pt-drop-shadow-opacity);
+      background-color: $dark-header-hover-background-color;
       color: $pt-dark-icon-color;
     }
   }
@@ -125,24 +169,6 @@
       color: $white;
     }
   }
-
-  .bp-table-header:hover &,
-  .bp-table-header-active & {
-    opacity: 1;
-    background-image: $menu-hover-background;
-
-    .pt-dark & {
-      background-image: $dark-menu-hover-background;
-    }
-  }
-
-  .bp-table-header.bp-table-header-selected & {
-    background-image: $menu-selected-background;
-
-    .pt-dark & {
-      background-image: $dark-menu-selected-background;
-    }
-  }
 }
 
 .bp-table-thead {
diff --git a/packages/table/src/headers/columnHeaderCell.tsx b/packages/table/src/headers/columnHeaderCell.tsx
index 1257de94af..57fc56fbe7 100644
--- a/packages/table/src/headers/columnHeaderCell.tsx
+++ b/packages/table/src/headers/columnHeaderCell.tsx
@@ -209,17 +209,20 @@ export class ColumnHeaderCell extends React.Component<IColumnHeaderCellProps, IC
         }];
 
         return (
-            <Popover
-                tetherOptions={{ constraints }}
-                content={menu}
-                position={Position.BOTTOM}
-                className={Classes.TABLE_TH_MENU}
-                popoverDidOpen={this.getPopoverStateChangeHandler(true)}
-                popoverWillClose={this.getPopoverStateChangeHandler(false)}
-                useSmartArrowPositioning={true}
-            >
-                <span className={popoverTargetClasses}/>
-            </Popover>
+            <div className={Classes.TABLE_TH_MENU_CONTAINER}>
+                <div className={Classes.TABLE_TH_MENU_CONTAINER_BACKGROUND} />
+                <Popover
+                    tetherOptions={{ constraints }}
+                    content={menu}
+                    position={Position.BOTTOM}
+                    className={Classes.TABLE_TH_MENU}
+                    popoverDidOpen={this.getPopoverStateChangeHandler(true)}
+                    popoverWillClose={this.getPopoverStateChangeHandler(false)}
+                    useSmartArrowPositioning={true}
+                >
+                    <span className={popoverTargetClasses}/>
+                </Popover>
+            </div>
         );
     }