HParams: Restyle remove column button (#6466)

## Motivation for features / changes The styling on the remove button was broken by one of the recent changes to the column header. I am fixing and improving it. ## Screenshots of UI changes (or N/A) Dark Mode ![image](https://github.com/tensorflow/tensorboard/assets/78179109/8ef08b52-3266-4b07-b846-7ed8cf7ae1ec) Light Mode ![image](https://github.com/tensorflow/tensorboard/assets/78179109/b42871d5-f6cf-46e4-8a29-c94fe911913a)
tensorflow · Jun 28, 2023 · 5d338e2 · 5d338e2
1 parent f6f6c44
commit 5d338e2
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 48 deletions.
diff --git a/tensorboard/webapp/widgets/data_table/header_cell_component.ng.html b/tensorboard/webapp/widgets/data_table/header_cell_component.ng.html
@@ -22,9 +22,12 @@
   [ngClass]="highlightStyle$ | async"
 >
   <ng-content></ng-content>
-  <div *ngIf="hparamsEnabled && header.removable" class="delete-icon-container">
+  <div
+    *ngIf="hparamsEnabled && header.removable"
+    class="delete-button-container"
+  >
     <button
-      class="delete-icon"
+      class="delete-button"
       mat-icon-button
       i18n-aria-label="A button to delete a data table column."
       aria-label="Delete column"
@@ -56,7 +59,10 @@
       svgIcon="arrow_downward_24px"
     ></mat-icon>
   </div>
-  <div *ngIf="controlsEnabled" class="context-menu-container">
+  <div
+    *ngIf="header.removable || header.sortable"
+    class="context-menu-container"
+  >
     <button
       mat-icon-button
       class="context-menu-trigger"

diff --git a/tensorboard/webapp/widgets/data_table/header_cell_component.scss b/tensorboard/webapp/widgets/data_table/header_cell_component.scss
@@ -17,6 +17,7 @@ limitations under the License.
 @import 'tensorboard/webapp/theme/tb_theme';
 
 $_accent: map-get(mat.get-color-config($tb-theme), accent);
+$_icon_size: 12px;
 
 :host {
   display: table-cell;
@@ -27,73 +28,69 @@ $_accent: map-get(mat.get-color-config($tb-theme), accent);
     .context-menu-container {
       opacity: 0.8;
     }
+
+    .show-on-hover {
+      opacity: 0.3;
+    }
   }
 }
+
 .sorting-icon-container {
-  width: 12px;
-  height: 12px;
+  width: $_icon_size;
+  height: $_icon_size;
   border-radius: 5px;
 }
-.cell mat-icon {
-  height: 12px;
-  width: 12px;
-}
+
 .cell {
   align-items: center;
   display: flex;
-}
-.cell .sorting-icon-container {
-  ::ng-deep path {
-    fill: unset;
-  }
-}
 
-.cell:hover .delete-icon-container {
-  opacity: 1;
-}
+  &:hover {
+    .delete-button-container {
+      opacity: 1;
+    }
+  }
 
-.delete-icon {
-  background-color: #fff;
-  border: 0;
-  border-radius: 50%;
-  color: mat.get-color-from-palette(mat.$gray-palette, 500);
-  font-size: 11px;
-  height: 11px;
-  margin-top: 2px;
-  padding: 0;
-  width: 11px;
+  mat-icon {
+    height: $_icon_size;
+    width: $_icon_size;
+  }
 
-  .mat-icon {
-    width: 100%;
-    height: 100%;
+  .sorting-icon-container {
+    ::ng-deep path {
+      fill: unset;
+    }
   }
 }
 
-.delete-icon:hover {
-  background-color: mat.get-color-from-palette(mat.$gray-palette, 400);
+.delete-button {
+  display: flex;
+  align-items: center;
   color: #fff;
+  font-size: $_icon_size;
+  height: $_icon_size;
+  width: $_icon_size;
+  line-height: $_icon_size;
   cursor: pointer;
-}
 
-.delete-icon-container {
-  margin-left: -12px;
-  opacity: 0;
-  position: absolute;
+  .mat-icon {
+    line-height: $_icon_size;
+  }
 }
 
-.delete-icon:hover {
+.delete-button-container {
+  border-radius: 50%;
   background-color: mat.get-color-from-palette(mat.$gray-palette, 400);
-  cursor: pointer;
+  margin-left: -$_icon_size;
+  height: $_icon_size;
+  opacity: 0;
+  position: absolute;
 }
 
 .show {
   opacity: 1;
 }
 
-:host:hover .show-on-hover {
-  opacity: 0.3;
-}
-
 .show-on-hover {
   opacity: 0;
 }

diff --git a/tensorboard/webapp/widgets/data_table/header_cell_component_test.ts b/tensorboard/webapp/widgets/data_table/header_cell_component_test.ts
@@ -133,7 +133,7 @@ describe('header cell', () => {
         .query(By.directive(HeaderCellComponent))
         .componentInstance.deleteButtonClicked.subscribe();
       fixture.debugElement
-        .query(By.css('.delete-icon'))
+        .query(By.css('.delete-button'))
         .triggerEventHandler('click', {});
 
       expect(deleteButtonClickedSpy).toHaveBeenCalledOnceWith({
@@ -150,13 +150,13 @@ describe('header cell', () => {
     it('renders delete button when hparamsEnabled is true', () => {
       const fixture = createComponent({hparamsEnabled: true});
 
-      expect(fixture.debugElement.query(By.css('.delete-icon'))).toBeTruthy();
+      expect(fixture.debugElement.query(By.css('.delete-button'))).toBeTruthy();
     });
 
     it('does not render delete button when hparamsEnabled is false', () => {
       const fixture = createComponent({hparamsEnabled: false});
 
-      expect(fixture.debugElement.query(By.css('.delete-icon'))).toBeFalsy();
+      expect(fixture.debugElement.query(By.css('.delete-button'))).toBeFalsy();
     });
 
     it('does not render delete button when removable is false', () => {
@@ -170,7 +170,7 @@ describe('header cell', () => {
         },
       });
 
-      expect(fixture.debugElement.query(By.css('.delete-icon'))).toBeFalsy();
+      expect(fixture.debugElement.query(By.css('.delete-button'))).toBeFalsy();
     });
   });