From 69e8cdd1d138c661ed56bbd5e03e31713e8113a4 Mon Sep 17 00:00:00 2001 From: Don Date: Mon, 26 Feb 2024 09:58:40 -0500 Subject: [PATCH] fix: keep active cell selection in first column from going offscreen (#1823) - change cell input to not go off edge - makes activeCellSelectionBorderWidth a theme variable, so it can be shared with js and css - change canvas active cell outline to only stick to edge if at left edge of screen, not in any column - re-style active cell input to match latest input variables - change outline of input to use selection border color active cell input in the first column fully onscreen, before: ![image](https://github.com/deephaven/web-client-ui/assets/1576283/468083d3-8ee7-4624-94c2-191cc7a541d9) after: ![image](https://github.com/deephaven/web-client-ui/assets/1576283/53d0372a-225d-44b7-8bdc-b08cd9a1cef1) cell selected but not active, and partially scrolled offscreen, before: ![image](https://github.com/deephaven/web-client-ui/assets/1576283/6be27c0e-763d-482d-8cee-1af545fdd15c) after: ![image](https://github.com/deephaven/web-client-ui/assets/1576283/ed556685-fcfa-4069-bc98-12ec9d4a6d92) --- packages/grid/src/Grid.tsx | 13 +++++++-- packages/grid/src/GridRenderer.ts | 28 +++++++++++-------- packages/grid/src/GridTheme.ts | 2 ++ packages/iris-grid/src/IrisGrid.scss | 9 +++--- .../iris-grid/src/IrisGridTheme.module.scss | 2 ++ packages/iris-grid/src/IrisGridTheme.ts | 3 ++ .../__snapshots__/IrisGridTheme.test.ts.snap | 1 + 7 files changed, 40 insertions(+), 18 deletions(-) diff --git a/packages/grid/src/Grid.tsx b/packages/grid/src/Grid.tsx index b29b4332f1..7a44863bc2 100644 --- a/packages/grid/src/Grid.tsx +++ b/packages/grid/src/Grid.tsx @@ -2133,6 +2133,7 @@ class Grid extends PureComponent { const { selectionRange, value, isQuickEdit } = editingCell; const { column, row } = editingCell; const { + scrollX, gridX, gridY, allColumnXs, @@ -2141,19 +2142,27 @@ class Grid extends PureComponent { allRowHeights, } = metrics; + const { activeCellSelectionBorderWidth } = this.getTheme(); + const x = allColumnXs.get(column); const y = allRowYs.get(row); const w = allColumnWidths.get(column); const h = allRowHeights.get(row); + // make sure cell doeesn't go off the left side of the grid + const leftBorderOffset = + gridX + (x ?? 0) <= 0 && scrollX <= 0 + ? activeCellSelectionBorderWidth + : 0; + // If the cell isn't visible, we still need to display an invisible cell for focus purposes const wrapperStyle: CSSProperties = x != null && y != null && w != null && h != null ? { position: 'absolute', - left: gridX + x, + left: gridX + x + leftBorderOffset, top: gridY + y, - width: w, + width: w - leftBorderOffset, height: h, } : { opacity: 0 }; diff --git a/packages/grid/src/GridRenderer.ts b/packages/grid/src/GridRenderer.ts index e98b86ad6b..920e31b707 100644 --- a/packages/grid/src/GridRenderer.ts +++ b/packages/grid/src/GridRenderer.ts @@ -30,9 +30,6 @@ export class GridRenderer { // Default radius in pixels for corners for some elements (like the active cell) static DEFAULT_EDGE_RADIUS = 2; - // Default width in pixels for the border of the active cell - static ACTIVE_CELL_BORDER_WIDTH = 2; - protected textCellRenderer = new TextCellRenderer(); protected dataBarCellRenderer = new DataBarCellRenderer(); @@ -2151,11 +2148,18 @@ export class GridRenderer { context: CanvasRenderingContext2D, state: GridRenderState, column: VisibleIndex, - row: VisibleIndex, - borderWidth = GridRenderer.ACTIVE_CELL_BORDER_WIDTH + row: VisibleIndex ): void { const { metrics, theme } = state; - const { allColumnWidths, allColumnXs, allRowHeights, allRowYs } = metrics; + const { + scrollX, + scrollY, + allColumnWidths, + allColumnXs, + allRowHeights, + allRowYs, + } = metrics; + const { activeCellSelectionBorderWidth: borderWidth } = theme; const cellX = getOrThrow(allColumnXs, column); const cellY = getOrThrow(allRowYs, row); const cellW = getOrThrow(allColumnWidths, column); @@ -2168,13 +2172,13 @@ export class GridRenderer { let h = cellH + borderWidth; // Make sure the outline is interior on the edge - if (x <= 0) { - w += x - 1; - x = 1; + if (x <= 0 && scrollX <= 0) { + w -= borderWidth - x; + x = borderWidth * 0.5; } - if (y <= 0) { - h += y - 1; - y = 1; + if (y <= 0 && scrollY <= 0) { + h -= borderWidth - y; + y = borderWidth * 0.5; } const { lineWidth } = context; diff --git a/packages/grid/src/GridTheme.ts b/packages/grid/src/GridTheme.ts index 09baa8e20d..7c30818b39 100644 --- a/packages/grid/src/GridTheme.ts +++ b/packages/grid/src/GridTheme.ts @@ -84,6 +84,7 @@ export type GridTheme = { scrollBarActiveSelectionTickColor: NullableGridColor; // Look of the current selection + activeCellSelectionBorderWidth: number; selectionColor: GridColor; selectionOutlineColor: GridColor; selectionOutlineCasingColor: GridColor; @@ -187,6 +188,7 @@ const defaultTheme: GridTheme = Object.freeze({ scrollBarCasingWidth: 1, scrollSnapToColumn: false, scrollSnapToRow: false, + activeCellSelectionBorderWidth: 2, selectionColor: '#4286f433', selectionOutlineColor: '#4286f4', selectionOutlineCasingColor: '#222222', diff --git a/packages/iris-grid/src/IrisGrid.scss b/packages/iris-grid/src/IrisGrid.scss index e8eb16f2d5..2e6428ccba 100644 --- a/packages/iris-grid/src/IrisGrid.scss +++ b/packages/iris-grid/src/IrisGrid.scss @@ -14,10 +14,11 @@ $iris-grid-bar-max-height: 250px; $iris-grid-bar-max-width: $table-sidebar-max-width; $transition-iris-grid-bar-flash: 1s; $cell-box-shadow: - 0 0 0 2px var(--dh-color-accent), + 0 0 0 $active-cell-selection-border-width + var(--dh-color-grid-selection-outline), 0 0 0 5px accent-opacity(25); $cell-invalid-box-shadow: - 0 0 0 2px var(--dh-color-negative), + 0 0 0 $active-cell-selection-border-width var(--dh-color-negative), 0 0 0 5px negative-opacity(25); .iris-grid { @@ -134,8 +135,8 @@ $cell-invalid-box-shadow: } .grid-cell-input-field { - color: $gray-200; - background: $gray-800; + color: var(--dh-color-input-fg); + background: var(--dh-color-input-bg); &:focus { box-shadow: $cell-box-shadow; diff --git a/packages/iris-grid/src/IrisGridTheme.module.scss b/packages/iris-grid/src/IrisGridTheme.module.scss index 4f89ffbe99..9de06ca638 100644 --- a/packages/iris-grid/src/IrisGridTheme.module.scss +++ b/packages/iris-grid/src/IrisGridTheme.module.scss @@ -4,6 +4,7 @@ $font-size: 12px; $row-height: 19px; $header-height: 30px; +$active-cell-selection-border-width: 2px; :export { grid-bg: var(--dh-color-grid-bg); @@ -29,6 +30,7 @@ $header-height: 30px; row-height: $row-height; row-background-colors: var(--dh-color-grid-row-0-bg) var(--dh-color-grid-row-1-bg); + active-cell-selection-border-width: $active-cell-selection-border-width; selection-color: var(--dh-color-grid-selection); selection-outline-color: var(--dh-color-grid-selection-outline); selection-outline-casing-color: var(--dh-color-grid-selection-outline-casing); diff --git a/packages/iris-grid/src/IrisGridTheme.ts b/packages/iris-grid/src/IrisGridTheme.ts index 58aa2a3f11..c2852ad8b3 100644 --- a/packages/iris-grid/src/IrisGridTheme.ts +++ b/packages/iris-grid/src/IrisGridTheme.ts @@ -150,6 +150,9 @@ export function createDefaultIrisGridTheme(): IrisGridThemeType { reverseHeaderBarHeight: 4, filterBarHorizontalPadding: 4, + activeCellSelectionBorderWidth: + parseInt(IrisGridTheme['active-cell-selection-border-width'], 10) || 2, + shadowAlpha: parseFloat(IrisGridTheme['row-shadow-alpha']) || 0.15, // Amount of blur to apply to the bottom of the scrim while animating in diff --git a/packages/iris-grid/src/__snapshots__/IrisGridTheme.test.ts.snap b/packages/iris-grid/src/__snapshots__/IrisGridTheme.test.ts.snap index 494a354bfa..ef9a692d52 100644 --- a/packages/iris-grid/src/__snapshots__/IrisGridTheme.test.ts.snap +++ b/packages/iris-grid/src/__snapshots__/IrisGridTheme.test.ts.snap @@ -2,6 +2,7 @@ exports[`createDefaultIrisGridTheme should derive the default Iris grid theme 1`] = ` { + "activeCellSelectionBorderWidth": 2, "allowColumnReorder": true, "allowColumnResize": true, "allowRowReorder": true,