Make highlight ring drawing unified

glideapps · Feb 3, 2024 · 37c6b6c · 37c6b6c
1 parent 7c25279
commit 37c6b6c
Show file tree

Hide file tree

Showing 10 changed files with 42 additions and 51 deletions.
diff --git a/packages/core/src/data-editor/data-editor.tsx b/packages/core/src/data-editor/data-editor.tsx
@@ -800,7 +800,7 @@ const DataEditorImpl: React.ForwardRefRenderFunction<DataEditorRef, DataEditorPr
         onColumnResizeStart: onColumnResizeStartIn,
         customRenderers: additionalRenderers,
         fillHandle,
-        drawFocusRing,
+        drawFocusRing = true,
         experimental,
         fixedShadowX,
         fixedShadowY,
@@ -1183,11 +1183,14 @@ const DataEditorImpl: React.ForwardRefRenderFunction<DataEditorRef, DataEditorPr
             ? gridSelection.current.range
             : undefined;
 
+    const highlightFocus = drawFocusRing ? gridSelection.current?.cell : undefined;
+    const highlightFocusCol = highlightFocus?.[0];
+    const highlightFocusRow = highlightFocus?.[1];
+
     const highlightRegions = React.useMemo(() => {
         if (
             (highlightRegionsIn === undefined || highlightRegionsIn.length === 0) &&
-            highlightRange === undefined &&
-            fillHighlightRegion === undefined
+            (highlightRange ?? highlightFocusCol ?? highlightFocusRow ?? fillHighlightRegion) === undefined
         )
             return undefined;
 
@@ -1226,10 +1229,25 @@ const DataEditorImpl: React.ForwardRefRenderFunction<DataEditorRef, DataEditorPr
             });
         }
 
