diff --git a/.changeset/large-carrots-end.md b/.changeset/large-carrots-end.md index 4ccf930b46f..364cc06792c 100644 --- a/.changeset/large-carrots-end.md +++ b/.changeset/large-carrots-end.md @@ -2,4 +2,19 @@ "@salt-ds/ag-grid-theme": patch --- -Fixed a few bugs (details to follow) +- Fixed background color for custom editor component. +- Fixed header text being cropped in HD compact. Closes #3675. +- Fixed Country Symbol taller than expected in HD compact. This alters `--salt-size-base` token so Salt Button, form controls (Input, Dropdown, Combo Box) will be impacted as well. Closes #3775. +- Fixed group value not center aligned vertically. +- Updated ag grid menu styling to match closer to Salt Menu component. + +Note: We previously made a mistake on `rowHeight` recommendation when configurating AG Grid, which should be 1px more to account for border between row. +`useAgGridHelpers` example hook is updated to reflect this. + +| Density | Row height ([`rowHeight`](https://www.ag-grid.com/javascript-data-grid/row-height/)) | Header height ([`headerHeight`](https://www.ag-grid.com/javascript-data-grid/column-headers/#header-height)) | +| ------------ | ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------ | +| HD (Compact) | 21 | 20 | +| HD | 25 | 24 | +| MD | 37 | 36 | +| LD | 49 | 48 | +| TD | 61 | 60 | diff --git a/package.json b/package.json index e8a4955939e..78458bb60ae 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,7 @@ "prettier:ci": "prettier --check .", "lint": "biome check", "lint:fix": "biome check --fix", + "lint:check:error": "biome check --diagnostic-level=error", "lint:style": "yarn lint:style:core && yarn lint:style:icon && yarn lint:style:lab && yarn lint:style:ag-theme", "lint:style:core": "yarn stylelint -f verbose \"packages/core/src/**/*.css\"", "lint:style:icon": "yarn stylelint -f verbose \"packages/icons/src/**/*.css\"", diff --git a/packages/ag-grid-theme/css/salt-ag-grid-theme.css b/packages/ag-grid-theme/css/salt-ag-grid-theme.css index 5f6ef53c7bb..93d41c4086f 100644 --- a/packages/ag-grid-theme/css/salt-ag-grid-theme.css +++ b/packages/ag-grid-theme/css/salt-ag-grid-theme.css @@ -254,6 +254,10 @@ div[class*="ag-theme-salt"] .ag-cell-inline-editing:focus-within { background: var(--salt-container-primary-background); } +div[class*="ag-theme-salt"] .ag-cell-inline-editing { + padding: 0; +} + 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); @@ -304,10 +308,6 @@ div[class*="ag-theme-salt"] .editable-cell.ag-cell-inline-editing:before { z-index: 2; } -div[class*="ag-theme-salt"] .editable-cell.ag-cell-inline-editing { - padding: 0; -} - 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/cell-editors/DropdownEditor.tsx b/packages/ag-grid-theme/src/dependencies/cell-editors/DropdownEditor.tsx index bc27f66a527..8263f17acdf 100644 --- a/packages/ag-grid-theme/src/dependencies/cell-editors/DropdownEditor.tsx +++ b/packages/ag-grid-theme/src/dependencies/cell-editors/DropdownEditor.tsx @@ -1,3 +1,5 @@ +import { Dropdown, type DropdownProps, Option } from "@salt-ds/core"; +import type { ICellEditorParams } from "ag-grid-community"; import React, { forwardRef, useImperativeHandle, @@ -7,8 +9,6 @@ import React, { type SyntheticEvent, type KeyboardEvent, } from "react"; -import type { ICellEditorParams } from "ag-grid-community"; -import { Dropdown, type DropdownProps, Option } from "@salt-ds/core"; export type GridCellValue = string | boolean | number; @@ -52,8 +52,6 @@ export const DropdownEditor = forwardRef((props: DropdownEditorParams, ref) => { useImperativeHandle(ref, () => ({ getValue: (): typeof value => value })); - console.log("--- DropdownEditor", { value }); - return ( { selected={[value || ""]} ref={button} onKeyDown={onEscapeKeyPressed} + className="DropdownEditor" {...dropdownProps} + style={{ + // Outline will be shown on the cell + outline: "none", + // Leave room for cell focus ring + marginInline: "2px", + width: "calc(100% - 4px)", + }} >
{ diff --git a/packages/ag-grid-theme/src/dependencies/cell-renderers/FlagRenderer.tsx b/packages/ag-grid-theme/src/dependencies/cell-renderers/FlagRenderer.tsx index 39edc9a289e..0681cba2e5e 100644 --- a/packages/ag-grid-theme/src/dependencies/cell-renderers/FlagRenderer.tsx +++ b/packages/ag-grid-theme/src/dependencies/cell-renderers/FlagRenderer.tsx @@ -1,5 +1,5 @@ -import type { CustomCellRendererProps } from "ag-grid-react"; import { countryMetaMap } from "@salt-ds/countries"; +import type { CustomCellRendererProps } from "ag-grid-react"; export const FlagRenderer = (props: CustomCellRendererProps) => { const isoCode = props.value as keyof typeof countryMetaMap; diff --git a/packages/ag-grid-theme/src/dependencies/dataGridExampleColumnsHdCompact.ts b/packages/ag-grid-theme/src/dependencies/dataGridExampleColumnsHdCompact.ts index 46cc67d6d20..5b25a1a76ce 100644 --- a/packages/ag-grid-theme/src/dependencies/dataGridExampleColumnsHdCompact.ts +++ b/packages/ag-grid-theme/src/dependencies/dataGridExampleColumnsHdCompact.ts @@ -1,6 +1,6 @@ import type { ColDef } from "ag-grid-community"; -import { FlagRenderer } from "./cell-renderers/FlagRenderer"; import { DropdownEditor } from "./cell-editors/DropdownEditor"; +import { FlagRenderer } from "./cell-renderers/FlagRenderer"; const dataGridExampleColumnsHdCompact: ColDef[] = [ { @@ -50,7 +50,7 @@ const dataGridExampleColumnsHdCompact: ColDef[] = [ field: "rating", // Not using `editable-cell` as it doesn't work with Dropdown focus style editable: true, - cellEditor: DropdownEditor, + cellEditor: "DropdownEditor", cellEditorParams: { source: [10, 20, 30, 40, 50, 60], }, diff --git a/packages/ag-grid-theme/src/dependencies/dataGridExampleData.ts b/packages/ag-grid-theme/src/dependencies/dataGridExampleData.ts index bdc5a223e78..9335acc583e 100644 --- a/packages/ag-grid-theme/src/dependencies/dataGridExampleData.ts +++ b/packages/ag-grid-theme/src/dependencies/dataGridExampleData.ts @@ -6,7 +6,7 @@ const dataGridExampleData = [ rating: 10, population: 8446790, date: "29/07/2004", - country: 'US' + country: "US", }, { name: "Alaska", @@ -15,7 +15,7 @@ const dataGridExampleData = [ rating: 20, population: 5492139, date: "20/05/2009", - country: 'US' + country: "US", }, { name: "Arizona", @@ -24,7 +24,7 @@ const dataGridExampleData = [ rating: 30, population: 806007, date: "23/02/2020", - country: 'US' + country: "US", }, { name: "Arkansas", @@ -33,7 +33,7 @@ const dataGridExampleData = [ rating: 40, population: 59453, date: "19/02/1999", - country: 'US' + country: "US", }, { name: "California", @@ -42,7 +42,7 @@ const dataGridExampleData = [ rating: 10, population: 8319396, date: "29/04/1967", - country: 'US' + country: "US", }, { name: "Colorado", @@ -51,7 +51,7 @@ const dataGridExampleData = [ rating: 10, population: 8822592, date: "19/02/1944", - country: 'US' + country: "US", }, { name: "Connecticut", @@ -60,7 +60,7 @@ const dataGridExampleData = [ rating: 10, population: 2465263, date: "30/03/2015", - country: 'US' + country: "US", }, { name: "Delaware", @@ -69,7 +69,7 @@ const dataGridExampleData = [ rating: 10, population: 3075357, date: "04/03/1949", - country: 'US' + country: "US", }, { name: "Florida", @@ -78,7 +78,7 @@ const dataGridExampleData = [ rating: 10, population: 7597316, date: "03/10/2005", - country: 'US' + country: "US", }, { name: "Georgia", @@ -87,7 +87,7 @@ const dataGridExampleData = [ rating: 10, population: 7271180, date: "16/02/1998", - country: 'US' + country: "US", }, { name: "Hawaii", @@ -96,7 +96,7 @@ const dataGridExampleData = [ rating: 10, population: 8534120, date: "17/02/2005", - country: 'US' + country: "US", }, { name: "Idaho", @@ -105,7 +105,7 @@ const dataGridExampleData = [ rating: 10, population: 5806269, date: "19/02/2007", - country: 'US' + country: "US", }, { name: "Illinois", @@ -114,7 +114,7 @@ const dataGridExampleData = [ rating: 10, population: 525951, date: "08/11/2022", - country: 'US' + country: "US", }, { name: "Indiana", @@ -123,7 +123,7 @@ const dataGridExampleData = [ rating: 10, population: 5220228, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "Iowa", @@ -132,7 +132,7 @@ const dataGridExampleData = [ rating: 10, population: 333600, date: "31/05/2020", - country: 'US' + country: "US", }, { name: "Kansas", @@ -141,7 +141,7 @@ const dataGridExampleData = [ rating: 10, population: 170082, date: "28/02/1999", - country: 'US' + country: "US", }, { name: "Kentucky", @@ -150,7 +150,7 @@ const dataGridExampleData = [ rating: 10, population: 1359657, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "Louisiana", @@ -159,7 +159,7 @@ const dataGridExampleData = [ rating: 10, population: 9267793, date: "17/02/1970", - country: 'US' + country: "US", }, { name: "Maine", @@ -168,7 +168,7 @@ const dataGridExampleData = [ rating: 10, population: 7366792, date: "13/02/2000", - country: 'US' + country: "US", }, { name: "Maryland", @@ -177,7 +177,7 @@ const dataGridExampleData = [ rating: 10, population: 2474500, date: "18/11/1902", - country: 'US' + country: "US", }, { name: "Massachusetts", @@ -186,7 +186,7 @@ const dataGridExampleData = [ rating: 10, population: 7858200, date: "19/02/1902", - country: 'US' + country: "US", }, { name: "Michigan", @@ -195,7 +195,7 @@ const dataGridExampleData = [ rating: 10, population: 4036589, date: "19/02/2002", - country: 'US' + country: "US", }, { name: "Minnesota", @@ -204,7 +204,7 @@ const dataGridExampleData = [ rating: 10, population: 490080, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "Mississippi", @@ -213,7 +213,7 @@ const dataGridExampleData = [ rating: 10, population: 2021576, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "Missouri", @@ -222,7 +222,7 @@ const dataGridExampleData = [ rating: 10, population: 3511147, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "Montana", @@ -231,7 +231,7 @@ const dataGridExampleData = [ rating: 10, population: 2856628, date: "19/05/0520", - country: 'US' + country: "US", }, { name: "Nebraska", @@ -240,7 +240,7 @@ const dataGridExampleData = [ rating: 10, population: 9584904, date: "05/02/0520", - country: 'US' + country: "US", }, { name: "Nevada", @@ -249,7 +249,7 @@ const dataGridExampleData = [ rating: 10, population: 489695, date: "05/02/2002", - country: 'US' + country: "US", }, { name: "New Hampshire", @@ -258,7 +258,7 @@ const dataGridExampleData = [ rating: 10, population: 8819049, date: "19/02/2002", - country: 'US' + country: "US", }, { name: "New Jersey", @@ -267,7 +267,7 @@ const dataGridExampleData = [ rating: 10, population: 2500770, date: "19/02/2002", - country: 'US' + country: "US", }, { name: "New Mexico", @@ -276,7 +276,7 @@ const dataGridExampleData = [ rating: 10, population: 536205, date: "19/02/2002", - country: 'US' + country: "US", }, { name: "New York", @@ -285,7 +285,7 @@ const dataGridExampleData = [ rating: 10, population: 5248173, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "North Carolina", @@ -294,7 +294,7 @@ const dataGridExampleData = [ rating: 10, population: 1452619, date: "10/02/1020", - country: 'US' + country: "US", }, { name: "North Dakota", @@ -303,7 +303,7 @@ const dataGridExampleData = [ rating: 10, population: 8890392, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "Ohio", @@ -312,7 +312,7 @@ const dataGridExampleData = [ rating: 10, population: 5968829, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "Oklahoma", @@ -321,7 +321,7 @@ const dataGridExampleData = [ rating: 10, population: 9044655, date: "21/02/1950", - country: 'US' + country: "US", }, { name: "Oregon", @@ -330,7 +330,7 @@ const dataGridExampleData = [ rating: 10, population: 8054969, date: "12/10/1920", - country: 'US' + country: "US", }, { name: "Pennsylvania", @@ -339,7 +339,7 @@ const dataGridExampleData = [ rating: 10, population: 1359410, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "Rhode Island", @@ -348,7 +348,7 @@ const dataGridExampleData = [ rating: 10, population: 4473590, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "South Carolina", @@ -357,7 +357,7 @@ const dataGridExampleData = [ rating: 10, population: 6527907, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "South Dakota", @@ -366,7 +366,7 @@ const dataGridExampleData = [ rating: 10, population: 3152416, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "Tennessee", @@ -375,7 +375,7 @@ const dataGridExampleData = [ rating: 10, population: 9717114, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "Texas", @@ -384,7 +384,7 @@ const dataGridExampleData = [ rating: 10, population: 6552290, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "Utah", @@ -393,7 +393,7 @@ const dataGridExampleData = [ rating: 10, population: 2815416, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "Vermont", @@ -402,7 +402,7 @@ const dataGridExampleData = [ rating: 10, population: 2845360, date: "19/02/1920", - country: 'US' + country: "US", }, { name: "Virginia", @@ -411,7 +411,7 @@ const dataGridExampleData = [ rating: 10, population: 4919143, date: "20/02/1920", - country: 'US' + country: "US", }, { name: "Washington", @@ -420,7 +420,7 @@ const dataGridExampleData = [ rating: 10, population: 4614717, date: "22/02/2009", - country: 'US' + country: "US", }, { name: "West Virginia", @@ -429,7 +429,7 @@ const dataGridExampleData = [ rating: 10, population: 6413104, date: "19/11/2002", - country: 'US' + country: "US", }, { name: "Wisconsin", @@ -438,7 +438,7 @@ const dataGridExampleData = [ rating: 10, population: 3934168, date: "01/08/2005", - country: 'US' + country: "US", }, { name: "Wyoming", @@ -447,7 +447,7 @@ const dataGridExampleData = [ rating: 10, population: 901078, date: "01/01/2000", - country: 'US' + country: "US", }, ]; diff --git a/packages/ag-grid-theme/src/dependencies/useAgGridHelpers.ts b/packages/ag-grid-theme/src/dependencies/useAgGridHelpers.ts index bc592e88599..2745b2d289a 100644 --- a/packages/ag-grid-theme/src/dependencies/useAgGridHelpers.ts +++ b/packages/ag-grid-theme/src/dependencies/useAgGridHelpers.ts @@ -45,20 +45,20 @@ export function useAgGridHelpers({ const mode = modeProp ?? contextMode; const density = densityProp ?? contextDensity; - const [rowHeight, listItemHeight] = useMemo(() => { + const [rowHeight, headerRowHeight] = useMemo(() => { switch (density) { case compact && "high": - return [20, 20]; + return [21, 20]; case "high": - return [24, 24]; // 20 + 4 + return [25, 24]; // 20 + 4 + [1 (border)] case "medium": - return [36, 36]; // 28 + 8 + return [37, 36]; // 28 + 8 + [1 (border)] case "low": - return [48, 48]; // 36 + 12 + return [49, 48]; // 36 + 12 + [1 (border)] case "touch": - return [60, 60]; // 44 + 16 + return [61, 60]; // 44 + 16 + [1 (border)] default: - return [24, 24]; + return [25, 24]; } }, [density, compact]); @@ -81,14 +81,14 @@ export function useAgGridHelpers({ agGridProps: { onGridReady, rowHeight, - headerHeight: rowHeight, + headerHeight: headerRowHeight, suppressMenuHide: true, defaultColDef: { filter: true, resizable: true, sortable: true, filterParams: { - cellHeight: listItemHeight, + cellHeight: rowHeight, }, }, }, diff --git a/packages/ag-grid-theme/src/examples/HDCompact.tsx b/packages/ag-grid-theme/src/examples/HDCompact.tsx index 81918f9e4fc..0149397e670 100644 --- a/packages/ag-grid-theme/src/examples/HDCompact.tsx +++ b/packages/ag-grid-theme/src/examples/HDCompact.tsx @@ -1,5 +1,6 @@ import { SaltProvider, SaltProviderNext, useTheme } from "@salt-ds/core"; import { AgGridReact, type AgGridReactProps } from "ag-grid-react"; +import { DropdownEditor } from "../dependencies/cell-editors/DropdownEditor"; import dataGridExampleColumnsHdCompact from "../dependencies/dataGridExampleColumnsHdCompact"; import dataGridExampleData from "../dependencies/dataGridExampleData"; import { useAgGridHelpers } from "../dependencies/useAgGridHelpers"; @@ -43,6 +44,9 @@ const HDCompact = (props: AgGridReactProps) => { } }); }} + components={{ + DropdownEditor, + }} />
diff --git a/site/src/examples/ag-grid-theme/Default.tsx b/site/src/examples/ag-grid-theme/Default.tsx index 702218f4fc0..a9597e63c83 100644 --- a/site/src/examples/ag-grid-theme/Default.tsx +++ b/site/src/examples/ag-grid-theme/Default.tsx @@ -35,15 +35,15 @@ export const Default = (): ReactElement => { const rowHeight = useMemo(() => { switch (density) { case "high": - return 24; + return 25; case "medium": - return 36; + return 37; case "low": - return 48; + return 49; case "touch": - return 60; + return 61; default: - return 20; + return 25; } }, [density]); diff --git a/site/src/examples/ag-grid-theme/useAgGridHelpers.ts b/site/src/examples/ag-grid-theme/useAgGridHelpers.ts index 2d4ee199b0e..3d1de51077d 100644 --- a/site/src/examples/ag-grid-theme/useAgGridHelpers.ts +++ b/site/src/examples/ag-grid-theme/useAgGridHelpers.ts @@ -23,20 +23,21 @@ export function useAgGridHelpers(compact = false): { const density = useDensity(); const { mode } = useTheme(); - const [rowHeight, listItemHeight] = useMemo(() => { + // Row height is 1px more than header row, to count for border between rows + const [rowHeight, headerRowHeight] = useMemo(() => { switch (density) { case compact && "high": - return [20, 20]; + return [21, 20]; case "high": - return [24, 24]; + return [25, 24]; case "medium": - return [36, 36]; + return [37, 36]; case "low": - return [48, 48]; + return [49, 48]; case "touch": - return [60, 60]; + return [61, 60]; default: - return [20, 24]; + return [25, 24]; } }, [density, compact]); @@ -58,14 +59,14 @@ export function useAgGridHelpers(compact = false): { agGridProps: { onGridReady, rowHeight, - headerHeight: rowHeight, + headerHeight: headerRowHeight, suppressMenuHide: true, defaultColDef: { filter: true, resizable: true, sortable: true, filterParams: { - cellHeight: listItemHeight, + cellHeight: rowHeight, }, }, },