From 4ce98a42a2dba57a77130dd94094720a29f16647 Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Mon, 18 Nov 2024 12:40:40 -0500 Subject: [PATCH 01/14] feat: editable cells on hover support --- src/components/Table/GridTable.stories.tsx | 87 +++++++++++++++++++++- src/components/Table/GridTableApi.ts | 2 +- src/components/Table/TableStyles.tsx | 2 + src/components/Table/components/Row.tsx | 47 ++++++++++-- src/components/Table/components/cell.tsx | 8 +- src/components/Table/types.ts | 6 +- src/components/Table/utils/utils.tsx | 10 ++- src/inputs/TextFieldBase.tsx | 2 + 8 files changed, 150 insertions(+), 14 deletions(-) diff --git a/src/components/Table/GridTable.stories.tsx b/src/components/Table/GridTable.stories.tsx index ac374cbe0..298b330ca 100644 --- a/src/components/Table/GridTable.stories.tsx +++ b/src/components/Table/GridTable.stories.tsx @@ -32,8 +32,9 @@ import { useGridTableApi, } from "src/components/index"; import { Css, Palette } from "src/Css"; +import { jan1, jan2, jan29 } from "src/forms/formStateDomain"; import { useComputed } from "src/hooks"; -import { SelectField } from "src/inputs"; +import { DateField, SelectField } from "src/inputs"; import { NumberField } from "src/inputs/NumberField"; import { noop } from "src/utils"; import { newStory, withRouter, zeroTo } from "src/utils/sb"; @@ -2144,3 +2145,87 @@ export function MinColumnWidths() { ); } + +enum EditableRowStatus { + Active = "Active", + Inactive = "Inactive", +} + +type EditableRowData = { + kind: "data"; + id: string; + data: { id: string; name: string; status: EditableRowStatus; value: number; date?: Date }; +}; +type EditableRow = EditableRowData | HeaderRow; + +export function EditableRows() { + const [rows, setRows] = useState[]>([ + simpleHeader, + { + kind: "data" as const, + id: "1", + data: { id: "1", name: "Tony Stark", status: EditableRowStatus.Active, value: 1, date: jan1 }, + }, + { + kind: "data" as const, + id: "2", + data: { id: "2", name: "Natasha Romanova", status: EditableRowStatus.Active, value: 2, date: jan2 }, + }, + { + kind: "data" as const, + id: "3", + data: { id: "3", name: "Thor Odinson", status: EditableRowStatus.Active, value: 3, date: jan29 }, + }, + ]); + + const nameColumn: GridColumn = { + header: "Name", + data: ({ name }) => name, + }; + + const selectColumn: GridColumn = { + header: "Status", + data: (row) => ({ + content: ( + ({ label: status, code: status }))} + value={row.status} + onSelect={noop} + /> + ), + editableOnHover: true, + }), + w: "100px", + }; + + const date1Column: GridColumn = { + header: "Date", + data: (row, { editable }) => ({ + content: ( + + ), + editableOnHover: true, + }), + w: "120px", + }; + + const date2Column: GridColumn = { + header: "Date", + data: (row, { editable }) => ({ + content: ( + + ), + editableOnHover: true, + }), + w: "120px", + }; + + return ( + + ); +} diff --git a/src/components/Table/GridTableApi.ts b/src/components/Table/GridTableApi.ts index d4b3360bd..96ac3c769 100644 --- a/src/components/Table/GridTableApi.ts +++ b/src/components/Table/GridTableApi.ts @@ -227,7 +227,7 @@ export class GridTableApiImpl implements GridTableApi { .filter((c) => !c.isAction) .map((c) => { // Just guessing for level=1 - const maybeContent = applyRowFn(c, rs.row, this as any as GridRowApi, 1, true, undefined); + const maybeContent = applyRowFn(c, rs.row, this as any as GridRowApi, 1, true, false, undefined); if (isGridCellContent(maybeContent)) { const cell = maybeContent; const content = maybeApply(cell.content); diff --git a/src/components/Table/TableStyles.tsx b/src/components/Table/TableStyles.tsx index c36b15c52..62e2151af 100644 --- a/src/components/Table/TableStyles.tsx +++ b/src/components/Table/TableStyles.tsx @@ -42,6 +42,8 @@ export interface GridStyle { firstRowMessageCss?: Properties; /** Applied on hover if a row has a rowLink/onClick set. */ rowHoverColor?: Palette | "none"; + /** Applied on hover to a cell TextFieldBase */ + rowEditableCellBorderColor?: Palette; /** Applied on hover of a row */ nonHeaderRowHoverCss?: Properties; /** Default content to put into an empty cell */ diff --git a/src/components/Table/components/Row.tsx b/src/components/Table/components/Row.tsx index 1d1759aa2..e06c4e92a 100644 --- a/src/components/Table/components/Row.tsx +++ b/src/components/Table/components/Row.tsx @@ -88,6 +88,7 @@ function RowImpl(props: RowProps): ReactElement { const sortOn = tableState.sortConfig?.on; const revealOnRowHoverClass = "revealOnRowHover"; + const editableOnRowHoverClass = "editableOnRowHover"; const showRowHoverColor = !reservedRowKinds.includes(row.kind) && !omitRowHover && style.rowHoverColor !== "none"; @@ -123,6 +124,11 @@ function RowImpl(props: RowProps): ReactElement { [` > .${revealOnRowHoverClass} > *`]: Css.vh.$, [`:hover > .${revealOnRowHoverClass} > *`]: Css.vv.$, }, + ...{ + [`:hover > .${editableOnRowHoverClass} .textFieldBaseWrapper`]: Css.px1.br4.ba.bc( + style.rowEditableCellBorderColor ?? Palette.Blue300, + ).$, + }, ...(isLastKeptRow && Css.addIn("&>*", style.keptLastRowCss).$), }; @@ -210,7 +216,19 @@ function RowImpl(props: RowProps): ReactElement { onDragOver: onDragOverDebounced, }; - const maybeContent = applyRowFn(column as GridColumnWithId, row, rowApi, level, isExpanded, dragData); + const cellId = `${row.kind}_${row.id}_${column.id}`; + const applyCellHighlight = cellHighlight && !!column.id && !isHeader && !isTotals; + const isCellActive = tableState.activeCellId === cellId; + + const maybeContent = applyRowFn( + column as GridColumnWithId, + row, + rowApi, + level, + isExpanded, + isCellActive, + dragData, + ); // Only use the `numExpandedColumns` as the `colspan` when rendering the "Expandable Header" currentColspan = @@ -220,6 +238,7 @@ function RowImpl(props: RowProps): ReactElement { ? numExpandedColumns + 1 : 1; const revealOnRowHover = isGridCellContent(maybeContent) ? maybeContent.revealOnRowHover : false; + const editableOnRowHover = isGridCellContent(maybeContent) ? maybeContent.editableOnHover : false; const canSortColumn = (sortOn === "client" && column.clientSideSort !== false) || @@ -274,11 +293,6 @@ function RowImpl(props: RowProps): ReactElement { // This relies on our column sizes being defined in pixel values, which is currently true as we calculate to pixel values in the `useSetupColumnSizes` hook minStickyLeftOffset += maybeSticky === "left" ? parseInt(columnSizes[columnIndex].replace("px", ""), 10) : 0; - - const cellId = `${row.kind}_${row.id}_${column.id}`; - const applyCellHighlight = cellHighlight && !!column.id && !isHeader && !isTotals; - const isCellActive = tableState.activeCellId === cellId; - // Note that it seems expensive to calc a per-cell class name/CSS-in-JS output, // vs. setting global/table-wide CSS like `style.cellCss` on the root grid div with // a few descendent selectors. However, that approach means the root grid-applied @@ -334,8 +348,15 @@ function RowImpl(props: RowProps): ReactElement { })`, }; - const cellClassNames = revealOnRowHover ? revealOnRowHoverClass : undefined; + const cellClassNames = [ + ...(revealOnRowHover ? [revealOnRowHoverClass] : []), + ...(editableOnRowHover && (isCellActive || !tableState.activeCellId) ? [editableOnRowHoverClass] : []), + ].join(" "); + const cellOnHover = + isGridCellContent(maybeContent) && maybeContent.editableOnHover + ? (enter: boolean) => (enter ? api.setActiveCellId(cellId) : api.setActiveCellId(undefined)) + : undefined; const cellOnClick = applyCellHighlight ? () => api.setActiveCellId(cellId) : undefined; const tooltip = isGridCellContent(maybeContent) ? maybeContent.tooltip : undefined; @@ -348,7 +369,17 @@ function RowImpl(props: RowProps): ReactElement { ? rowClickRenderFn(as, api, currentColspan) : defaultRenderFn(as, currentColspan); - return renderFn(columnIndex, cellCss, content, row, rowStyle, cellClassNames, cellOnClick, tooltip); + return renderFn( + columnIndex, + cellCss, + content, + row, + rowStyle, + cellClassNames, + cellOnClick, + cellOnHover, + tooltip, + ); }) )} diff --git a/src/components/Table/components/cell.tsx b/src/components/Table/components/cell.tsx index 4cc0f29f2..01ffa48f7 100644 --- a/src/components/Table/components/cell.tsx +++ b/src/components/Table/components/cell.tsx @@ -34,6 +34,8 @@ export type GridCellContent = { revealOnRowHover?: true; /** Tooltip to add to a cell */ tooltip?: ReactNode; + /** Allows cell to be editable when hovering, and also highlights the field on hover */ + editableOnHover?: true; }; /** Allows rendering a specific cell. */ @@ -45,12 +47,14 @@ export type RenderCellFn = ( rowStyle: RowStyle | undefined, classNames: string | undefined, onClick: VoidFunction | undefined, + onHover: ((enter: boolean) => void) | undefined, tooltip: ReactNode | undefined, ) => ReactNode; /** Renders our default cell element, i.e. if no row links and no custom renderCell are used. */ export const defaultRenderFn: (as: RenderAs, colSpan: number) => RenderCellFn = - (as: RenderAs, colSpan) => (key, css, content, row, rowStyle, classNames: string | undefined, onClick, tooltip) => { + (as: RenderAs, colSpan) => + (key, css, content, row, rowStyle, classNames: string | undefined, onClick, onHover, tooltip) => { const Cell = as === "table" ? "td" : "div"; return ( RenderCellFn onHover?.(true)} + onMouseLeave={() => onHover?.(false)} {...(as === "table" && { colSpan })} > {content} diff --git a/src/components/Table/types.ts b/src/components/Table/types.ts index 4c727b8e4..6efd7eb89 100644 --- a/src/components/Table/types.ts +++ b/src/components/Table/types.ts @@ -39,11 +39,11 @@ export type GridColumn = { | (DiscriminateUnion extends { data: infer D } ? ( data: D, - opts: { row: GridRowKind; api: GridRowApi; level: number; expanded: boolean }, + opts: { row: GridRowKind; api: GridRowApi; level: number; expanded: boolean; editable: boolean }, ) => ReactNode | GridCellContent : ( data: undefined, - opts: { row: GridRowKind; api: GridRowApi; level: number; expanded: boolean }, + opts: { row: GridRowKind; api: GridRowApi; level: number; expanded: boolean; editable: boolean }, ) => ReactNode | GridCellContent); } & { /** @@ -85,6 +85,8 @@ export type GridColumn = { initExpanded?: boolean; /** Determines whether this column should be hidden when expanded (only the 'expandColumns' would show) */ hideOnExpand?: boolean; + /** Flag that changes the field behavior to be editable on hover */ + editableOnHover?: boolean; }; /** diff --git a/src/components/Table/utils/utils.tsx b/src/components/Table/utils/utils.tsx index afd30de1c..cad2b3c3d 100644 --- a/src/components/Table/utils/utils.tsx +++ b/src/components/Table/utils/utils.tsx @@ -146,13 +146,21 @@ export function applyRowFn( api: GridRowApi, level: number, expanded: boolean, + editable: boolean, dragData?: DragData, ): ReactNode | GridCellContent { // Usually this is a function to apply against the row, but sometimes it's a hard-coded value, i.e. for headers const maybeContent = column[row.kind]; if (typeof maybeContent === "function") { // Auto-destructure data - return (maybeContent as Function)((row as any)["data"], { row: row as any, api, level, expanded, dragData }); + return (maybeContent as Function)((row as any)["data"], { + row: row as any, + api, + level, + expanded, + editable, + dragData, + }); } else { return maybeContent; } diff --git a/src/inputs/TextFieldBase.tsx b/src/inputs/TextFieldBase.tsx index 77434e97b..cea445799 100644 --- a/src/inputs/TextFieldBase.tsx +++ b/src/inputs/TextFieldBase.tsx @@ -229,6 +229,7 @@ export function TextFieldBase>(props: TextFieldB ...(multiline ? Css.fdc.aifs.gap2.$ : Css.if(wrap === false).truncate.$), ...xss, }} + className="textFieldBaseWrapper" data-readonly="true" {...tid} > @@ -259,6 +260,7 @@ export function TextFieldBase>(props: TextFieldB ...(errorMsg && !inputProps.disabled ? fieldStyles.error : {}), ...Css.if(multiline).aifs.oh.mhPx(textAreaMinHeight).$, }} + className="textFieldBaseWrapper" {...hoverProps} ref={inputWrapRef as any} onClick={unfocusedPlaceholder ? handleUnfocusedPlaceholderClick : undefined} From 3f328cc474c9ff66e8feea2d0089eea6066c7fa4 Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Mon, 18 Nov 2024 12:44:26 -0500 Subject: [PATCH 02/14] fix: default editable value --- src/components/Table/utils/RowState.ts | 2 +- src/components/Table/utils/sortRows.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/Table/utils/RowState.ts b/src/components/Table/utils/RowState.ts index 0784de30b..0eec79499 100644 --- a/src/components/Table/utils/RowState.ts +++ b/src/components/Table/utils/RowState.ts @@ -371,7 +371,7 @@ export class RowState { const { visibleColumns, search } = this.states.table; return search.every((term) => visibleColumns - .map((c) => applyRowFn(c, this.row, this.api, 0, false)) + .map((c) => applyRowFn(c, this.row, this.api, 0, false, false)) .some((maybeContent) => matchesFilter(maybeContent, term)), ); } finally { diff --git a/src/components/Table/utils/sortRows.ts b/src/components/Table/utils/sortRows.ts index 43a16a657..1d85d0ed5 100644 --- a/src/components/Table/utils/sortRows.ts +++ b/src/components/Table/utils/sortRows.ts @@ -73,8 +73,8 @@ function compare( invert: boolean, caseSensitive: boolean, ) { - const v1 = sortValue(applyRowFn(column, a, {} as any, 0, false), caseSensitive); - const v2 = sortValue(applyRowFn(column, b, {} as any, 0, false), caseSensitive); + const v1 = sortValue(applyRowFn(column, a, {} as any, 0, false, false), caseSensitive); + const v2 = sortValue(applyRowFn(column, b, {} as any, 0, false, false), caseSensitive); const v1e = v1 === null || v1 === undefined; const v2e = v2 === null || v2 === undefined; if ((v1e && v2e) || v1 === v2) { From c7a280c0a205e1cd7a369ea7ecfa27552306a657 Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Wed, 20 Nov 2024 12:22:39 -0500 Subject: [PATCH 03/14] fix: PR feedback --- src/components/Table/GridTable.stories.tsx | 28 +++++++++++++++++++--- src/components/Table/components/Row.tsx | 3 ++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/src/components/Table/GridTable.stories.tsx b/src/components/Table/GridTable.stories.tsx index 298b330ca..379bba712 100644 --- a/src/components/Table/GridTable.stories.tsx +++ b/src/components/Table/GridTable.stories.tsx @@ -2178,6 +2178,14 @@ export function EditableRows() { }, ]); + const handleCellChange = (rowId: string, field: keyof EditableRowData["data"], value: any) => { + setRows((rows) => + rows.map((row) => + row.kind === "data" && row.id === rowId ? { ...row, data: { ...row.data, [field]: value } } : row, + ), + ); + }; + const nameColumn: GridColumn = { header: "Name", data: ({ name }) => name, @@ -2191,7 +2199,7 @@ export function EditableRows() { label="" options={Object.values(EditableRowStatus).map((status) => ({ label: status, code: status }))} value={row.status} - onSelect={noop} + onSelect={(status) => handleCellChange(row.id, "status", status)} /> ), editableOnHover: true, @@ -2203,7 +2211,14 @@ export function EditableRows() { header: "Date", data: (row, { editable }) => ({ content: ( - + handleCellChange(row.id, "date", date)} + readOnly={!editable} + hideCalendarIcon + format="medium" + /> ), editableOnHover: true, }), @@ -2214,7 +2229,14 @@ export function EditableRows() { header: "Date", data: (row, { editable }) => ({ content: ( - + handleCellChange(row.id, "date", date)} + readOnly={!editable} + hideCalendarIcon + format="medium" + /> ), editableOnHover: true, }), diff --git a/src/components/Table/components/Row.tsx b/src/components/Table/components/Row.tsx index e06c4e92a..b7e1953d1 100644 --- a/src/components/Table/components/Row.tsx +++ b/src/components/Table/components/Row.tsx @@ -125,7 +125,8 @@ function RowImpl(props: RowProps): ReactElement { [`:hover > .${revealOnRowHoverClass} > *`]: Css.vv.$, }, ...{ - [`:hover > .${editableOnRowHoverClass} .textFieldBaseWrapper`]: Css.px1.br4.ba.bc( + [`.textFieldBaseWrapper`]: Css.px1.br4.mlPx(-8).ba.bcTransparent.$, + [`:hover > .${editableOnRowHoverClass} .textFieldBaseWrapper`]: Css.ba.bc( style.rowEditableCellBorderColor ?? Palette.Blue300, ).$, }, From c3fb6f04b341850c3e8ed6a06c9583280499bb49 Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Wed, 20 Nov 2024 15:56:43 -0500 Subject: [PATCH 04/14] fix: PR feedback (no editable states) --- src/components/Table/GridTable.stories.tsx | 6 ++---- src/components/Table/GridTableApi.ts | 2 +- src/components/Table/components/Row.tsx | 12 ++---------- src/components/Table/utils/RowState.ts | 2 +- src/components/Table/utils/sortRows.ts | 4 ++-- src/components/Table/utils/utils.tsx | 2 -- 6 files changed, 8 insertions(+), 20 deletions(-) diff --git a/src/components/Table/GridTable.stories.tsx b/src/components/Table/GridTable.stories.tsx index 379bba712..a6ab0846f 100644 --- a/src/components/Table/GridTable.stories.tsx +++ b/src/components/Table/GridTable.stories.tsx @@ -2209,13 +2209,12 @@ export function EditableRows() { const date1Column: GridColumn = { header: "Date", - data: (row, { editable }) => ({ + data: (row) => ({ content: ( handleCellChange(row.id, "date", date)} - readOnly={!editable} hideCalendarIcon format="medium" /> @@ -2227,13 +2226,12 @@ export function EditableRows() { const date2Column: GridColumn = { header: "Date", - data: (row, { editable }) => ({ + data: (row) => ({ content: ( handleCellChange(row.id, "date", date)} - readOnly={!editable} hideCalendarIcon format="medium" /> diff --git a/src/components/Table/GridTableApi.ts b/src/components/Table/GridTableApi.ts index 96ac3c769..d4b3360bd 100644 --- a/src/components/Table/GridTableApi.ts +++ b/src/components/Table/GridTableApi.ts @@ -227,7 +227,7 @@ export class GridTableApiImpl implements GridTableApi { .filter((c) => !c.isAction) .map((c) => { // Just guessing for level=1 - const maybeContent = applyRowFn(c, rs.row, this as any as GridRowApi, 1, true, false, undefined); + const maybeContent = applyRowFn(c, rs.row, this as any as GridRowApi, 1, true, undefined); if (isGridCellContent(maybeContent)) { const cell = maybeContent; const content = maybeApply(cell.content); diff --git a/src/components/Table/components/Row.tsx b/src/components/Table/components/Row.tsx index b7e1953d1..e0478cd6c 100644 --- a/src/components/Table/components/Row.tsx +++ b/src/components/Table/components/Row.tsx @@ -125,7 +125,7 @@ function RowImpl(props: RowProps): ReactElement { [`:hover > .${revealOnRowHoverClass} > *`]: Css.vv.$, }, ...{ - [`.textFieldBaseWrapper`]: Css.px1.br4.mlPx(-8).ba.bcTransparent.$, + [`.textFieldBaseWrapper`]: Css.pl1.mlPx(-8).br4.ba.bcTransparent.$, [`:hover > .${editableOnRowHoverClass} .textFieldBaseWrapper`]: Css.ba.bc( style.rowEditableCellBorderColor ?? Palette.Blue300, ).$, @@ -221,15 +221,7 @@ function RowImpl(props: RowProps): ReactElement { const applyCellHighlight = cellHighlight && !!column.id && !isHeader && !isTotals; const isCellActive = tableState.activeCellId === cellId; - const maybeContent = applyRowFn( - column as GridColumnWithId, - row, - rowApi, - level, - isExpanded, - isCellActive, - dragData, - ); + const maybeContent = applyRowFn(column as GridColumnWithId, row, rowApi, level, isExpanded, dragData); // Only use the `numExpandedColumns` as the `colspan` when rendering the "Expandable Header" currentColspan = diff --git a/src/components/Table/utils/RowState.ts b/src/components/Table/utils/RowState.ts index 0eec79499..0784de30b 100644 --- a/src/components/Table/utils/RowState.ts +++ b/src/components/Table/utils/RowState.ts @@ -371,7 +371,7 @@ export class RowState { const { visibleColumns, search } = this.states.table; return search.every((term) => visibleColumns - .map((c) => applyRowFn(c, this.row, this.api, 0, false, false)) + .map((c) => applyRowFn(c, this.row, this.api, 0, false)) .some((maybeContent) => matchesFilter(maybeContent, term)), ); } finally { diff --git a/src/components/Table/utils/sortRows.ts b/src/components/Table/utils/sortRows.ts index 1d85d0ed5..43a16a657 100644 --- a/src/components/Table/utils/sortRows.ts +++ b/src/components/Table/utils/sortRows.ts @@ -73,8 +73,8 @@ function compare( invert: boolean, caseSensitive: boolean, ) { - const v1 = sortValue(applyRowFn(column, a, {} as any, 0, false, false), caseSensitive); - const v2 = sortValue(applyRowFn(column, b, {} as any, 0, false, false), caseSensitive); + const v1 = sortValue(applyRowFn(column, a, {} as any, 0, false), caseSensitive); + const v2 = sortValue(applyRowFn(column, b, {} as any, 0, false), caseSensitive); const v1e = v1 === null || v1 === undefined; const v2e = v2 === null || v2 === undefined; if ((v1e && v2e) || v1 === v2) { diff --git a/src/components/Table/utils/utils.tsx b/src/components/Table/utils/utils.tsx index cad2b3c3d..14a33dc02 100644 --- a/src/components/Table/utils/utils.tsx +++ b/src/components/Table/utils/utils.tsx @@ -146,7 +146,6 @@ export function applyRowFn( api: GridRowApi, level: number, expanded: boolean, - editable: boolean, dragData?: DragData, ): ReactNode | GridCellContent { // Usually this is a function to apply against the row, but sometimes it's a hard-coded value, i.e. for headers @@ -158,7 +157,6 @@ export function applyRowFn( api, level, expanded, - editable, dragData, }); } else { From 50b9c127804ff4ab877da179265c1b77c93cea77 Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Thu, 21 Nov 2024 12:02:32 -0500 Subject: [PATCH 05/14] fix: use just CSS for cell highlighting --- src/components/Table/GridTable.stories.tsx | 11 ++++++-- src/components/Table/GridTable.tsx | 1 + src/components/Table/TableStyles.tsx | 1 + src/components/Table/components/Row.tsx | 33 ++++++++-------------- src/components/Table/components/cell.tsx | 6 +--- 5 files changed, 23 insertions(+), 29 deletions(-) diff --git a/src/components/Table/GridTable.stories.tsx b/src/components/Table/GridTable.stories.tsx index a6ab0846f..e1f3bf2f3 100644 --- a/src/components/Table/GridTable.stories.tsx +++ b/src/components/Table/GridTable.stories.tsx @@ -15,6 +15,7 @@ import { defaultStyle, dragHandleColumn, emptyCell, + getTableStyles, GridCellAlignment, GridColumn, GridDataRow, @@ -2204,7 +2205,7 @@ export function EditableRows() { ), editableOnHover: true, }), - w: "100px", + w: "120px", }; const date1Column: GridColumn = { @@ -2241,11 +2242,17 @@ export function EditableRows() { w: "120px", }; + const style = getTableStyles({ bordered: true, allWhite: true }); + return ( ); } diff --git a/src/components/Table/GridTable.tsx b/src/components/Table/GridTable.tsx index 7863f3c10..5ebf2b52d 100644 --- a/src/components/Table/GridTable.tsx +++ b/src/components/Table/GridTable.tsx @@ -260,6 +260,7 @@ export function GridTable = an }; const style = resolveStyles(maybeStyle); + console.log(style); const { tableState } = api; tableState.onRowSelect = onRowSelect; diff --git a/src/components/Table/TableStyles.tsx b/src/components/Table/TableStyles.tsx index 62e2151af..7a0052f4b 100644 --- a/src/components/Table/TableStyles.tsx +++ b/src/components/Table/TableStyles.tsx @@ -175,6 +175,7 @@ function memoizedTableStyles() { presentationSettings: { borderless: true, typeScale: "xs", wrap: rowHeight === "flexible" }, levels: grouped ? groupedLevels : defaultLevels, rowHoverColor: Palette.Blue100, + rowEditableCellBorderColor: Palette.Blue300, keptGroupRowCss: Css.bgYellow100.gray900.xsMd.df.aic.$, keptLastRowCss: Css.boxShadow("inset 0px -14px 8px -11px rgba(63,63,63,.18)").$, }; diff --git a/src/components/Table/components/Row.tsx b/src/components/Table/components/Row.tsx index e0478cd6c..84e36c01d 100644 --- a/src/components/Table/components/Row.tsx +++ b/src/components/Table/components/Row.tsx @@ -125,10 +125,11 @@ function RowImpl(props: RowProps): ReactElement { [`:hover > .${revealOnRowHoverClass} > *`]: Css.vv.$, }, ...{ - [`.textFieldBaseWrapper`]: Css.pl1.mlPx(-8).br4.ba.bcTransparent.$, - [`:hover > .${editableOnRowHoverClass} .textFieldBaseWrapper`]: Css.ba.bc( - style.rowEditableCellBorderColor ?? Palette.Blue300, - ).$, + [`.textFieldBaseWrapper`]: Css.pl1.mlPx(-8).br4.ba.bcTransparent.bgTransparent.$, + [`.textFieldBaseWrapper > *`]: Css.bgTransparent.$, + [`.${editableOnRowHoverClass}:hover .textFieldBaseWrapper`]: Css.bgBlue100.$, + [`:hover:not(:has(.${editableOnRowHoverClass}:hover)) > .${editableOnRowHoverClass} .textFieldBaseWrapper, .${editableOnRowHoverClass}:hover .textFieldBaseWrapper`]: + Css.ba.bc(style.rowEditableCellBorderColor ?? Palette.Blue300).$, }, ...(isLastKeptRow && Css.addIn("&>*", style.keptLastRowCss).$), }; @@ -231,7 +232,7 @@ function RowImpl(props: RowProps): ReactElement { ? numExpandedColumns + 1 : 1; const revealOnRowHover = isGridCellContent(maybeContent) ? maybeContent.revealOnRowHover : false; - const editableOnRowHover = isGridCellContent(maybeContent) ? maybeContent.editableOnHover : false; + const editableOnRowHover = isGridCellContent(maybeContent) && !!maybeContent.editableOnHover; const canSortColumn = (sortOn === "client" && column.clientSideSort !== false) || @@ -334,6 +335,8 @@ function RowImpl(props: RowProps): ReactElement { ...(isGridCellContent(maybeContent) && maybeContent.css ? maybeContent.css : {}), // Apply cell highlight styles to active cell and hover ...Css.if(applyCellHighlight && isCellActive).br4.boxShadow(`inset 0 0 0 1px ${Palette.Blue700}`).$, + ...Css.if(editableOnRowHover && isCellActive).addIn("& .textFieldBaseWrapper", Css.bgBlue50.ba.bcBlue500.$) + .$, // Define the width of the column on each cell. Supports col spans. // If we have a 'levelIndent' defined, then subtract that amount from the first content column's width to ensure all columns will still line up properly width: `calc(${columnSizes.slice(columnIndex, columnIndex + currentColspan).join(" + ")}${ @@ -343,14 +346,10 @@ function RowImpl(props: RowProps): ReactElement { const cellClassNames = [ ...(revealOnRowHover ? [revealOnRowHoverClass] : []), - ...(editableOnRowHover && (isCellActive || !tableState.activeCellId) ? [editableOnRowHoverClass] : []), + ...(editableOnRowHover ? [editableOnRowHoverClass] : []), ].join(" "); - const cellOnHover = - isGridCellContent(maybeContent) && maybeContent.editableOnHover - ? (enter: boolean) => (enter ? api.setActiveCellId(cellId) : api.setActiveCellId(undefined)) - : undefined; - const cellOnClick = applyCellHighlight ? () => api.setActiveCellId(cellId) : undefined; + const cellOnClick = applyCellHighlight || editableOnRowHover ? () => api.setActiveCellId(cellId) : undefined; const tooltip = isGridCellContent(maybeContent) ? maybeContent.tooltip : undefined; const renderFn: RenderCellFn = @@ -362,17 +361,7 @@ function RowImpl(props: RowProps): ReactElement { ? rowClickRenderFn(as, api, currentColspan) : defaultRenderFn(as, currentColspan); - return renderFn( - columnIndex, - cellCss, - content, - row, - rowStyle, - cellClassNames, - cellOnClick, - cellOnHover, - tooltip, - ); + return renderFn(columnIndex, cellCss, content, row, rowStyle, cellClassNames, cellOnClick, tooltip); }) )} diff --git a/src/components/Table/components/cell.tsx b/src/components/Table/components/cell.tsx index 01ffa48f7..3e30226d2 100644 --- a/src/components/Table/components/cell.tsx +++ b/src/components/Table/components/cell.tsx @@ -47,14 +47,12 @@ export type RenderCellFn = ( rowStyle: RowStyle | undefined, classNames: string | undefined, onClick: VoidFunction | undefined, - onHover: ((enter: boolean) => void) | undefined, tooltip: ReactNode | undefined, ) => ReactNode; /** Renders our default cell element, i.e. if no row links and no custom renderCell are used. */ export const defaultRenderFn: (as: RenderAs, colSpan: number) => RenderCellFn = - (as: RenderAs, colSpan) => - (key, css, content, row, rowStyle, classNames: string | undefined, onClick, onHover, tooltip) => { + (as: RenderAs, colSpan) => (key, css, content, row, rowStyle, classNames: string | undefined, onClick, tooltip) => { const Cell = as === "table" ? "td" : "div"; return ( RenderCellFn onHover?.(true)} - onMouseLeave={() => onHover?.(false)} {...(as === "table" && { colSpan })} > {content} From 47b5bdc853dfe72bb1a9cc8c68d2fd6be04bfa54 Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Thu, 21 Nov 2024 12:27:01 -0500 Subject: [PATCH 06/14] fix: update CSS for editable cell hover effects --- src/components/Table/components/Row.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/Table/components/Row.tsx b/src/components/Table/components/Row.tsx index 84e36c01d..224cf83c4 100644 --- a/src/components/Table/components/Row.tsx +++ b/src/components/Table/components/Row.tsx @@ -125,8 +125,8 @@ function RowImpl(props: RowProps): ReactElement { [`:hover > .${revealOnRowHoverClass} > *`]: Css.vv.$, }, ...{ - [`.textFieldBaseWrapper`]: Css.pl1.mlPx(-8).br4.ba.bcTransparent.bgTransparent.$, - [`.textFieldBaseWrapper > *`]: Css.bgTransparent.$, + [`.${editableOnRowHoverClass} .textFieldBaseWrapper`]: Css.pl1.mlPx(-8).br4.ba.bcTransparent.bgTransparent.$, + [`.${editableOnRowHoverClass} .textFieldBaseWrapper > *`]: Css.bgTransparent.$, [`.${editableOnRowHoverClass}:hover .textFieldBaseWrapper`]: Css.bgBlue100.$, [`:hover:not(:has(.${editableOnRowHoverClass}:hover)) > .${editableOnRowHoverClass} .textFieldBaseWrapper, .${editableOnRowHoverClass}:hover .textFieldBaseWrapper`]: Css.ba.bc(style.rowEditableCellBorderColor ?? Palette.Blue300).$, From bd79761cecad1344a173b989e639bf64a72fd9ba Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Thu, 28 Nov 2024 13:50:34 -0500 Subject: [PATCH 07/14] fix: use presentation context --- src/components/PresentationContext.tsx | 1 + src/components/Table/GridTable.stories.tsx | 25 +++++++++++----------- src/components/Table/GridTable.tsx | 4 +++- src/components/Table/TableStyles.tsx | 13 +++++++++-- src/components/Table/components/Row.tsx | 14 ++++++------ src/components/Table/components/cell.tsx | 2 -- src/components/Table/types.ts | 2 -- src/inputs/TextFieldBase.tsx | 18 ++++++++++++---- 8 files changed, 47 insertions(+), 32 deletions(-) diff --git a/src/components/PresentationContext.tsx b/src/components/PresentationContext.tsx index 0bec8d15d..6d86e1627 100644 --- a/src/components/PresentationContext.tsx +++ b/src/components/PresentationContext.tsx @@ -11,6 +11,7 @@ export interface PresentationFieldProps { labelSuffix?: LabelSuffixStyle; // Typically used for compact fields in a table. Removes border and uses an box-shadow for focus behavior borderless?: boolean; + borderOnHover?: boolean; // Defines height of the field compact?: boolean; // Changes default font styles for input fields and Chips diff --git a/src/components/Table/GridTable.stories.tsx b/src/components/Table/GridTable.stories.tsx index e1f3bf2f3..72df8435d 100644 --- a/src/components/Table/GridTable.stories.tsx +++ b/src/components/Table/GridTable.stories.tsx @@ -2203,7 +2203,6 @@ export function EditableRows() { onSelect={(status) => handleCellChange(row.id, "status", status)} /> ), - editableOnHover: true, }), w: "120px", }; @@ -2220,7 +2219,6 @@ export function EditableRows() { format="medium" /> ), - editableOnHover: true, }), w: "120px", }; @@ -2237,22 +2235,23 @@ export function EditableRows() { format="medium" /> ), - editableOnHover: true, }), w: "120px", }; - const style = getTableStyles({ bordered: true, allWhite: true }); + const style = getTableStyles({ bordered: true, allWhite: true, highlightOnHover: true }); return ( - +
+ +
); } diff --git a/src/components/Table/GridTable.tsx b/src/components/Table/GridTable.tsx index 5ebf2b52d..b5f15bed8 100644 --- a/src/components/Table/GridTable.tsx +++ b/src/components/Table/GridTable.tsx @@ -471,6 +471,7 @@ export function GridTable = an const borderless = style?.presentationSettings?.borderless; const typeScale = style?.presentationSettings?.typeScale; + const borderOnHover = style?.presentationSettings?.borderOnHover; const fieldProps: PresentationFieldProps = useMemo( () => ({ labelStyle: "hidden", @@ -480,8 +481,9 @@ export function GridTable = an // Avoid passing `undefined` as it will unset existing PresentationContext settings ...(borderless !== undefined ? { borderless } : {}), ...(typeScale !== undefined ? { typeScale } : {}), + ...(borderOnHover !== undefined ? { borderOnHover } : {}), }), - [borderless, typeScale], + [borderOnHover, borderless, typeScale], ); // If we're running in Jest, force using `as=div` b/c jsdom doesn't support react-virtuoso. diff --git a/src/components/Table/TableStyles.tsx b/src/components/Table/TableStyles.tsx index 7a0052f4b..185060a40 100644 --- a/src/components/Table/TableStyles.tsx +++ b/src/components/Table/TableStyles.tsx @@ -48,7 +48,7 @@ export interface GridStyle { nonHeaderRowHoverCss?: Properties; /** Default content to put into an empty cell */ emptyCell?: ReactNode; - presentationSettings?: Pick & + presentationSettings?: Pick & Pick; /** Minimum table width in pixels. Used when calculating columns sizes */ minWidthPx?: number; @@ -92,6 +92,8 @@ export interface GridStyleDef { vAlign?: "top" | "center" | "bottom"; /** Defines the Typography for the table body's cells (not the header). This only applies to rows that are not nested/grouped */ cellTypography?: Typography; + /** */ + highlightOnHover?: boolean; } // Returns a "blessed" style of GridTable @@ -107,6 +109,7 @@ function memoizedTableStyles() { bordered = false, vAlign = "center", cellTypography = "xs" as const, + highlightOnHover = true, } = props; const key = safeKeys(props) @@ -172,7 +175,12 @@ function memoizedTableStyles() { Css.borderRadius("0 0 8px 0").$, ).$ : Css.addIn("> *", Css.bsh0.$).$, - presentationSettings: { borderless: true, typeScale: "xs", wrap: rowHeight === "flexible" }, + presentationSettings: { + borderless: true, + typeScale: "xs", + wrap: rowHeight === "flexible", + borderOnHover: highlightOnHover, + }, levels: grouped ? groupedLevels : defaultLevels, rowHoverColor: Palette.Blue100, rowEditableCellBorderColor: Palette.Blue300, @@ -251,6 +259,7 @@ export function resolveStyles(style: GridStyle | GridStyleDef): GridStyle { rowHover: true, vAlign: true, cellTypography: true, + highlightOnHover: true, }; const keys = safeKeys(style); const defKeys = safeKeys(defKeysRecord); diff --git a/src/components/Table/components/Row.tsx b/src/components/Table/components/Row.tsx index 224cf83c4..a3cb74abb 100644 --- a/src/components/Table/components/Row.tsx +++ b/src/components/Table/components/Row.tsx @@ -125,10 +125,8 @@ function RowImpl(props: RowProps): ReactElement { [`:hover > .${revealOnRowHoverClass} > *`]: Css.vv.$, }, ...{ - [`.${editableOnRowHoverClass} .textFieldBaseWrapper`]: Css.pl1.mlPx(-8).br4.ba.bcTransparent.bgTransparent.$, - [`.${editableOnRowHoverClass} .textFieldBaseWrapper > *`]: Css.bgTransparent.$, [`.${editableOnRowHoverClass}:hover .textFieldBaseWrapper`]: Css.bgBlue100.$, - [`:hover:not(:has(.${editableOnRowHoverClass}:hover)) > .${editableOnRowHoverClass} .textFieldBaseWrapper, .${editableOnRowHoverClass}:hover .textFieldBaseWrapper`]: + [`:hover:not(:has(.textFieldBaseWrapper:hover)) > .${editableOnRowHoverClass} .textFieldBaseWrapper, .${editableOnRowHoverClass}:hover .textFieldBaseWrapper`]: Css.ba.bc(style.rowEditableCellBorderColor ?? Palette.Blue300).$, }, ...(isLastKeptRow && Css.addIn("&>*", style.keptLastRowCss).$), @@ -232,7 +230,7 @@ function RowImpl(props: RowProps): ReactElement { ? numExpandedColumns + 1 : 1; const revealOnRowHover = isGridCellContent(maybeContent) ? maybeContent.revealOnRowHover : false; - const editableOnRowHover = isGridCellContent(maybeContent) && !!maybeContent.editableOnHover; + const borderOnHover = style.presentationSettings?.borderOnHover ?? false; const canSortColumn = (sortOn === "client" && column.clientSideSort !== false) || @@ -335,8 +333,8 @@ function RowImpl(props: RowProps): ReactElement { ...(isGridCellContent(maybeContent) && maybeContent.css ? maybeContent.css : {}), // Apply cell highlight styles to active cell and hover ...Css.if(applyCellHighlight && isCellActive).br4.boxShadow(`inset 0 0 0 1px ${Palette.Blue700}`).$, - ...Css.if(editableOnRowHover && isCellActive).addIn("& .textFieldBaseWrapper", Css.bgBlue50.ba.bcBlue500.$) - .$, + // Apply cell hover styles when the row is hovered + ...Css.if(borderOnHover && isCellActive).addIn("& .textFieldBaseWrapper", Css.bgBlue50.ba.bcBlue500.$).$, // Define the width of the column on each cell. Supports col spans. // If we have a 'levelIndent' defined, then subtract that amount from the first content column's width to ensure all columns will still line up properly width: `calc(${columnSizes.slice(columnIndex, columnIndex + currentColspan).join(" + ")}${ @@ -346,10 +344,10 @@ function RowImpl(props: RowProps): ReactElement { const cellClassNames = [ ...(revealOnRowHover ? [revealOnRowHoverClass] : []), - ...(editableOnRowHover ? [editableOnRowHoverClass] : []), + ...(borderOnHover ? [editableOnRowHoverClass] : []), ].join(" "); - const cellOnClick = applyCellHighlight || editableOnRowHover ? () => api.setActiveCellId(cellId) : undefined; + const cellOnClick = applyCellHighlight ? () => api.setActiveCellId(cellId) : undefined; const tooltip = isGridCellContent(maybeContent) ? maybeContent.tooltip : undefined; const renderFn: RenderCellFn = diff --git a/src/components/Table/components/cell.tsx b/src/components/Table/components/cell.tsx index 3e30226d2..4cc0f29f2 100644 --- a/src/components/Table/components/cell.tsx +++ b/src/components/Table/components/cell.tsx @@ -34,8 +34,6 @@ export type GridCellContent = { revealOnRowHover?: true; /** Tooltip to add to a cell */ tooltip?: ReactNode; - /** Allows cell to be editable when hovering, and also highlights the field on hover */ - editableOnHover?: true; }; /** Allows rendering a specific cell. */ diff --git a/src/components/Table/types.ts b/src/components/Table/types.ts index 6efd7eb89..97bba13cf 100644 --- a/src/components/Table/types.ts +++ b/src/components/Table/types.ts @@ -85,8 +85,6 @@ export type GridColumn = { initExpanded?: boolean; /** Determines whether this column should be hidden when expanded (only the 'expandColumns' would show) */ hideOnExpand?: boolean; - /** Flag that changes the field behavior to be editable on hover */ - editableOnHover?: boolean; }; /** diff --git a/src/inputs/TextFieldBase.tsx b/src/inputs/TextFieldBase.tsx index cea445799..774570424 100644 --- a/src/inputs/TextFieldBase.tsx +++ b/src/inputs/TextFieldBase.tsx @@ -37,6 +37,7 @@ export interface TextFieldBaseProps | "placeholder" | "compact" | "borderless" + | "borderOnHover" | "visuallyDisabled" | "fullWidth" | "xss" @@ -90,6 +91,7 @@ export function TextFieldBase>(props: TextFieldB labelStyle = fieldProps?.labelStyle ?? "above", contrast = false, borderless = fieldProps?.borderless ?? false, + borderOnHover = fieldProps?.borderOnHover ?? false, textAreaMinHeight = 96, clearable = false, tooltip, @@ -120,9 +122,12 @@ export function TextFieldBase>(props: TextFieldB const [bgColor, hoverBgColor, disabledBgColor] = contrast ? [Palette.Gray700, Palette.Gray600, Palette.Gray700] - : borderless && !compound - ? [Palette.Gray100, Palette.Gray200, Palette.Gray200] - : [Palette.White, Palette.Gray100, Palette.Gray100]; + : borderOnHover + ? // Use transparent backgrounds to blend with the table row hover color + [Palette.Transparent, Palette.Transparent, Palette.Gray100] + : borderless && !compound + ? [Palette.Gray100, Palette.Gray200, Palette.Gray200] + : [Palette.White, Palette.Gray100, Palette.Gray100]; const fieldMaxWidth = getFieldWidth(fullWidth); @@ -143,6 +148,8 @@ export function TextFieldBase>(props: TextFieldB : Css.bcGray300.if(contrast).bcGray700.$), // Do not add borders to compound fields. A compound field is responsible for drawing its own borders ...(!compound ? Css.ba.$ : {}), + ...(borderOnHover && Css.pl1.ml(-1).br4.ba.bcTransparent.$), + ...(borderOnHover && Css.if(isHovered).bgBlue100.ba.bcBlue300.$), // When multiline is true, then we want to allow the field to grow to the height of the content, but not shrink below the minHeight // Otherwise, set fixed heights values accordingly. ...(multiline @@ -168,11 +175,13 @@ export function TextFieldBase>(props: TextFieldB ...Css.w100.mw0.outline0.fg1.bgColor(bgColor).$, // Not using Truss's inline `if` statement here because `addIn` properties do not respect the if statement. ...(contrast && Css.addIn("&::selection", Css.bgGray800.$).$), + // Make the background transparent when highlighting the field on hover + ...(borderOnHover && Css.bgTransparent.$), // For "multiline" fields we add top and bottom padding of 7px for compact, or 11px for non-compact, to properly match the height of the single line fields ...(multiline ? Css.br4.pyPx(compact ? 7 : 11).add("resize", "none").$ : Css.truncate.$), }, hover: Css.bgColor(hoverBgColor).if(contrast).bcGray600.$, - focus: Css.bcBlue700.if(contrast).bcBlue500.$, + focus: Css.bcBlue700.if(contrast).bcBlue500.if(borderOnHover).bgBlue100.bcBlue300.$, disabled: visuallyDisabled ? Css.cursorNotAllowed.gray600.bgColor(disabledBgColor).if(contrast).gray500.$ : Css.cursorNotAllowed.$, @@ -260,6 +269,7 @@ export function TextFieldBase>(props: TextFieldB ...(errorMsg && !inputProps.disabled ? fieldStyles.error : {}), ...Css.if(multiline).aifs.oh.mhPx(textAreaMinHeight).$, }} + // Class name used for the grid table on row hover for highlighting className="textFieldBaseWrapper" {...hoverProps} ref={inputWrapRef as any} From 2a8c8c6a8f3cf4b5d69c997fe34761ad7424e5f6 Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Mon, 2 Dec 2024 11:01:09 -0500 Subject: [PATCH 08/14] fix: remove console log --- src/components/Table/GridTable.tsx | 1 - src/components/Table/TableStyles.tsx | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/Table/GridTable.tsx b/src/components/Table/GridTable.tsx index b5f15bed8..ad8acbe9d 100644 --- a/src/components/Table/GridTable.tsx +++ b/src/components/Table/GridTable.tsx @@ -260,7 +260,6 @@ export function GridTable = an }; const style = resolveStyles(maybeStyle); - console.log(style); const { tableState } = api; tableState.onRowSelect = onRowSelect; diff --git a/src/components/Table/TableStyles.tsx b/src/components/Table/TableStyles.tsx index 185060a40..217eb3350 100644 --- a/src/components/Table/TableStyles.tsx +++ b/src/components/Table/TableStyles.tsx @@ -182,7 +182,7 @@ function memoizedTableStyles() { borderOnHover: highlightOnHover, }, levels: grouped ? groupedLevels : defaultLevels, - rowHoverColor: Palette.Blue100, + rowHoverColor: Palette.Blue50, rowEditableCellBorderColor: Palette.Blue300, keptGroupRowCss: Css.bgYellow100.gray900.xsMd.df.aic.$, keptLastRowCss: Css.boxShadow("inset 0px -14px 8px -11px rgba(63,63,63,.18)").$, From fdec08f4d5980d2fc49ae5789f3dcaa7e82d4114 Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Mon, 2 Dec 2024 13:45:37 -0500 Subject: [PATCH 09/14] fix: PR feedback --- src/components/Table/components/Row.tsx | 18 +++++------------- src/inputs/TextFieldBase.tsx | 8 +++++++- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/components/Table/components/Row.tsx b/src/components/Table/components/Row.tsx index a3cb74abb..559386537 100644 --- a/src/components/Table/components/Row.tsx +++ b/src/components/Table/components/Row.tsx @@ -88,7 +88,6 @@ function RowImpl(props: RowProps): ReactElement { const sortOn = tableState.sortConfig?.on; const revealOnRowHoverClass = "revealOnRowHover"; - const editableOnRowHoverClass = "editableOnRowHover"; const showRowHoverColor = !reservedRowKinds.includes(row.kind) && !omitRowHover && style.rowHoverColor !== "none"; @@ -124,11 +123,6 @@ function RowImpl(props: RowProps): ReactElement { [` > .${revealOnRowHoverClass} > *`]: Css.vh.$, [`:hover > .${revealOnRowHoverClass} > *`]: Css.vv.$, }, - ...{ - [`.${editableOnRowHoverClass}:hover .textFieldBaseWrapper`]: Css.bgBlue100.$, - [`:hover:not(:has(.textFieldBaseWrapper:hover)) > .${editableOnRowHoverClass} .textFieldBaseWrapper, .${editableOnRowHoverClass}:hover .textFieldBaseWrapper`]: - Css.ba.bc(style.rowEditableCellBorderColor ?? Palette.Blue300).$, - }, ...(isLastKeptRow && Css.addIn("&>*", style.keptLastRowCss).$), }; @@ -151,7 +145,7 @@ function RowImpl(props: RowProps): ReactElement { const onDragOverDebounced = useDebouncedCallback(dragOverCallback, 100); const RowContent = () => ( - + {isKeptGroupRow ? ( ) : ( @@ -333,8 +327,6 @@ function RowImpl(props: RowProps): ReactElement { ...(isGridCellContent(maybeContent) && maybeContent.css ? maybeContent.css : {}), // Apply cell highlight styles to active cell and hover ...Css.if(applyCellHighlight && isCellActive).br4.boxShadow(`inset 0 0 0 1px ${Palette.Blue700}`).$, - // Apply cell hover styles when the row is hovered - ...Css.if(borderOnHover && isCellActive).addIn("& .textFieldBaseWrapper", Css.bgBlue50.ba.bcBlue500.$).$, // Define the width of the column on each cell. Supports col spans. // If we have a 'levelIndent' defined, then subtract that amount from the first content column's width to ensure all columns will still line up properly width: `calc(${columnSizes.slice(columnIndex, columnIndex + currentColspan).join(" + ")}${ @@ -342,10 +334,7 @@ function RowImpl(props: RowProps): ReactElement { })`, }; - const cellClassNames = [ - ...(revealOnRowHover ? [revealOnRowHoverClass] : []), - ...(borderOnHover ? [editableOnRowHoverClass] : []), - ].join(" "); + const cellClassNames = [CELL_CSS_SELECTOR, ...(revealOnRowHover ? [revealOnRowHoverClass] : [])].join(" "); const cellOnClick = applyCellHighlight ? () => api.setActiveCellId(cellId) : undefined; const tooltip = isGridCellContent(maybeContent) ? maybeContent.tooltip : undefined; @@ -441,3 +430,6 @@ export type GridDataRow = { /** Whether this row is draggable, usually to allow drag & drop reordering of rows */ draggable?: boolean; } & IfAny>; + +export const ROW_CSS_SELECTOR = "Row"; +export const CELL_CSS_SELECTOR = "Cell"; diff --git a/src/inputs/TextFieldBase.tsx b/src/inputs/TextFieldBase.tsx index 774570424..c6dc68211 100644 --- a/src/inputs/TextFieldBase.tsx +++ b/src/inputs/TextFieldBase.tsx @@ -14,6 +14,7 @@ import { Icon, IconButton, maybeTooltip } from "src/components"; import { HelperText } from "src/components/HelperText"; import { InlineLabel, Label } from "src/components/Label"; import { usePresentationContext } from "src/components/PresentationContext"; +import { CELL_CSS_SELECTOR, ROW_CSS_SELECTOR } from "src/components/Table/components/Row"; import { Css, Only, Palette } from "src/Css"; import { getLabelSuffix } from "src/forms/labelUtils"; import { useGetRef } from "src/hooks/useGetRef"; @@ -150,6 +151,11 @@ export function TextFieldBase>(props: TextFieldB ...(!compound ? Css.ba.$ : {}), ...(borderOnHover && Css.pl1.ml(-1).br4.ba.bcTransparent.$), ...(borderOnHover && Css.if(isHovered).bgBlue100.ba.bcBlue300.$), + ...{ + // Highlight the field when hovering over the row in a table + [`.${ROW_CSS_SELECTOR}:hover:not(:has(.textFieldBaseWrapper:hover)) &`]: Css.ba.bcBlue300.$, + [`.${CELL_CSS_SELECTOR}:hover &`]: Css.bgBlue100.$, + }, // When multiline is true, then we want to allow the field to grow to the height of the content, but not shrink below the minHeight // Otherwise, set fixed heights values accordingly. ...(multiline @@ -181,7 +187,7 @@ export function TextFieldBase>(props: TextFieldB ...(multiline ? Css.br4.pyPx(compact ? 7 : 11).add("resize", "none").$ : Css.truncate.$), }, hover: Css.bgColor(hoverBgColor).if(contrast).bcGray600.$, - focus: Css.bcBlue700.if(contrast).bcBlue500.if(borderOnHover).bgBlue100.bcBlue300.$, + focus: Css.bcBlue700.if(contrast).bcBlue500.if(borderOnHover).bgBlue100.bcBlue500.$, disabled: visuallyDisabled ? Css.cursorNotAllowed.gray600.bgColor(disabledBgColor).if(contrast).gray500.$ : Css.cursorNotAllowed.$, From 3c13d728c4fda7da4a6af9d2f9eed3b6e6617f95 Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Mon, 2 Dec 2024 15:13:26 -0500 Subject: [PATCH 10/14] fix: clean up PR --- src/components/Table/GridTable.stories.tsx | 1 - src/components/Table/TableStyles.tsx | 5 +---- src/components/Table/components/Row.tsx | 9 +++++---- src/components/Table/types.ts | 4 ++-- src/components/Table/utils/utils.tsx | 8 +------- 5 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/components/Table/GridTable.stories.tsx b/src/components/Table/GridTable.stories.tsx index 72df8435d..f552cfd33 100644 --- a/src/components/Table/GridTable.stories.tsx +++ b/src/components/Table/GridTable.stories.tsx @@ -2249,7 +2249,6 @@ export function EditableRows() { style={{ ...style, rowHoverColor: Palette.Blue50, - rowEditableCellBorderColor: Palette.Blue300, }} /> diff --git a/src/components/Table/TableStyles.tsx b/src/components/Table/TableStyles.tsx index 217eb3350..a3fc354a2 100644 --- a/src/components/Table/TableStyles.tsx +++ b/src/components/Table/TableStyles.tsx @@ -42,8 +42,6 @@ export interface GridStyle { firstRowMessageCss?: Properties; /** Applied on hover if a row has a rowLink/onClick set. */ rowHoverColor?: Palette | "none"; - /** Applied on hover to a cell TextFieldBase */ - rowEditableCellBorderColor?: Palette; /** Applied on hover of a row */ nonHeaderRowHoverCss?: Properties; /** Default content to put into an empty cell */ @@ -92,7 +90,7 @@ export interface GridStyleDef { vAlign?: "top" | "center" | "bottom"; /** Defines the Typography for the table body's cells (not the header). This only applies to rows that are not nested/grouped */ cellTypography?: Typography; - /** */ + /** Defines if the table should highlight the row on hover. Defaults to true */ highlightOnHover?: boolean; } @@ -183,7 +181,6 @@ function memoizedTableStyles() { }, levels: grouped ? groupedLevels : defaultLevels, rowHoverColor: Palette.Blue50, - rowEditableCellBorderColor: Palette.Blue300, keptGroupRowCss: Css.bgYellow100.gray900.xsMd.df.aic.$, keptLastRowCss: Css.boxShadow("inset 0px -14px 8px -11px rgba(63,63,63,.18)").$, }; diff --git a/src/components/Table/components/Row.tsx b/src/components/Table/components/Row.tsx index 559386537..b7629ca58 100644 --- a/src/components/Table/components/Row.tsx +++ b/src/components/Table/components/Row.tsx @@ -210,10 +210,6 @@ function RowImpl(props: RowProps): ReactElement { onDragOver: onDragOverDebounced, }; - const cellId = `${row.kind}_${row.id}_${column.id}`; - const applyCellHighlight = cellHighlight && !!column.id && !isHeader && !isTotals; - const isCellActive = tableState.activeCellId === cellId; - const maybeContent = applyRowFn(column as GridColumnWithId, row, rowApi, level, isExpanded, dragData); // Only use the `numExpandedColumns` as the `colspan` when rendering the "Expandable Header" @@ -279,6 +275,11 @@ function RowImpl(props: RowProps): ReactElement { // This relies on our column sizes being defined in pixel values, which is currently true as we calculate to pixel values in the `useSetupColumnSizes` hook minStickyLeftOffset += maybeSticky === "left" ? parseInt(columnSizes[columnIndex].replace("px", ""), 10) : 0; + + const cellId = `${row.kind}_${row.id}_${column.id}`; + const applyCellHighlight = cellHighlight && !!column.id && !isHeader && !isTotals; + const isCellActive = tableState.activeCellId === cellId; + // Note that it seems expensive to calc a per-cell class name/CSS-in-JS output, // vs. setting global/table-wide CSS like `style.cellCss` on the root grid div with // a few descendent selectors. However, that approach means the root grid-applied diff --git a/src/components/Table/types.ts b/src/components/Table/types.ts index 97bba13cf..4c727b8e4 100644 --- a/src/components/Table/types.ts +++ b/src/components/Table/types.ts @@ -39,11 +39,11 @@ export type GridColumn = { | (DiscriminateUnion extends { data: infer D } ? ( data: D, - opts: { row: GridRowKind; api: GridRowApi; level: number; expanded: boolean; editable: boolean }, + opts: { row: GridRowKind; api: GridRowApi; level: number; expanded: boolean }, ) => ReactNode | GridCellContent : ( data: undefined, - opts: { row: GridRowKind; api: GridRowApi; level: number; expanded: boolean; editable: boolean }, + opts: { row: GridRowKind; api: GridRowApi; level: number; expanded: boolean }, ) => ReactNode | GridCellContent); } & { /** diff --git a/src/components/Table/utils/utils.tsx b/src/components/Table/utils/utils.tsx index 14a33dc02..afd30de1c 100644 --- a/src/components/Table/utils/utils.tsx +++ b/src/components/Table/utils/utils.tsx @@ -152,13 +152,7 @@ export function applyRowFn( const maybeContent = column[row.kind]; if (typeof maybeContent === "function") { // Auto-destructure data - return (maybeContent as Function)((row as any)["data"], { - row: row as any, - api, - level, - expanded, - dragData, - }); + return (maybeContent as Function)((row as any)["data"], { row: row as any, api, level, expanded, dragData }); } else { return maybeContent; } From b3658bffb875a88cb64829368097faf18eeade1f Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Mon, 2 Dec 2024 15:54:25 -0500 Subject: [PATCH 11/14] fix: PR feedback --- src/components/Table/GridTable.stories.tsx | 22 +++++++++++----------- src/inputs/TextFieldBase.tsx | 5 ++--- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/components/Table/GridTable.stories.tsx b/src/components/Table/GridTable.stories.tsx index f552cfd33..fdf72ede7 100644 --- a/src/components/Table/GridTable.stories.tsx +++ b/src/components/Table/GridTable.stories.tsx @@ -2148,8 +2148,8 @@ export function MinColumnWidths() { } enum EditableRowStatus { - Active = "Active", - Inactive = "Inactive", + Foo = "Foo", + Bar = "Bar", } type EditableRowData = { @@ -2159,33 +2159,33 @@ type EditableRowData = { }; type EditableRow = EditableRowData | HeaderRow; -export function EditableRows() { +export function HighlightFields() { const [rows, setRows] = useState[]>([ simpleHeader, { kind: "data" as const, id: "1", - data: { id: "1", name: "Tony Stark", status: EditableRowStatus.Active, value: 1, date: jan1 }, + data: { id: "1", name: "Tony Stark", status: EditableRowStatus.Foo, value: 1, date: jan1 }, }, { kind: "data" as const, id: "2", - data: { id: "2", name: "Natasha Romanova", status: EditableRowStatus.Active, value: 2, date: jan2 }, + data: { id: "2", name: "Natasha Romanova", status: EditableRowStatus.Foo, value: 2, date: jan2 }, }, { kind: "data" as const, id: "3", - data: { id: "3", name: "Thor Odinson", status: EditableRowStatus.Active, value: 3, date: jan29 }, + data: { id: "3", name: "Thor Odinson", status: EditableRowStatus.Bar, value: 3, date: jan29 }, }, ]); - const handleCellChange = (rowId: string, field: keyof EditableRowData["data"], value: any) => { + const setRow = useCallback((rowId: string, field: keyof EditableRowData["data"], value: any) => { setRows((rows) => rows.map((row) => row.kind === "data" && row.id === rowId ? { ...row, data: { ...row.data, [field]: value } } : row, ), ); - }; + }, []); const nameColumn: GridColumn = { header: "Name", @@ -2200,7 +2200,7 @@ export function EditableRows() { label="" options={Object.values(EditableRowStatus).map((status) => ({ label: status, code: status }))} value={row.status} - onSelect={(status) => handleCellChange(row.id, "status", status)} + onSelect={(status) => setRow(row.id, "status", status)} /> ), }), @@ -2214,7 +2214,7 @@ export function EditableRows() { handleCellChange(row.id, "date", date)} + onChange={(date) => setRow(row.id, "date", date)} hideCalendarIcon format="medium" /> @@ -2230,7 +2230,7 @@ export function EditableRows() { handleCellChange(row.id, "date", date)} + onChange={(date) => setRow(row.id, "date", date)} hideCalendarIcon format="medium" /> diff --git a/src/inputs/TextFieldBase.tsx b/src/inputs/TextFieldBase.tsx index c6dc68211..a4386335c 100644 --- a/src/inputs/TextFieldBase.tsx +++ b/src/inputs/TextFieldBase.tsx @@ -14,7 +14,7 @@ import { Icon, IconButton, maybeTooltip } from "src/components"; import { HelperText } from "src/components/HelperText"; import { InlineLabel, Label } from "src/components/Label"; import { usePresentationContext } from "src/components/PresentationContext"; -import { CELL_CSS_SELECTOR, ROW_CSS_SELECTOR } from "src/components/Table/components/Row"; +import { ROW_CSS_SELECTOR } from "src/components/Table/components/Row"; import { Css, Only, Palette } from "src/Css"; import { getLabelSuffix } from "src/forms/labelUtils"; import { useGetRef } from "src/hooks/useGetRef"; @@ -125,7 +125,7 @@ export function TextFieldBase>(props: TextFieldB ? [Palette.Gray700, Palette.Gray600, Palette.Gray700] : borderOnHover ? // Use transparent backgrounds to blend with the table row hover color - [Palette.Transparent, Palette.Transparent, Palette.Gray100] + [Palette.Transparent, Palette.Blue100, Palette.Gray100] : borderless && !compound ? [Palette.Gray100, Palette.Gray200, Palette.Gray200] : [Palette.White, Palette.Gray100, Palette.Gray100]; @@ -154,7 +154,6 @@ export function TextFieldBase>(props: TextFieldB ...{ // Highlight the field when hovering over the row in a table [`.${ROW_CSS_SELECTOR}:hover:not(:has(.textFieldBaseWrapper:hover)) &`]: Css.ba.bcBlue300.$, - [`.${CELL_CSS_SELECTOR}:hover &`]: Css.bgBlue100.$, }, // When multiline is true, then we want to allow the field to grow to the height of the content, but not shrink below the minHeight // Otherwise, set fixed heights values accordingly. From 9be0585aa49924b4f297dccd8a2ab3c989885f69 Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Mon, 2 Dec 2024 17:31:43 -0500 Subject: [PATCH 12/14] fix: PR feedback --- src/components/Table/GridTable.stories.tsx | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/components/Table/GridTable.stories.tsx b/src/components/Table/GridTable.stories.tsx index fdf72ede7..2ca537015 100644 --- a/src/components/Table/GridTable.stories.tsx +++ b/src/components/Table/GridTable.stories.tsx @@ -15,7 +15,6 @@ import { defaultStyle, dragHandleColumn, emptyCell, - getTableStyles, GridCellAlignment, GridColumn, GridDataRow, @@ -2239,18 +2238,9 @@ export function HighlightFields() { w: "120px", }; - const style = getTableStyles({ bordered: true, allWhite: true, highlightOnHover: true }); - return (
- +
); } From a6eec0946e51df6dddb6b5fe42ce3a50df83ad3d Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Tue, 3 Dec 2024 11:17:20 -0500 Subject: [PATCH 13/14] fix: PR feedback --- src/components/PresentationContext.tsx | 1 + src/components/Table/components/Row.tsx | 10 +++++----- src/inputs/TextFieldBase.tsx | 12 ++++++------ 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/components/PresentationContext.tsx b/src/components/PresentationContext.tsx index 6d86e1627..fae8c8265 100644 --- a/src/components/PresentationContext.tsx +++ b/src/components/PresentationContext.tsx @@ -11,6 +11,7 @@ export interface PresentationFieldProps { labelSuffix?: LabelSuffixStyle; // Typically used for compact fields in a table. Removes border and uses an box-shadow for focus behavior borderless?: boolean; + // Typically used for highlighting editable fields in a table. Adds a border on hover. borderOnHover?: boolean; // Defines height of the field compact?: boolean; diff --git a/src/components/Table/components/Row.tsx b/src/components/Table/components/Row.tsx index b7629ca58..af4153155 100644 --- a/src/components/Table/components/Row.tsx +++ b/src/components/Table/components/Row.tsx @@ -145,7 +145,7 @@ function RowImpl(props: RowProps): ReactElement { const onDragOverDebounced = useDebouncedCallback(dragOverCallback, 100); const RowContent = () => ( - + {isKeptGroupRow ? ( ) : ( @@ -220,7 +220,6 @@ function RowImpl(props: RowProps): ReactElement { ? numExpandedColumns + 1 : 1; const revealOnRowHover = isGridCellContent(maybeContent) ? maybeContent.revealOnRowHover : false; - const borderOnHover = style.presentationSettings?.borderOnHover ?? false; const canSortColumn = (sortOn === "client" && column.clientSideSort !== false) || @@ -335,7 +334,7 @@ function RowImpl(props: RowProps): ReactElement { })`, }; - const cellClassNames = [CELL_CSS_SELECTOR, ...(revealOnRowHover ? [revealOnRowHoverClass] : [])].join(" "); + const cellClassNames = revealOnRowHover ? revealOnRowHoverClass : undefined; const cellOnClick = applyCellHighlight ? () => api.setActiveCellId(cellId) : undefined; const tooltip = isGridCellContent(maybeContent) ? maybeContent.tooltip : undefined; @@ -432,5 +431,6 @@ export type GridDataRow = { draggable?: boolean; } & IfAny>; -export const ROW_CSS_SELECTOR = "Row"; -export const CELL_CSS_SELECTOR = "Cell"; +// Used by TextFieldBase to set a border when the row is being hovered over +export const BorderHoverParent = "BorderHoverParent"; +export const BorderHoverChild = "BorderHoverChild"; diff --git a/src/inputs/TextFieldBase.tsx b/src/inputs/TextFieldBase.tsx index a4386335c..0f3daff25 100644 --- a/src/inputs/TextFieldBase.tsx +++ b/src/inputs/TextFieldBase.tsx @@ -14,7 +14,7 @@ import { Icon, IconButton, maybeTooltip } from "src/components"; import { HelperText } from "src/components/HelperText"; import { InlineLabel, Label } from "src/components/Label"; import { usePresentationContext } from "src/components/PresentationContext"; -import { ROW_CSS_SELECTOR } from "src/components/Table/components/Row"; +import { BorderHoverChild, BorderHoverParent } from "src/components/Table/components/Row"; import { Css, Only, Palette } from "src/Css"; import { getLabelSuffix } from "src/forms/labelUtils"; import { useGetRef } from "src/hooks/useGetRef"; @@ -149,11 +149,11 @@ export function TextFieldBase>(props: TextFieldB : Css.bcGray300.if(contrast).bcGray700.$), // Do not add borders to compound fields. A compound field is responsible for drawing its own borders ...(!compound ? Css.ba.$ : {}), - ...(borderOnHover && Css.pl1.ml(-1).br4.ba.bcTransparent.$), + ...(borderOnHover && Css.br4.ba.bcTransparent.$), ...(borderOnHover && Css.if(isHovered).bgBlue100.ba.bcBlue300.$), ...{ - // Highlight the field when hovering over the row in a table - [`.${ROW_CSS_SELECTOR}:hover:not(:has(.textFieldBaseWrapper:hover)) &`]: Css.ba.bcBlue300.$, + // Highlight the field when hovering over the row in a table, unless some other edit component (including ourselves) is hovered + [`.${BorderHoverParent}:hover:not(:has(.${BorderHoverChild}:hover)) &`]: Css.ba.bcBlue300.$, }, // When multiline is true, then we want to allow the field to grow to the height of the content, but not shrink below the minHeight // Otherwise, set fixed heights values accordingly. @@ -243,7 +243,7 @@ export function TextFieldBase>(props: TextFieldB ...(multiline ? Css.fdc.aifs.gap2.$ : Css.if(wrap === false).truncate.$), ...xss, }} - className="textFieldBaseWrapper" + className={BorderHoverChild} data-readonly="true" {...tid} > @@ -275,7 +275,7 @@ export function TextFieldBase>(props: TextFieldB ...Css.if(multiline).aifs.oh.mhPx(textAreaMinHeight).$, }} // Class name used for the grid table on row hover for highlighting - className="textFieldBaseWrapper" + className={BorderHoverChild} {...hoverProps} ref={inputWrapRef as any} onClick={unfocusedPlaceholder ? handleUnfocusedPlaceholderClick : undefined} From 257e758a4b399dc378a9c9dcfe03e7f1c075edaf Mon Sep 17 00:00:00 2001 From: Edwar Plata Date: Tue, 3 Dec 2024 11:43:11 -0500 Subject: [PATCH 14/14] fix: add transition effect to border on hover in TextFieldBase --- src/inputs/TextFieldBase.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/inputs/TextFieldBase.tsx b/src/inputs/TextFieldBase.tsx index 0f3daff25..ec50fc717 100644 --- a/src/inputs/TextFieldBase.tsx +++ b/src/inputs/TextFieldBase.tsx @@ -149,7 +149,7 @@ export function TextFieldBase>(props: TextFieldB : Css.bcGray300.if(contrast).bcGray700.$), // Do not add borders to compound fields. A compound field is responsible for drawing its own borders ...(!compound ? Css.ba.$ : {}), - ...(borderOnHover && Css.br4.ba.bcTransparent.$), + ...(borderOnHover && Css.br4.ba.bcTransparent.add("transition", "border-color 200ms").$), ...(borderOnHover && Css.if(isHovered).bgBlue100.ba.bcBlue300.$), ...{ // Highlight the field when hovering over the row in a table, unless some other edit component (including ourselves) is hovered