From 3584a84428bb395a41685f513b7a16bc4b48d271 Mon Sep 17 00:00:00 2001
From: James <j.c.hollyer@gmail.com>
Date: Mon, 25 Sep 2023 10:06:24 -0700
Subject: [PATCH] HParams: Make icons buttons in data table header (#6594)

## Motivation for features / changes
To allow for tabbing to buttons and for general best practice clickable
elements should be buttons. So the 3 dot menu button was changed into a
button instead of a div. For sorting the entire header is clickable. We
did not want to make the entire header a button so I turned the sorting
icon into a button.

I also made some styling changes to spread out the icons and make them
easier to click.

## Technical description of changes
When I made the sorting icon a button I had to add the click event to
make it sort. I also had to stop propagation so the header's click event
was not fired.

## Screenshots of UI changes (or N/A)
![2023-09-22_11-05-35
(1)](https://github.com/tensorflow/tensorboard/assets/8672809/29b33b4c-6122-49fd-b299-b55a12d69011)
---
 .../data_table/header_cell_component.ng.html  | 20 +++++++++++--------
 .../data_table/header_cell_component.scss     | 11 ++++++++--
 2 files changed, 21 insertions(+), 10 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
index 374326d677..4aebef1261 100644
--- a/tensorboard/webapp/widgets/data_table/header_cell_component.ng.html
+++ b/tensorboard/webapp/widgets/data_table/header_cell_component.ng.html
@@ -23,7 +23,12 @@
 >
   <ng-content></ng-content>
   <tb-data-table-header [header]="header"></tb-data-table-header>
-  <div *ngIf="header.sortable" class="sorting-icon-container">
+  <button
+    mat-icon-button
+    *ngIf="header.sortable"
+    class="sorting-icon-container"
+    (click)="$event.stopPropagation(); headerClickedHandler()"
+  >
     <mat-icon
       *ngIf="
             sortingInfo.order === SortingOrder.ASCENDING ||
@@ -44,14 +49,13 @@
           "
       svgIcon="arrow_downward_24px"
     ></mat-icon>
-  </div>
-  <div
+  </button>
+  <button
+    mat-icon-button
     *ngIf="(header.removable || header.sortable) && !disableContextMenu"
     class="context-menu-container show-on-hover"
+    (click)="onContextMenuOpened($event)"
   >
-    <mat-icon
-      (click)="onContextMenuOpened($event)"
-      svgIcon="more_vert_24px"
-    ></mat-icon>
-  </div>
+    <mat-icon svgIcon="more_vert_24px"></mat-icon>
+  </button>
 </div>
diff --git a/tensorboard/webapp/widgets/data_table/header_cell_component.scss b/tensorboard/webapp/widgets/data_table/header_cell_component.scss
index 5c8b9b0209..704f98ccda 100644
--- a/tensorboard/webapp/widgets/data_table/header_cell_component.scss
+++ b/tensorboard/webapp/widgets/data_table/header_cell_component.scss
@@ -34,14 +34,20 @@ $_icon_size: 12px;
     &:hover {
       opacity: 1;
     }
+    &:focus {
+      opacity: 1;
+    }
   }
 }
 
 .sorting-icon-container,
 .context-menu-container {
-  width: $_icon_size;
-  height: $_icon_size;
+  width: $_icon_size + 4px;
+  height: $_icon_size + 4px;
   border-radius: 5px;
+  display: flex;
+  justify-content: center;
+  align-items: center;
 }
 
 .cell {
@@ -51,6 +57,7 @@ $_icon_size: 12px;
   mat-icon {
     height: $_icon_size;
     width: $_icon_size;
+    line-height: 1;
   }
 
   .sorting-icon-container {