sadpandajoe commented on code in PR #45090:
URL: https://github.com/apache/superset/pull/45090#discussion_r4258113541


##########
superset-frontend/src/pages/Canvas/index.tsx:
##########
@@ -101,10 +117,105 @@ const notFound = (
   />
 );
 
+interface CanvasBodyProps {
+  canvasId: number;
+  result: CanvasDefinitionResult;
+  reload: () => void;
+  values: ScopeValues;
+  onValueChange: (
+    kind: canvasApi.ScopeKind,
+    nodeId: string,
+    value: unknown,
+  ) => void;
+  refreshKeys: Record<string, number>;
+  /** Set while a draft is open; layout writes then go to the draft. */
+  draftToken?: string;
+  /** Leaves the draft, returning to the published canvas. */
+  onDraftClosed: () => void;
+  /** True when the canvas holds no widgets yet. */
+  isEmpty: boolean;
+}
+
+/**
+ * The canvas itself, once its definition has loaded. Drag and resize persist
+ * from here; a write that doesn't land says so and leaves the layout where the
+ * server still has it.
+ */
+function CanvasBody({
+  canvasId,
+  result,
+  reload,
+  values,
+  onValueChange,
+  refreshKeys,
+  draftToken,
+  onDraftClosed,
+  isEmpty,
+}: CanvasBodyProps) {
+  const layout = useCanvasLayout(canvasId, result, reload, draftToken);
+  // True while the draft is being published or discarded.
+  const [committing, setCommitting] = useState(false);
+  // Changes are made in a plan; the published canvas is read-only even for
+  // someone who may edit it. Editing also stops once a publish starts: the
+  // commit names a revision and then deletes the draft, so a change accepted
+  // after that read would be replayed by nothing. The single place this
+  // policy is decided.
+  const editable = result.canEdit && Boolean(draftToken) && !committing;
+  return (
+    <>
+      {draftToken && (
+        <DraftBanner
+          token={draftToken}
+          // The revision the layout writes have reached, not the one the
+          // definition was fetched at: a commit must name the draft's current
+          // revision, and dragging moves it on well before the next poll.
+          revision={layout.revision}
+          // Held while a drag or resize is still in flight or queued: a
+          // commit then would publish the older revision and delete the
+          // draft, silently dropping the move the user just made.
+          saving={layout.saving}

Review Comment:
   The wiring that stops a lost edit, `saving={layout.saving}` here and 
`onBusyChange={setCommitting}` on the next line (with `editable` on line 163), 
has no test that exercises it. `DraftBanner.test.tsx` supplies `saving` and 
observes `onBusyChange` by hand, and `Canvas.test.tsx` never sets `canEdit`, a 
draft token or placements. Delete either prop, or the `!committing` term, and 
every test still passes while Publish is again clickable with a PATCH in 
flight. Could a `CanvasBody` test hold a draft PATCH with a second drop queued, 
assert Publish stays disabled until both acknowledgements land and then commits 
the acknowledged revision, and assert the handles disappear while the commit is 
pending?



##########
superset-frontend/src/features/canvas/CanvasGridSurface.tsx:
##########
@@ -0,0 +1,462 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * @fileoverview One grid of a canvas, with drag and resize built in.
+ *
+ * There is no edit mode: a user who may edit gets a drag grip and a resize
+ * corner on each widget, on the canvas as they view it. A user who may not
+ * gets the same grid with no handles. Dragging never covers the widget's own
+ * surface, so a filter or chart underneath stays clickable.
+ *
+ * While a gesture is live the widget follows the pointer and a dashed outline
+ * marks the cells it would take; releasing persists exactly those cells.
+ */
+
+import {
+  PointerEvent as ReactPointerEvent,
+  KeyboardEvent as ReactKeyboardEvent,
+  ReactNode,
+  useCallback,
+  useEffect,
+  useRef,
+  useState,
+} from 'react';
+import { css, styled } from '@apache-superset/core/theme';
+import { t } from '@apache-superset/core/translation';
+import { Icons } from '@superset-ui/core/components';
+import {
+  GridMetrics,
+  gridDelta,
+  movedPlacement,
+  NO_DELTA,
+  overlappedSiblings,
+  resizedPlacement,
+  samePlacement,
+} from './gridGeometry';
+import type { GridPlacement, SpanConstraints } from './types';
+
+type GestureKind = 'move' | 'resize';
+
+interface Gesture {
+  nodeId: string;
+  kind: GestureKind;
+  pointerId: number;
+  startX: number;
+  startY: number;
+  dx: number;
+  dy: number;
+  /**
+   * Where the widget sat when it was grabbed. The gesture resolves against
+   * this, not against the current placement: a write landing mid-gesture (a
+   * sibling's drop pushing this one down, or a draft poll) would otherwise
+   * move the ground under the pointer and persist the drop relative to a row
+   * the user never aimed at.
+   */
+  origin: GridPlacement;
+}
+
+const Grid = styled.div<GridMetrics>`
+  display: grid;
+  grid-template-columns: repeat(${({ columns }) => columns}, minmax(0, 1fr));
+  grid-auto-rows: ${({ rowUnit }) => rowUnit}px;
+  gap: ${({ gap }) => gap}px;
+`;
+
+const GridItem = styled.div<{
+  placement?: GridPlacement;
+  dragging?: boolean;
+  offset?: { x: number; y: number };
+}>`
+  ${({ placement, dragging, offset }) => css`
+    min-width: 0;
+    min-height: 0;
+    position: relative;
+    ${
+      placement &&
+      css`
+        grid-column: ${placement.col} / span ${placement.colSpan};
+        grid-row: ${placement.row} / span ${placement.rowSpan};
+      `
+    }
+    ${
+      dragging &&
+      offset &&
+      css`
+        /* Follow the pointer without reflowing the grid under it. */
+        transform: translate(${offset.x}px, ${offset.y}px);
+        z-index: 2;
+        opacity: 0.85;
+      `
+    }
+  `}
+`;
+
+/**
+ * Where the widget being dragged would land. Turns red once it covers a
+ * sibling, so the user sees an overlap before releasing rather than being
+ * surprised when the server pushes a widget down to resolve it.
+ */
+const DropPreview = styled.div<{
+  placement: GridPlacement;
+  colliding?: boolean;
+}>`
+  ${({ theme, placement, colliding }) => css`
+    grid-column: ${placement.col} / span ${placement.colSpan};
+    grid-row: ${placement.row} / span ${placement.rowSpan};
+    border: 2px dashed ${colliding ? theme.colorError : theme.colorPrimary};
+    border-radius: ${theme.borderRadius}px;
+    background: ${colliding ? theme.colorErrorBg : theme.colorPrimaryBg};
+    pointer-events: none;
+    z-index: 1;
+  `}
+`;
+
+/**
+ * Wraps a widget so its handles can sit on top of it. The handles stay hidden
+ * until the pointer is over the widget or a handle has focus, so a canvas at
+ * rest looks the same whether or not the user may edit it.
+ */
+const Interactive = styled.div`
+  ${({ theme }) => css`
+    height: 100%;
+    position: relative;
+
+    /* Direct children only: hovering a container must not reveal the
+       handles of every widget nested inside it. */
+    & > .canvas-handle {
+      opacity: 0;
+      transition: opacity ${theme.motionDurationMid};
+    }
+
+    &:hover > .canvas-handle,
+    & > .canvas-handle:focus-visible {
+      opacity: 1;
+    }
+  `}
+`;
+
+const Grip = styled.button`
+  ${({ theme }) => css`
+    position: absolute;
+    top: ${theme.sizeUnit}px;
+    right: ${theme.sizeUnit}px;
+    z-index: 3;
+    display: flex;
+    align-items: center;
+    justify-content: center;
+    padding: ${theme.sizeUnit / 2}px;
+    border: 1px solid ${theme.colorBorder};
+    border-radius: ${theme.borderRadiusSM}px;
+    background: ${theme.colorBgElevated};
+    color: ${theme.colorTextSecondary};
+    cursor: grab;
+    touch-action: none;
+
+    &:active {
+      cursor: grabbing;
+    }
+  `}
+`;
+
+const ResizeCorner = styled.button`
+  ${({ theme }) => css`
+    position: absolute;
+    right: 0;
+    bottom: 0;
+    z-index: 3;
+    width: ${theme.sizeUnit * 4}px;
+    height: ${theme.sizeUnit * 4}px;
+    padding: 0;
+    border: none;
+    background: transparent;
+    cursor: nwse-resize;
+    touch-action: none;
+
+    /* A two-line corner, the conventional resize affordance. */
+    &::after {
+      content: '';
+      position: absolute;
+      right: ${theme.sizeUnit / 2}px;
+      bottom: ${theme.sizeUnit / 2}px;
+      width: ${theme.sizeUnit * 2}px;
+      height: ${theme.sizeUnit * 2}px;
+      border-right: 2px solid ${theme.colorTextTertiary};
+      border-bottom: 2px solid ${theme.colorTextTertiary};
+    }
+  `}
+`;
+
+/** The grid's content width, kept current as the viewport changes. */
+function useElementWidth(): [(element: HTMLDivElement | null) => void, number] 
{
+  const [element, setElement] = useState<HTMLDivElement | null>(null);
+  const [width, setWidth] = useState(0);
+
+  useEffect(() => {
+    if (!element) return undefined;
+    const measure = () => setWidth(element.clientWidth);
+    measure();
+    if (typeof ResizeObserver === 'undefined') return undefined;
+    const observer = new ResizeObserver(measure);
+    observer.observe(element);
+    return () => observer.disconnect();
+  }, [element]);
+
+  return [setElement, width];
+}
+
+export interface CanvasGridSurfaceProps {
+  childIds: string[];
+  columns: number;
+  gap: number;
+  rowUnit: number;
+  placements: Record<string, GridPlacement>;
+  constraints: Record<string, SpanConstraints>;
+  /** Whether the user may drag and resize these widgets. */
+  editable: boolean;
+  onPlace: (nodeId: string, placement: GridPlacement) => void;
+  renderNode: (nodeId: string) => ReactNode;
+}
+
+export default function CanvasGridSurface({
+  childIds,
+  columns,
+  gap,
+  rowUnit,
+  placements,
+  constraints,
+  editable,
+  onPlace,
+  renderNode,
+}: CanvasGridSurfaceProps) {
+  const [gridRef, width] = useElementWidth();
+  const [gesture, setGesture] = useState<Gesture>();
+  // The handlers read the gesture directly, so committing never runs as a
+  // side effect inside a state updater.
+  const live = useRef<Gesture>();
+
+  const update = useCallback((next: Gesture | undefined) => {
+    live.current = next;
+    setGesture(next);
+  }, []);
+
+  /** Where `from` would land given a pointer delta, in whole grid units. */
+  const targetOf = useCallback(
+    (
+      nodeId: string,
+      kind: GestureKind,
+      from: GridPlacement,
+      dx: number,
+      dy: number,
+    ) => {
+      const metrics: GridMetrics = { columns, gap, rowUnit };
+      const delta = width > 0 ? gridDelta(dx, dy, width, metrics) : NO_DELTA;
+      return kind === 'move'
+        ? movedPlacement(from, delta, columns)
+        : resizedPlacement(from, delta, columns, constraints[nodeId]);
+    },
+    [width, columns, gap, rowUnit, constraints],
+  );
+
+  const commit = useCallback(
+    (nodeId: string, next: GridPlacement | undefined) => {
+      const placement = placements[nodeId];
+      if (next && placement && !samePlacement(next, placement)) {
+        onPlace(nodeId, next);
+      }
+    },
+    [onPlace, placements],
+  );
+
+  const begin =
+    (nodeId: string, kind: GestureKind) =>
+    (event: ReactPointerEvent<HTMLButtonElement>) => {
+      if (event.button !== 0) return;
+      // One gesture at a time. A second finger on another handle also reports
+      // button 0, and letting it replace the live gesture would strand the
+      // first pointer: its move and up events fail the pointerId guards, so
+      // its drag would be dropped without ever being committed.
+      if (live.current) return;

Review Comment:
   Nothing clears `live.current` unless the handle that owns the gesture 
receives its own up/cancel event, and this guard now turns any leftover gesture 
into a permanent block. Start a touch drag on widget A in a draft, and have a 
draft poll remove A (or flip `editable` off, e.g. Publish starts and then fails 
with a 409). A's handle unmounts mid-gesture, so 
`onPointerUp`/`onPointerCancel` never run. `live.current` stays set and the 
drop preview stays on screen. Every later `pointerdown` on any widget then 
returns here, so drag and resize stay dead until the surface remounts. Should 
the gesture be dropped when its node leaves `placements` or `editable` goes 
false (an effect, or an `onLostPointerCapture` handler)?



##########
superset-frontend/src/features/canvas/CanvasGridSurface.tsx:
##########
@@ -0,0 +1,462 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * @fileoverview One grid of a canvas, with drag and resize built in.
+ *
+ * There is no edit mode: a user who may edit gets a drag grip and a resize
+ * corner on each widget, on the canvas as they view it. A user who may not
+ * gets the same grid with no handles. Dragging never covers the widget's own
+ * surface, so a filter or chart underneath stays clickable.
+ *
+ * While a gesture is live the widget follows the pointer and a dashed outline
+ * marks the cells it would take; releasing persists exactly those cells.
+ */
+
+import {
+  PointerEvent as ReactPointerEvent,
+  KeyboardEvent as ReactKeyboardEvent,
+  ReactNode,
+  useCallback,
+  useEffect,
+  useRef,
+  useState,
+} from 'react';
+import { css, styled } from '@apache-superset/core/theme';
+import { t } from '@apache-superset/core/translation';
+import { Icons } from '@superset-ui/core/components';
+import {
+  GridMetrics,
+  gridDelta,
+  movedPlacement,
+  NO_DELTA,
+  overlappedSiblings,
+  resizedPlacement,
+  samePlacement,
+} from './gridGeometry';
+import type { GridPlacement, SpanConstraints } from './types';
+
+type GestureKind = 'move' | 'resize';
+
+interface Gesture {
+  nodeId: string;
+  kind: GestureKind;
+  pointerId: number;
+  startX: number;
+  startY: number;
+  dx: number;
+  dy: number;
+  /**
+   * Where the widget sat when it was grabbed. The gesture resolves against
+   * this, not against the current placement: a write landing mid-gesture (a
+   * sibling's drop pushing this one down, or a draft poll) would otherwise
+   * move the ground under the pointer and persist the drop relative to a row
+   * the user never aimed at.
+   */
+  origin: GridPlacement;
+}
+
+const Grid = styled.div<GridMetrics>`
+  display: grid;
+  grid-template-columns: repeat(${({ columns }) => columns}, minmax(0, 1fr));
+  grid-auto-rows: ${({ rowUnit }) => rowUnit}px;
+  gap: ${({ gap }) => gap}px;
+`;
+
+const GridItem = styled.div<{
+  placement?: GridPlacement;
+  dragging?: boolean;
+  offset?: { x: number; y: number };
+}>`
+  ${({ placement, dragging, offset }) => css`
+    min-width: 0;
+    min-height: 0;
+    position: relative;
+    ${
+      placement &&
+      css`
+        grid-column: ${placement.col} / span ${placement.colSpan};
+        grid-row: ${placement.row} / span ${placement.rowSpan};
+      `
+    }
+    ${
+      dragging &&
+      offset &&
+      css`
+        /* Follow the pointer without reflowing the grid under it. */
+        transform: translate(${offset.x}px, ${offset.y}px);
+        z-index: 2;
+        opacity: 0.85;
+      `
+    }
+  `}
+`;
+
+/**
+ * Where the widget being dragged would land. Turns red once it covers a
+ * sibling, so the user sees an overlap before releasing rather than being
+ * surprised when the server pushes a widget down to resolve it.
+ */
+const DropPreview = styled.div<{
+  placement: GridPlacement;
+  colliding?: boolean;
+}>`
+  ${({ theme, placement, colliding }) => css`
+    grid-column: ${placement.col} / span ${placement.colSpan};
+    grid-row: ${placement.row} / span ${placement.rowSpan};
+    border: 2px dashed ${colliding ? theme.colorError : theme.colorPrimary};
+    border-radius: ${theme.borderRadius}px;
+    background: ${colliding ? theme.colorErrorBg : theme.colorPrimaryBg};
+    pointer-events: none;
+    z-index: 1;
+  `}
+`;
+
+/**
+ * Wraps a widget so its handles can sit on top of it. The handles stay hidden
+ * until the pointer is over the widget or a handle has focus, so a canvas at
+ * rest looks the same whether or not the user may edit it.
+ */
+const Interactive = styled.div`
+  ${({ theme }) => css`
+    height: 100%;
+    position: relative;
+
+    /* Direct children only: hovering a container must not reveal the
+       handles of every widget nested inside it. */
+    & > .canvas-handle {
+      opacity: 0;
+      transition: opacity ${theme.motionDurationMid};
+    }
+
+    &:hover > .canvas-handle,
+    & > .canvas-handle:focus-visible {
+      opacity: 1;
+    }
+  `}
+`;
+
+const Grip = styled.button`
+  ${({ theme }) => css`
+    position: absolute;
+    top: ${theme.sizeUnit}px;
+    right: ${theme.sizeUnit}px;
+    z-index: 3;
+    display: flex;
+    align-items: center;
+    justify-content: center;
+    padding: ${theme.sizeUnit / 2}px;
+    border: 1px solid ${theme.colorBorder};
+    border-radius: ${theme.borderRadiusSM}px;
+    background: ${theme.colorBgElevated};
+    color: ${theme.colorTextSecondary};
+    cursor: grab;
+    touch-action: none;
+
+    &:active {
+      cursor: grabbing;
+    }
+  `}
+`;
+
+const ResizeCorner = styled.button`
+  ${({ theme }) => css`
+    position: absolute;
+    right: 0;
+    bottom: 0;
+    z-index: 3;
+    width: ${theme.sizeUnit * 4}px;
+    height: ${theme.sizeUnit * 4}px;
+    padding: 0;
+    border: none;
+    background: transparent;
+    cursor: nwse-resize;
+    touch-action: none;
+
+    /* A two-line corner, the conventional resize affordance. */
+    &::after {
+      content: '';
+      position: absolute;
+      right: ${theme.sizeUnit / 2}px;
+      bottom: ${theme.sizeUnit / 2}px;
+      width: ${theme.sizeUnit * 2}px;
+      height: ${theme.sizeUnit * 2}px;
+      border-right: 2px solid ${theme.colorTextTertiary};
+      border-bottom: 2px solid ${theme.colorTextTertiary};
+    }
+  `}
+`;
+
+/** The grid's content width, kept current as the viewport changes. */
+function useElementWidth(): [(element: HTMLDivElement | null) => void, number] 
{
+  const [element, setElement] = useState<HTMLDivElement | null>(null);
+  const [width, setWidth] = useState(0);
+
+  useEffect(() => {
+    if (!element) return undefined;
+    const measure = () => setWidth(element.clientWidth);
+    measure();
+    if (typeof ResizeObserver === 'undefined') return undefined;
+    const observer = new ResizeObserver(measure);
+    observer.observe(element);
+    return () => observer.disconnect();
+  }, [element]);
+
+  return [setElement, width];
+}
+
+export interface CanvasGridSurfaceProps {
+  childIds: string[];
+  columns: number;
+  gap: number;
+  rowUnit: number;
+  placements: Record<string, GridPlacement>;
+  constraints: Record<string, SpanConstraints>;
+  /** Whether the user may drag and resize these widgets. */
+  editable: boolean;
+  onPlace: (nodeId: string, placement: GridPlacement) => void;
+  renderNode: (nodeId: string) => ReactNode;
+}
+
+export default function CanvasGridSurface({
+  childIds,
+  columns,
+  gap,
+  rowUnit,
+  placements,
+  constraints,
+  editable,
+  onPlace,
+  renderNode,
+}: CanvasGridSurfaceProps) {
+  const [gridRef, width] = useElementWidth();
+  const [gesture, setGesture] = useState<Gesture>();
+  // The handlers read the gesture directly, so committing never runs as a
+  // side effect inside a state updater.
+  const live = useRef<Gesture>();
+
+  const update = useCallback((next: Gesture | undefined) => {
+    live.current = next;
+    setGesture(next);
+  }, []);
+
+  /** Where `from` would land given a pointer delta, in whole grid units. */
+  const targetOf = useCallback(
+    (
+      nodeId: string,
+      kind: GestureKind,
+      from: GridPlacement,
+      dx: number,
+      dy: number,
+    ) => {
+      const metrics: GridMetrics = { columns, gap, rowUnit };
+      const delta = width > 0 ? gridDelta(dx, dy, width, metrics) : NO_DELTA;
+      return kind === 'move'
+        ? movedPlacement(from, delta, columns)
+        : resizedPlacement(from, delta, columns, constraints[nodeId]);
+    },
+    [width, columns, gap, rowUnit, constraints],
+  );
+
+  const commit = useCallback(
+    (nodeId: string, next: GridPlacement | undefined) => {
+      const placement = placements[nodeId];
+      if (next && placement && !samePlacement(next, placement)) {
+        onPlace(nodeId, next);
+      }
+    },
+    [onPlace, placements],
+  );
+
+  const begin =
+    (nodeId: string, kind: GestureKind) =>
+    (event: ReactPointerEvent<HTMLButtonElement>) => {
+      if (event.button !== 0) return;
+      // One gesture at a time. A second finger on another handle also reports
+      // button 0, and letting it replace the live gesture would strand the
+      // first pointer: its move and up events fail the pointerId guards, so
+      // its drag would be dropped without ever being committed.
+      if (live.current) return;
+      const origin = placements[nodeId];
+      if (!origin) return;
+      event.preventDefault();
+      event.stopPropagation();
+      event.currentTarget.setPointerCapture(event.pointerId);
+      update({
+        nodeId,
+        kind,
+        pointerId: event.pointerId,
+        startX: event.clientX,
+        startY: event.clientY,
+        dx: 0,
+        dy: 0,
+        origin,
+      });
+    };
+
+  const onPointerMove = (event: ReactPointerEvent<HTMLButtonElement>) => {
+    const { current } = live;
+    if (!current || current.pointerId !== event.pointerId) return;
+    update({
+      ...current,
+      dx: event.clientX - current.startX,
+      dy: event.clientY - current.startY,
+    });
+  };
+
+  const onPointerUp = (event: ReactPointerEvent<HTMLButtonElement>) => {
+    const { current } = live;
+    if (!current || current.pointerId !== event.pointerId) return;
+    update(undefined);
+    // From the release event's own position: the pointer can travel between
+    // the last move event and the release, and the drop belongs where the
+    // user let go.
+    commit(
+      current.nodeId,
+      targetOf(
+        current.nodeId,
+        current.kind,
+        current.origin,
+        event.clientX - current.startX,
+        event.clientY - current.startY,
+      ),
+    );
+  };
+
+  // Only the pointer that owns the gesture may cancel it, so a second
+  // pointer going away elsewhere doesn't discard this one.
+  const onPointerCancel = (event: ReactPointerEvent<HTMLButtonElement>) => {
+    const { current } = live;
+    if (!current || current.pointerId !== event.pointerId) return;
+    update(undefined);
+  };
+
+  /**
+   * Arrow keys do what dragging that handle does, one cell at a time, so the
+   * layout is reachable without a pointer: the grip moves the widget, the
+   * resize corner resizes it.
+   */
+  const onKeyDown =
+    (nodeId: string, kind: GestureKind) =>
+    (event: ReactKeyboardEvent<HTMLButtonElement>) => {
+      const steps: Record<string, [number, number]> = {
+        ArrowLeft: [-1, 0],
+        ArrowRight: [1, 0],
+        ArrowUp: [0, -1],
+        ArrowDown: [0, 1],
+      };
+      const step = steps[event.key];
+      if (!step) return;
+      event.preventDefault();
+      const placement = placements[nodeId];
+      if (!placement) return;
+      const delta = { cols: step[0], rows: step[1] };
+      commit(
+        nodeId,
+        kind === 'move'
+          ? movedPlacement(placement, delta, columns)
+          : resizedPlacement(placement, delta, columns, constraints[nodeId]),
+      );
+    };
+
+  const handlers = {
+    onPointerMove,
+    onPointerUp,
+    onPointerCancel,
+  };
+
+  const preview = gesture
+    ? targetOf(
+        gesture.nodeId,
+        gesture.kind,
+        gesture.origin,
+        gesture.dx,
+        gesture.dy,
+      )
+    : undefined;
+  const covered =
+    preview && gesture
+      ? overlappedSiblings(preview, childIds, placements, gesture.nodeId)
+      : [];
+
+  return (
+    <Grid
+      ref={gridRef}
+      columns={columns}
+      gap={gap}
+      rowUnit={rowUnit}
+      data-test="canvas-grid-surface"
+    >
+      {preview && gesture && !samePlacement(preview, gesture.origin) && (
+        <DropPreview
+          placement={preview}
+          colliding={covered.length > 0}
+          data-test="canvas-drop-preview"
+          data-colliding={covered.length > 0}
+          title={
+            covered.length > 0 ? t('This overlaps another widget') : undefined
+          }
+        />
+      )}
+      {childIds.map(childId => {
+        const dragging = gesture?.nodeId === childId;
+        return (
+          <GridItem
+            key={childId}
+            placement={
+              dragging && gesture ? gesture.origin : placements[childId]
+            }
+            dragging={dragging && gesture?.kind === 'move'}
+            offset={dragging ? { x: gesture.dx, y: gesture.dy } : undefined}
+            data-test="canvas-node"
+            data-node-id={childId}
+          >
+            {editable && placements[childId] ? (

Review Comment:
   Switching between `<Interactive>{renderNode(childId)}…</Interactive>` and 
the bare `renderNode(childId)` changes the element type, so every widget 
remounts whenever `editable` flips. `editable` includes `!committing`, so each 
Publish or Discard click does it, and again when the request fails. 
`FilterSelectRenderer` keeps its selection in local state seeded from 
`defaultSelection`, while `CanvasContent` keeps the filter value. Pick EMEA in 
a `filter.select` with no default, click Publish and get a 409: the filter goes 
back to showing nothing selected while the charts stay filtered to EMEA. Tabs 
lose their active tab the same way. Could the wrapper stay mounted and only the 
handles be conditional?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to