Updates to menu size

jpmorganchase · Jul 19, 2024 · 6266500 · 6266500
1 parent 533753a
commit 6266500
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 12 deletions.
diff --git a/packages/ag-grid-theme/css/salt-ag-grid-theme.css b/packages/ag-grid-theme/css/salt-ag-grid-theme.css
@@ -61,11 +61,11 @@ div[class*="ag-theme-salt"] {
 }
 
 div[class*="ag-theme-salt-compact"] {
-  /* Ensures icon / country symbol renders correct height  */
+  /* 
+    Ensures icon / country symbol renders correct height.
+    This also means we don't need to touch `--ag-row-height`, which would impact some internal ag grid height calculation like `--ag-internal-padded-row-height` vs `--ag-internal-calculated-line-height`
+   */
   --salt-size-base: 16px;
-
-  /* --ag-row-height: var(--salt-size-base); */
-  /* --ag-header-height: var(--salt-size-base); */
 }
 
 div[class*="ag-theme-salt"] .ag-root-wrapper {
@@ -166,10 +166,14 @@ div[class*="ag-theme-salt"] .ag-cell-label-container {
 /* MENU */
 
 div[class*="ag-theme-salt"] .ag-menu {
-  padding: var(--salt-spacing-100);
+  padding: 0;
   border: var(--salt-size-border) var(--salt-container-borderStyle) var(--salt-selectable-borderColor-selected);
 }
 
+div[class*="ag-theme-salt"] .ag-tabs {
+  padding: var(--salt-spacing-100);
+}
+
 div[class*="ag-theme-salt"] .ag-popup-child:not(.ag-tooltip-custom) {
   box-shadow: var(--salt-overlayable-shadow-popout);
 }
@@ -195,8 +199,9 @@ div[class*="ag-theme-salt"] .ag-menu-option {
   height: calc(var(--salt-size-base) + var(--salt-spacing-100));
 }
 
-div[class*="ag-theme-salt"] .ag-menu-option-icon {
-  padding: 0 var(--salt-spacing-100);
+div[class*="ag-theme-salt"] .ag-menu-option-icon,
+div[class*="ag-theme-salt"] .ag-compact-menu-option-icon {
+  padding-inline-start: var(--salt-spacing-100);
 }
 
 div[class*="ag-theme-salt"] .ag-tab {
@@ -244,6 +249,11 @@ div[class*="ag-theme-salt"] .ag-cell {
   padding-right: var(--salt-spacing-100);
 }
 
+/* This is not restricted to `.editable-cell`, so any custom editor would get the same background treatment */
+div[class*="ag-theme-salt"] .ag-cell-inline-editing:focus-within {
+  background: var(--salt-container-primary-background);
+}
+
 div[class*="ag-theme-salt"] .editable-cell,
 div[class*="ag-theme-salt"] .editable-numeric-cell {
   outline: var(--salt-size-border) var(--salt-container-borderStyle) var(--salt-editable-borderColor);
@@ -298,10 +308,6 @@ div[class*="ag-theme-salt"] .editable-cell.ag-cell-inline-editing {
   padding: 0;
 }
 
-div[class*="ag-theme-salt"] .editable-cell.ag-cell-inline-editing:focus-within {
-  background: var(--salt-container-primary-background);
-}
-
 div[class*="ag-theme-salt"] .editable-numeric-cell input,
 div[class*="ag-theme-salt"] input[class^="ag-"][type="number"] {
   padding: 0 var(--salt-spacing-100);

diff --git a/packages/ag-grid-theme/src/dependencies/dataGridExampleColumnsHdCompact.ts b/packages/ag-grid-theme/src/dependencies/dataGridExampleColumnsHdCompact.ts
@@ -48,12 +48,12 @@ const dataGridExampleColumnsHdCompact: ColDef[] = [
   {
     headerName: "Rating",
     field: "rating",
+    // Not using `editable-cell` as it doesn't work with Dropdown focus style
     editable: true,
     cellEditor: DropdownEditor,
     cellEditorParams: {
       source: [10, 20, 30, 40, 50, 60],
     },
-    cellClass: ["editable-cell"],
   },
   {
     headerName: "Flag",