sadpandajoe commented on code in PR #45090:
URL: https://github.com/apache/superset/pull/45090#discussion_r4377079368
##########
superset-frontend/src/features/canvas/CanvasGrid.test.tsx:
##########
@@ -190,3 +223,122 @@ test('setting a cross-filter does nothing while
cross-filters are off', () => {
expect(onValueChange).not.toHaveBeenCalled();
});
+
+test('a canvas that is not editable gets no drag or resize handles', () => {
+ renderGrid();
+
+ expect(
+ screen.queryByRole('button', { name: 'Move widget' }),
+ ).not.toBeInTheDocument();
+ expect(
+ screen.queryByRole('button', { name: 'Resize widget' }),
Review Comment:
This query uses the exact name `'Resize widget'`, but the rendered buttons
are named `'Resize widget from the … corner'`, so it matches nothing and the
assertion passes even if resize corners leak onto a read-only canvas. The same
query appears in `CanvasGridSurface.test.tsx` (line 317) and
`CanvasBody.test.tsx` (line 257). Could these use `/^Resize widget from/` so
the not-in-the-document check can fail?
##########
superset-frontend/src/features/canvas/gridGeometry.ts:
##########
@@ -0,0 +1,174 @@
+/**
+ * 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 Turning pointer movement into grid placement.
+ *
+ * A drag or resize is a pointer delta in pixels; a placement is in whole grid
+ * units. These helpers convert between the two and clamp the result to the
+ * grid and to the widget's declared span limits, so a gesture can only ever
+ * produce a layout the server also accepts.
+ *
+ * Collision resolution is deliberately absent: the server owns push-down (see
+ * `superset/canvas/definition/grid.py`) and returns resolved placements after
+ * the write, so nothing here guesses where other widgets end up.
+ */
+
+import type { GridPlacement, SpanConstraints } from './types';
+
+export interface GridMetrics {
+ columns: number;
+ gap: number;
+ rowUnit: number;
+}
+
+/** A gesture's movement, in whole grid units. */
+export interface GridDelta {
+ cols: number;
+ rows: number;
+}
+
+export const NO_DELTA: GridDelta = { cols: 0, rows: 0 };
+
+/** Which corner of a widget a resize is being dragged by. */
+export type ResizeCorner = 'nw' | 'ne' | 'sw' | 'se';
+
+export const RESIZE_CORNERS: ResizeCorner[] = ['nw', 'ne', 'sw', 'se'];
+
+const clamp = (value: number, min: number, max: number) =>
+ Math.min(Math.max(value, min), max);
+
+/**
+ * Width in pixels of one column on a grid of `width` pixels.
+ *
+ * Mirrors `grid-template-columns: repeat(columns, minmax(0, 1fr))` with
+ * `gap`: the columns share whatever the gaps leave.
+ */
+export function columnWidth(width: number, metrics: GridMetrics): number {
+ const { columns, gap } = metrics;
+ if (columns <= 0) return 0;
+ // A grid narrower than its own gaps would give a negative column, which
+ // would then invert the direction of every gesture.
+ return Math.max(0, (width - gap * (columns - 1)) / columns);
+}
+
+/**
+ * A pointer delta in pixels as whole grid units, rounded to the nearest cell
+ * so a gesture snaps rather than drifting.
+ */
+export function gridDelta(
+ dx: number,
+ dy: number,
+ width: number,
+ metrics: GridMetrics,
+): GridDelta {
+ // An unmeasured grid has no cells to snap to, so nothing has moved yet.
+ if (width <= 0) return NO_DELTA;
+ const colPitch = columnWidth(width, metrics) + metrics.gap;
+ const rowPitch = metrics.rowUnit + metrics.gap;
+ return {
+ cols: colPitch > 0 ? Math.round(dx / colPitch) : 0,
+ rows: rowPitch > 0 ? Math.round(dy / rowPitch) : 0,
+ };
+}
+
+/**
+ * `placement` moved by `delta`, kept inside the grid.
+ *
+ * The spans don't change, so the widget stays whole: it stops at the right
+ * edge rather than being clipped, and can't move above the first row.
+ */
+export function movedPlacement(
+ placement: GridPlacement,
+ delta: GridDelta,
+ columns: number,
+): GridPlacement {
+ const lastCol = Math.max(1, columns - placement.colSpan + 1);
+ return {
+ ...placement,
+ col: clamp(placement.col + delta.cols, 1, lastCol),
+ row: Math.max(1, placement.row + delta.rows),
+ };
+}
+
+/**
+ * `placement` resized by `delta`, kept inside the grid and within the
+ * widget's declared limits.
+ *
+ * `col`/`row` stay put, so resizing grows or shrinks towards the bottom
+ * right. A column span can't reach past the grid's last column, whatever the
+ * widget's own maximum allows.
+ */
+export function resizedPlacement(
+ placement: GridPlacement,
+ delta: GridDelta,
+ columns: number,
+ constraints: SpanConstraints = {},
+ corner: ResizeCorner = 'se',
+): GridPlacement {
+ // The corner being dragged moves; the one opposite it stays put. Dragging a
+ // west or north edge outwards means a negative delta, so the span grows as
+ // the delta shrinks.
+ const west = corner === 'nw' || corner === 'sw';
+ const north = corner === 'nw' || corner === 'ne';
+ const right = placement.col + placement.colSpan - 1;
+ const bottom = placement.row + placement.rowSpan - 1;
+
+ // Room for the span, measured from whichever edge is anchored: to the
+ // grid's last column going east, or back to column 1 going west.
+ const colRoom = west ? right : columns - placement.col + 1;
+ const maxColSpan = Math.min(constraints.maxColSpan ?? columns, colRoom);
+ // The stored placement already satisfies the widget's minimum, so this
+ // range is non-empty; the guard only keeps it that way if limits change.
+ const minColSpan = Math.min(constraints.minColSpan ?? 1, maxColSpan);
+ const colSpan = clamp(
+ placement.colSpan + (west ? -delta.cols : delta.cols),
+ minColSpan,
+ maxColSpan,
+ );
+
+ // Rows are unbounded downwards; going north they stop at row 1.
+ const rowRoom = north ? bottom : Number.MAX_SAFE_INTEGER;
+ const maxRowSpan = Math.min(
+ constraints.maxRowSpan ?? Number.MAX_SAFE_INTEGER,
+ rowRoom,
+ );
+ const minRowSpan = Math.min(constraints.minRowSpan ?? 1, maxRowSpan);
Review Comment:
`minRowSpan` only reflects the widget's declared limit, but the server's
`_fit` also grows a container to its children's content height. Take a group at
row 1, spanning 6 rows, whose child fills all 6, and drag its north-east corner
down 3 rows: this sends `{row: 4, rowSpan: 3}`, the server restores `rowSpan:
6` and keeps `row: 4`, so the group's bottom edge moves from row 6 to 9 and
pushes the widget below it, instead of the shrink that was previewed. Should
`layoutConstraints` carry the content-derived row minimum for containers so the
north-corner origin is computed against it?
--
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]
