Skip to content

Commit 62c3957

Browse files
fix: comments change hasSharedDragItem to ref dragStateRef, sharedDragPositionRef
1 parent 1f8f65d commit 62c3957

3 files changed

Lines changed: 8 additions & 27 deletions

File tree

src/components/GridLayout/GridLayout.tsx

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import type {DragOverEvent} from 'react-grid-layout';
55
import type {PluginRef, PluginWidgetProps, ReactGridLayoutProps} from 'src/typings';
66

77
import {COMPACT_TYPE_HORIZONTAL_NOWRAP, DEFAULT_GROUP, TEMPORARY_ITEM_ID} from '../../constants';
8-
import {DashKitContext} from '../../context';
98
import type {DashKitCtxShape} from '../../context';
9+
import {DashKitContext} from '../../context';
1010
import type {ConfigItem, ConfigLayout, DraggedOverItem} from '../../shared';
1111
import {resolveLayoutGroup} from '../../utils';
1212
import GridItem from '../GridItem/GridItem';
@@ -205,9 +205,7 @@ export default class GridLayout extends React.PureComponent<GridLayoutProps, Gri
205205
renderLayout: ConfigLayout[],
206206
nextProperties: Partial<ReactGridLayoutProps>,
207207
) => {
208-
// Return a stable properties reference when values are shallowly equal.
209-
// This prevents useMemo inside GroupLayout from invalidating on every render
210-
// when groupGridProperties() returns a structurally identical but new object.
208+
// Return stable ref to prevent useMemo invalidation when properties are shallowly equal.
211209
const prevProperties = this._memoGroupsProps[group];
212210
const keysNext = Object.keys(nextProperties) as Array<keyof ReactGridLayoutProps>;
213211
const stableProperties =
@@ -369,8 +367,7 @@ export default class GridLayout extends React.PureComponent<GridLayoutProps, Gri
369367
cursorPosition: {offsetX, offsetY},
370368
};
371369

372-
// Update imperatively so DragOverLayout can read cursor offset without
373-
// a React prop change (and thus without re-rendering non-source groups).
370+
// Update imperatively so DragOverLayout reads fresh cursor offset without re-render.
374371
this._sharedDragPositionRef.current = {offsetX, offsetY};
375372
}
376373

@@ -804,10 +801,7 @@ export default class GridLayout extends React.PureComponent<GridLayoutProps, Gri
804801
? currentDraggingElement.item.id
805802
: null;
806803

807-
// Scope drag props to the source group only.
808-
// Non-source groups receive stable false/null values so their React.memo
809-
// comparator does not see a change on these three props — only hasSharedDragItem
810-
// can still trigger their re-render (required for cross-group drop readiness).
804+
// Non-source groups get stable false/null — memo skips re-render on drag move.
811805
const isSourceGroup = Boolean(currentDraggingElement?.group === group);
812806
const groupIsAnyDragging = isDragging && isSourceGroup;
813807
const groupCurrentDraggingItemId = isSourceGroup ? currentDraggingItemId : null;

src/components/GridLayout/GroupLayout.tsx

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ export interface GroupLayoutProps {
2121
layout: ConfigLayout[];
2222
temporaryPlaceholder: React.ReactNode;
2323

24-
// Drag state specific to this group (computed in GridLayout.renderGroup)
25-
// hasSharedDragItem and sharedDragPosition replaced by stable refs read
26-
// imperatively by DragOverLayout — no React prop change on drag start.
24+
// Refs read imperatively by DragOverLayout — no re-render on drag start.
2725
isDragCaptured: boolean;
2826
dragStateRef: React.MutableRefObject<{isDragging: boolean; sourceGroup: string | null}>;
2927
sharedDragPositionRef: React.MutableRefObject<{offsetX: number; offsetY: number} | null>;
@@ -75,9 +73,7 @@ function groupLayoutPropsAreEqual(prev: GroupLayoutProps, next: GroupLayoutProps
7573
return false;
7674
}
7775

78-
// Re-render if drag state scoped to this group changed.
79-
// hasSharedDragItem and sharedDragPosition are now refs — same object reference
80-
// forever — so they never trigger a re-render here.
76+
// Re-render only on group-scoped props; drag refs are stable, never trigger this.
8177
if (
8278
prev.isDragCaptured !== next.isDragCaptured ||
8379
prev.isAnyDragging !== next.isAnyDragging ||
@@ -128,12 +124,7 @@ export const GroupLayout = React.memo(function GroupLayout({
128124
// otherwise fall back to the dashboard-level noOverlay from context.
129125
const resolvedNoOverlay = 'noOverlay' in properties ? properties.noOverlay : noOverlay;
130126

131-
// Memoize the items element array so that when GroupLayout re-renders due to
132-
// hasSharedDragItem changing (all non-source groups become drop-ready on drag start),
133-
// the items subtree is not recreated if the drag-relevant props are unchanged.
134-
// For non-source groups isAnyDragging/currentDraggingItemId/isAnyDraggedOut are
135-
// scoped to stable false/null by GridLayout.renderGroup, so the cached array is
136-
// returned and React skips all GridItem subtrees entirely.
127+
// Memoize items array; non-source groups have stable false/null drag props so React skips their subtrees.
137128
const itemElements = React.useMemo(() => {
138129
return renderItems.map((item, i) => {
139130
const keyId = item.id;

src/components/GridLayout/ReactGridLayout.tsx

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -326,11 +326,7 @@ class DragOverLayout extends ReactGridLayout {
326326
return gridItem;
327327
}
328328

329-
// When a transformScaleRef is provided, inject a lazy proxy so that
330-
// react-draggable reads the fresh scale via valueOf() at drag time
331-
// instead of the stale number baked into the last rendered props.
332-
// This avoids a re-render requirement when the canvas scale changes
333-
// (e.g. after d3-zoom auto-fit) before the user starts dragging.
329+
// Lazy proxy for transformScaleRef so react-draggable reads fresh scale without re-render.
334330
const {transformScaleRef} = this.props;
335331
const lazyScale = transformScaleRef
336332
? ({valueOf: () => transformScaleRef.current} as unknown as number)

0 commit comments

Comments
 (0)