+        if (highlightFocusCol !== undefined && highlightFocusRow !== undefined) {
+            regions.push({
+                color: mergedTheme.accentColor,
+                range: {
+                    x: highlightFocusCol,
+                    y: highlightFocusRow,
+                    width: 1,
+                    height: 1,
+                },
+                style: "solid-outline",
+            });
+        }
+
         return regions.length > 0 ? regions : undefined;
     }, [
         fillHighlightRegion,
         highlightRange,
+        highlightFocusCol,
+        highlightFocusRow,
         highlightRegionsIn,
         mangledCols.length,
         mergedTheme.accentColor,

diff --git a/packages/core/src/docs/examples/freeze-columns.stories.tsx b/packages/core/src/docs/examples/freeze-columns.stories.tsx
@@ -40,7 +40,7 @@ export const FreezeColumns: React.VFC<any> = (p: { freezeColumns: number }) => {
             freezeColumns={p.freezeColumns}
             getCellContent={getCellContent}
             columns={cols}
-            verticalBorder={c => c > 0}
+            verticalBorder={false}
             rows={1000}
         />
     );

diff --git a/packages/core/src/internal/data-grid/data-grid.stories.tsx b/packages/core/src/internal/data-grid/data-grid.stories.tsx
@@ -98,7 +98,7 @@ export function Simplenotest() {
             translateX={undefined}
             translateY={undefined}
             dragAndDropState={undefined}
-            drawFocusRing={undefined}
+            drawFocusRing={true}
             isFocused={true}
             cellYOffset={y}
             isFilling={false}
@@ -183,7 +183,7 @@ export function SelectedCellnotest() {
             translateX={undefined}
             translateY={undefined}
             dragAndDropState={undefined}
-            drawFocusRing={undefined}
+            drawFocusRing={true}
             onMouseMove={() => undefined}
             accessibilityHeight={50}
             isFilling={false}
@@ -275,7 +275,7 @@ export function SelectedRownotest() {
             translateX={undefined}
             translateY={undefined}
             dragAndDropState={undefined}
-            drawFocusRing={undefined}
+            drawFocusRing={true}
             groupHeaderHeight={34}
             accessibilityHeight={50}
             isFilling={false}
@@ -361,7 +361,7 @@ export const SelectedColumnnotest = () => {
             translateY={undefined}
             dragAndDropState={undefined}
             drawCell={undefined}
-            drawFocusRing={undefined}
+            drawFocusRing={true}
             accessibilityHeight={50}
             isFilling={false}
             groupHeaderHeight={34}

diff --git a/packages/core/src/internal/data-grid/data-grid.tsx b/packages/core/src/internal/data-grid/data-grid.tsx
@@ -221,7 +221,7 @@ export interface DataGridProps {
      * @defaultValue true
      * @group Style
      */
-    readonly drawFocusRing: boolean | undefined;
+    readonly drawFocusRing: boolean;
 
     readonly dragAndDropState:
         | {
@@ -332,7 +332,7 @@ const DataGrid: React.ForwardRefRenderFunction<DataGridRef, DataGridProps> = (p,
         freezeTrailingRows,
         fixedShadowX = true,
         fixedShadowY = true,
-        drawFocusRing = true,
+        drawFocusRing,
         onMouseDown,
         onMouseUp,
         onMouseMoveRaw,

diff --git a/packages/core/src/internal/data-grid/render/data-grid-render.blit.ts b/packages/core/src/internal/data-grid/render/data-grid-render.blit.ts
@@ -70,8 +70,7 @@ export function blitLastFrame(
     }
     deltaX += translateX - last.translateX;
 
-    let stickyWidth = getStickyWidth(effectiveCols);
-    if (stickyWidth > 0) stickyWidth++;
+    const stickyWidth = getStickyWidth(effectiveCols);
 
     if (deltaX !== 0 && deltaY !== 0) {
         return {

diff --git a/packages/core/src/internal/data-grid/render/data-grid-render.ts b/packages/core/src/internal/data-grid/render/data-grid-render.ts
@@ -11,7 +11,7 @@ import { drawCells } from "./data-grid-render.cells.js";
 import { drawGridHeaders } from "./data-grid-render.header.js";
 import { drawGridLines, overdrawStickyBoundaries, drawBlanks, drawExtraRowThemes } from "./data-grid-render.lines.js";
 import { blitLastFrame, blitResizedCol, computeCanBlit } from "./data-grid-render.blit.js";
-import { drawHighlightRings, drawFocusRing, drawColumnResizeOutline } from "./data-grid.render.rings.js";
+import { drawHighlightRings, drawFillHandle, drawColumnResizeOutline } from "./data-grid.render.rings.js";
 
 // Future optimization opportunities
 // - Create a cache of a buffer used to render the full view of a partially displayed column so that when
@@ -341,7 +341,7 @@ export function drawGrid(arg: DrawGridArg, lastArg: DrawGridArg | undefined) {
         }
 
         if (mustDrawFocusOnHeader) {
-            drawFocusRing(
+            drawFillHandle(
                 overlayCtx,
                 width,
                 height,
@@ -447,7 +447,7 @@ export function drawGrid(arg: DrawGridArg, lastArg: DrawGridArg | undefined) {
                 selectionCurrent !== undefined &&
                 damage.has(rectBottomRight(selectionCurrent.range))
             ) {
-                drawFocusRing(
+                drawFillHandle(
                     ctx,
                     width,
                     height,
@@ -586,7 +586,7 @@ export function drawGrid(arg: DrawGridArg, lastArg: DrawGridArg | undefined) {
 
     // the overdraw may have nuked out our focus ring right edge.
     const focusRedraw = drawFocus
-        ? drawFocusRing(
+        ? drawFillHandle(
               targetCtx,
               width,
               height,

diff --git a/packages/core/src/internal/data-grid/render/data-grid-render.walk.ts b/packages/core/src/internal/data-grid/render/data-grid-render.walk.ts
@@ -75,7 +75,7 @@ export function walkColumns(
     const drawY = totalHeaderHeight + translateY;
     for (const c of effectiveCols) {
         const drawX = c.sticky ? clipX : x + translateX;
-        if (cb(c, drawX, drawY, c.sticky ? 0 : clipX + 1, cellYOffset) === true) {
+        if (cb(c, drawX, drawY, c.sticky ? 0 : clipX, cellYOffset) === true) {
             break;
         }
 

diff --git a/packages/core/src/internal/data-grid/render/data-grid.render.rings.ts b/packages/core/src/internal/data-grid/render/data-grid.render.rings.ts
@@ -170,7 +170,7 @@ export function drawColumnResizeOutline(
     ctx.globalAlpha = 1;
 }
 
-export function drawFocusRing(
+export function drawFillHandle(
     ctx: CanvasRenderingContext2D,
     width: number,
     height: number,
@@ -212,7 +212,6 @@ export function drawFocusRing(
 
     const fillHandleRow = fillHandleTarget[1];
 
-    let drawCb: (() => void) | undefined = undefined;
     let drawHandleCb: (() => void) | undefined = undefined;
 
     walkColumns(
@@ -222,7 +221,7 @@ export function drawFocusRing(
         translateY,
         totalHeaderHeight,
         (col, drawX, colDrawY, clipX, startRow) => {
-            clipX -= 1; // we need to be allowed to draw onto this border
+            clipX;
             if (col.sticky && targetCol > col.sourceIndex) return;
 
             const isBeforeTarget = col.sourceIndex < targetColSpan[0];
@@ -250,9 +249,6 @@ export function drawFocusRing(
                     let cellX = drawX;
                     let cellWidth = col.width;
 
-                    const isLastColumn = col.sourceIndex === allColumns.length - 1;
-                    const isLastRow = row === rows - 1;
-
                     if (cell.span !== undefined) {
                         const areas = getSpanBounds(cell.span, drawX, drawY, col.width, rh, col, allColumns);
                         const area = col.sticky ? areas[0] : areas[1];
@@ -264,11 +260,10 @@ export function drawFocusRing(
                     }
 
                     const doHandle = row === fillHandleRow && isFillHandleCol && fillHandle;
-                    const doRing = row === targetRow && !isBeforeTarget && !isAfterTarget && drawCb === undefined;
 
                     if (doHandle) {
                         drawHandleCb = () => {
-                            if (clipX > cellX && !col.sticky && !doRing) {
+                            if (clipX > cellX && !col.sticky) {
                                 ctx.beginPath();
                                 ctx.rect(clipX, 0, width - clipX, height);
                                 ctx.clip();
@@ -279,43 +274,22 @@ export function drawFocusRing(
                             ctx.fill();
                         };
                     }
-
-                    if (doRing) {
-                        drawCb = () => {
-                            if (clipX > cellX && !col.sticky) {
-                                ctx.beginPath();
-                                ctx.rect(clipX, 0, width - clipX, height);
-                                ctx.clip();
-                            }
-                            ctx.beginPath();
-                            ctx.rect(
-                                cellX + 0.5,
-                                drawY + 0.5,
-                                cellWidth - (isLastColumn ? 1 : 0),
-                                rh - (isLastRow ? 1 : 0)
-                            );
-                            ctx.strokeStyle = col.themeOverride?.accentColor ?? theme.accentColor;
-                            ctx.lineWidth = 1;
-                            ctx.stroke();
-                        };
-                    }
-                    return drawCb !== undefined && (fillHandle ? drawHandleCb !== undefined : true);
+                    return drawHandleCb !== undefined;
                 }
             );
 
-            return drawCb !== undefined && (fillHandle ? drawHandleCb !== undefined : true);
+            return drawHandleCb !== undefined;
         }
     );
 
-    if (drawCb === undefined && drawHandleCb === undefined) return undefined;
+    if (drawHandleCb === undefined) return undefined;
 
     const result = () => {
         ctx.save();
         ctx.beginPath();
         ctx.rect(0, totalHeaderHeight, width, height - totalHeaderHeight - stickRowHeight);
         ctx.clip();
 
-        drawCb?.();
         drawHandleCb?.();
 
         ctx.restore();

diff --git a/packages/core/src/internal/scrolling-data-grid/scrolling-data-grid.stories.tsx b/packages/core/src/internal/scrolling-data-grid/scrolling-data-grid.stories.tsx
@@ -120,7 +120,7 @@ export function Simplenotest() {
             imageWindowLoader={new ImageWindowLoaderImpl()}
             onHeaderMenuClick={undefined}
             prelightCells={undefined}
-            drawFocusRing={undefined}
+            drawFocusRing={true}
             initialSize={undefined}
             overscrollX={undefined}
             overscrollY={undefined}

diff --git a/packages/core/test/data-grid.test.tsx b/packages/core/test/data-grid.test.tsx
@@ -46,7 +46,7 @@ const basicProps: DataGridProps = {
     translateX: undefined,
     translateY: undefined,
     dragAndDropState: undefined,
-    drawFocusRing: undefined,
+    drawFocusRing: true,
     drawHeader: undefined,
     drawCell: undefined,
     isFocused: true,