fix: Fixing Anvil DnD for nested containers (#28362)

> Pull Request Template
>
> Use this template to quickly create a well written pull request.
Delete all quotes before creating the pull request.
>
## Description
In this pr, 
- we are making sure layouts inside dragged widgets are not activated
- we are making sure layoutId is newly generated when a widget with a
canvas is added instead of using the defaults layoutId coz its a frozen
object which was using same set of layoutIds for every time we add
widgets.
This fix works only for one level on the canvas widgets, bringing this
to your notice @prsidhu

#### PR fixes following issue(s)
Fixes #28337
> if no issue exists, please create an issue and ask the maintainers
about this first
>
>
#### Media
> A video or a GIF is preferred. when using Loom, don’t embed because it
looks like it’s a GIF. instead, just link to the video
>
>
#### Type of change
> Please delete options that are not relevant.
- Bug fix (non-breaking change which fixes an issue)
- New feature (non-breaking change which adds functionality)
- Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- Chore (housekeeping or task changes that don't impact user perception)
- This change requires a documentation update
>
>
>
## Testing
>
#### How Has This Been Tested?
> Please describe the tests that you ran to verify your changes. Also
list any relevant details for your test configuration.
> Delete anything that is not relevant
- [ ] Manual
- [ ] JUnit
- [ ] Jest
- [ ] Cypress
>
>
#### Test Plan
> Add Testsmith test cases links that relate to this PR
>
>
#### Issues raised during DP testing
> Link issues raised during DP testing for better visiblity and tracking
(copy link from comments dropped on this PR)
>
>
>
## Checklist:
#### Dev activity
- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


#### QA activity:
- [ ] [Speedbreak
features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-)
have been covered
- [ ] Test plan covers all impacted features and [areas of
interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-)
- [ ] Test plan has been peer reviewed by project stakeholders and other
QA members
- [ ] Manually tested functionality on DP
- [ ] We had an implementation alignment call with stakeholders post QA
Round 2
- [ ] Cypress test cases have been added and approved by SDET/manual QA
- [ ] Added `Test Plan Approved` label after Cypress tests were reviewed
- [ ] Added `Test Plan Approved` label after JUnit tests were reviewed
This commit is contained in:
Ashok Kumar M 2023-10-26 11:54:24 +05:30 committed by GitHub
parent d199b49eee
commit b1933e347f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 71 additions and 272 deletions

View File

@ -6,7 +6,6 @@ import type { WidgetProps } from "widgets/BaseWidget";
import { renderLayouts } from "../utils/layouts/renderUtils";
import { getAnvilCanvasId } from "./utils";
import { RenderModes } from "constants/WidgetConstants";
import { getCanvasClassName } from "utils/generators";
export const AnvilCanvas = (props: BaseWidgetProps) => {
const map: LayoutComponentProps["childrenMap"] = {};
@ -14,9 +13,7 @@ export const AnvilCanvas = (props: BaseWidgetProps) => {
map[child.widgetId] = child;
});
const className: string = `anvil-canvas ${getCanvasClassName()} ${props.classList?.join(
" ",
)}`;
const className: string = `anvil-canvas ${props.classList?.join(" ")}`;
return (
<div className={className} id={getAnvilCanvasId(props.widgetId)}>

View File

@ -24,7 +24,7 @@ export interface AnvilDnDStates {
draggedBlocks: DraggedWidget[];
dragDetails: DragDetails;
selectedWidgets: string[];
isChildOfCanvas: boolean;
isChildOfLayout: boolean;
isCurrentDraggedCanvas: boolean;
isDragging: boolean;
isNewWidget: boolean;
@ -94,7 +94,6 @@ const checkIfWidgetTypeDraggedIsAllowedToDrop = (
export const useAnvilDnDStates = ({
allowedWidgetTypes,
canvasId,
layoutId,
}: AnvilDnDStatesProps): AnvilDnDStates => {
const mainCanvasLayoutId: string = useSelector((state) =>
@ -122,9 +121,9 @@ export const useAnvilDnDStates = ({
*/
const isNewWidget = !!newWidget && !dragParent;
/**
* boolean to indicate if the widget being dragged is this particular canvas's child.
* boolean to indicate if the widget being dragged is this particular layout's child.
*/
const isChildOfCanvas = dragParent === canvasId;
const isChildOfLayout = dragParent === layoutId;
/**
* boolean to indicate if the widget is being dragged on this particular canvas.
*/
@ -172,7 +171,7 @@ export const useAnvilDnDStates = ({
draggedBlocks,
dragDetails,
selectedWidgets,
isChildOfCanvas,
isChildOfLayout,
isCurrentDraggedCanvas,
isDragging,
isNewWidget,

View File

@ -1,6 +1,7 @@
import { CANVAS_ART_BOARD } from "constants/componentClassNameConstants";
import { Indices } from "constants/Layers";
import { MAIN_CONTAINER_WIDGET_ID } from "constants/WidgetConstants";
import type { LayoutElementPosition } from "layoutSystems/common/types";
import { positionObserver } from "layoutSystems/common/utils/LayoutElementPositionsObserver";
import { getAnvilLayoutDOMId } from "layoutSystems/common/utils/LayoutElementPositionsObserver/utils";
import { useEffect, useRef } from "react";
@ -11,6 +12,22 @@ export const AnvilCanvasZIndex = {
activated: Indices.Layer10.toString(),
deactivated: "",
};
const checkIfMousePositionIsInsideBlock = (
e: MouseEvent,
mainCanvasRect: DOMRect,
layoutElementPosition: LayoutElementPosition,
) => {
return (
layoutElementPosition.left <= e.clientX - mainCanvasRect.left &&
e.clientX - mainCanvasRect.left <=
layoutElementPosition.left + layoutElementPosition.width &&
layoutElementPosition.top <= e.clientY - mainCanvasRect.top &&
e.clientY - mainCanvasRect.top <=
layoutElementPosition.top + layoutElementPosition.height
);
};
export const useCanvasActivation = (
anvilDragStates: AnvilDnDStates,
layoutId: string,
@ -26,6 +43,9 @@ export const useCanvasActivation = (
const mainContainerDOMNode = document.getElementById(CANVAS_ART_BOARD);
const { setDraggingCanvas, setDraggingNewWidget, setDraggingState } =
useWidgetDragResize();
const draggedWidgetPositions = anvilDragStates.selectedWidgets.map((each) => {
return layoutElementPositions[each];
});
/**
* boolean ref that indicates if the mouse position is outside of main canvas while dragging
* this is being tracked in order to activate/deactivate canvas.
@ -72,6 +92,7 @@ export const useCanvasActivation = (
return currentPositions && !!layoutInfo.isDropTarget;
})
.map((each) => allLayouts[each].layoutId);
/**
* layoutIds sorted by area of each layout in ascending order.
* This is done because a point can be inside multiple canvas areas, but only the smallest of them is the immediate parent.
@ -101,21 +122,23 @@ export const useCanvasActivation = (
smallToLargeSortedDroppableLayoutIds.length > 0
) {
const mainCanvasRect = mainContainerDOMNode.getBoundingClientRect();
const hoveredCanvas = smallToLargeSortedDroppableLayoutIds.find(
(each) => {
const currentCanvasPositions = layoutElementPositions[each];
if (currentCanvasPositions) {
return (
currentCanvasPositions.left <= e.clientX - mainCanvasRect.left &&
e.clientX - mainCanvasRect.left <=
currentCanvasPositions.left + currentCanvasPositions.width &&
currentCanvasPositions.top <= e.clientY - mainCanvasRect.top &&
e.clientY - mainCanvasRect.top <=
currentCanvasPositions.top + currentCanvasPositions.height
);
}
},
);
const isMousePositionOutsideOfDraggingWidgets =
!isNewWidget &&
draggedWidgetPositions.find((each) => {
return checkIfMousePositionIsInsideBlock(e, mainCanvasRect, each);
});
const hoveredCanvas = isMousePositionOutsideOfDraggingWidgets
? dragDetails.dragGroupActualParent
: smallToLargeSortedDroppableLayoutIds.find((each) => {
const currentCanvasPositions = layoutElementPositions[each];
if (currentCanvasPositions) {
return checkIfMousePositionIsInsideBlock(
e,
mainCanvasRect,
currentCanvasPositions,
);
}
});
if (dragDetails.draggedOn !== hoveredCanvas) {
if (hoveredCanvas) {
isMouseOutOfMainCanvas.current = false;

View File

@ -26,15 +26,14 @@ import { getWidgetSizeConfiguration } from "../utils/widgetUtils";
* @returns Enhanced Widget
*/
export const AnvilEditorWidgetOnion = (props: BaseWidgetProps) => {
const { layoutId, parentId } = props;
const { layoutId } = props;
// if layoutId is not present on widget props then we need a selector to fetch layout id of a widget.
// const layoutId = useSelector(getLayoutIdByWidgetId(props.widgetId));
const generateDragState = useCallback(() => {
return generateDragStateForAnvilLayout({
canvasId: parentId || "",
layoutId: layoutId,
layoutId,
});
}, [layoutId, parentId]);
}, [layoutId]);
const widgetSize: SizeConfig = useMemo(
() => getWidgetSizeConfiguration(props.type, props),
[props.type],

View File

@ -15,6 +15,8 @@ import { addWidgetsToPreset } from "../../utils/layouts/update/additionUtils";
import { moveWidgets } from "../../utils/layouts/update/moveUtils";
import { AnvilReduxActionTypes } from "../actions/actionTypes";
import { generateDefaultLayoutPreset } from "layoutSystems/anvil/layoutComponents/presets/DefaultLayoutPreset";
import { selectWidgetInitAction } from "actions/widgetSelectionActions";
import { SelectionRequestType } from "sagas/WidgetSelectUtils";
function* addWidgetsSaga(
actionPayload: ReduxAction<{
@ -82,6 +84,9 @@ function* addWidgetsSaga(
},
};
yield put(updateAndSaveLayout(updatedWidgets));
yield put(
selectWidgetInitAction(SelectionRequestType.One, [newWidget.newWidgetId]),
);
log.debug("Anvil : add new widget took", performance.now() - start, "ms");
} catch (error) {
yield put({

View File

@ -1,151 +0,0 @@
import type { CanvasWidgetsReduxState } from "reducers/entityReducers/canvasWidgetsReducer";
import type { WidgetProps } from "widgets/BaseWidget";
import { getAffectedWidgetsFromLayers, getAllChildWidgets } from "./utils";
const widgets = {
"0": {
children: ["1", "3", "4"],
detachFromLayout: true,
flexLayers: [
{ children: [{ id: "1" }, { id: "3" }] },
{ children: [{ id: "4" }] },
],
},
"1": {
children: ["2"],
},
"2": {
children: ["5", "6", "7", "8"],
detachFromLayout: true,
flexLayers: [
{ children: [{ id: "5" }] },
{ children: [{ id: "6" }, { id: "7" }] },
{ children: [{ id: "8" }] },
],
},
"3": { children: [] },
"4": { children: [] },
"5": { children: [] },
"6": { children: [] },
"7": { children: [] },
"8": { children: [] },
} as unknown as CanvasWidgetsReduxState;
describe("should test getAffectedWidgetsFromLayers", () => {
const layerQueue1 = {
"0": 0,
};
const layerQueue2 = {
"0": 1,
"2": 1,
};
const layerQueue3 = {
"2": 0,
};
const layerQueue4 = {
"0": 1,
"2": 2,
};
const affectedWidgets1 = {
"1": true,
"3": true,
"4": true,
"5": true,
"6": true,
"7": true,
"8": true,
};
const affectedWidgets2 = {
"4": true,
"6": true,
"7": true,
"8": true,
};
const affectedWidgets3 = {
"5": true,
"6": true,
"7": true,
"8": true,
};
const affectedWidgets4 = {
"4": true,
"8": true,
};
it("should return all the affected widgets derived from layer queue", () => {
expect(getAffectedWidgetsFromLayers(layerQueue1, widgets)).toEqual(
affectedWidgets1,
);
expect(getAffectedWidgetsFromLayers(layerQueue2, widgets)).toEqual(
affectedWidgets2,
);
expect(getAffectedWidgetsFromLayers(layerQueue3, widgets)).toEqual(
affectedWidgets3,
);
expect(getAffectedWidgetsFromLayers(layerQueue4, widgets)).toEqual(
affectedWidgets4,
);
});
});
describe("should test getAllChildWidgets", () => {
const widget1 = {
widgetId: "0",
children: ["1", "3", "4"],
} as unknown as WidgetProps;
const widget2 = {
widgetId: "1",
children: ["2"],
} as unknown as WidgetProps;
const widget3 = {
widgetId: "2",
children: ["5", "6", "7", "8"],
} as unknown as WidgetProps;
const widget4 = {
widgetId: "3",
children: [],
} as unknown as WidgetProps;
const childWidgets1 = {
"1": true,
"3": true,
"4": true,
"5": true,
"6": true,
"7": true,
"8": true,
};
const childWidgets2 = {
"5": true,
"6": true,
"7": true,
"8": true,
};
const childWidgets3 = {
"5": true,
"6": true,
"7": true,
"8": true,
};
const childWidgets4 = {};
it("should return all the child widgets except canvas widgets", () => {
expect(getAllChildWidgets(widget1, widgets)).toEqual(childWidgets1);
expect(getAllChildWidgets(widget2, widgets)).toEqual(childWidgets2);
expect(getAllChildWidgets(widget3, widgets)).toEqual(childWidgets3);
expect(getAllChildWidgets(widget4, widgets)).toEqual(childWidgets4);
});
});

View File

@ -1,90 +0,0 @@
import type { CanvasWidgetsReduxState } from "reducers/entityReducers/canvasWidgetsReducer";
import type { WidgetProps } from "widgets/BaseWidget";
/**
* This method is used to determine all the affected widgets from all the layers that have changed
* @param layersProcessQueue Changed layer Ids, will have one layer id per canvas
* @param widgets all widget dsl Properties
* @returns list of all affected widgets
*/
export function getAffectedWidgetsFromLayers(
layersProcessQueue: {
[canvasId: string]: number;
},
widgets: CanvasWidgetsReduxState,
) {
let affectedWidgets: { [widgetDOMId: string]: boolean } = {};
//Even though it has many nested iterations it will go through all teh affected widgets only once
//Iterate through all the canvases and it's first layer that got affected
for (const [canvasId, layerIndex] of Object.entries(layersProcessQueue)) {
const flexLayers = widgets[canvasId]?.flexLayers || [];
//iterate through all the layers below the changed layer id inculuding the layer
for (let i = layerIndex; i < flexLayers.length; i++) {
const children = flexLayers[i]?.children || [];
//iterate through all the child widgets inside the layer
for (const child of children) {
const childWidget = widgets[child.id];
if (!childWidget) continue;
affectedWidgets[child.id] = true;
//if the widget has children get all the nested children
if (childWidget.children && childWidget.children.length > 0) {
affectedWidgets = {
...affectedWidgets,
...getAllChildWidgets(childWidget, widgets, layersProcessQueue),
};
}
}
}
}
return affectedWidgets;
}
/**
* This Method gets all the nested child widgets,
* within the given widgets ignoring the canvas type widgets
* @param widget Widget whose nested children have to be found
* @param widgets all widget dsl Properties
* @returns list of all the nested child widgets of widget
*/
export function getAllChildWidgets(
widget: WidgetProps,
widgets: CanvasWidgetsReduxState,
layersProcessQueue?: {
[canvasId: string]: number;
},
) {
let childWidgets: { [widgetDOMId: string]: boolean } = {};
const children = widget.children;
//iterate through children if widget
for (const childId of children) {
const childWidget = widgets[childId];
if (!childWidget) continue;
//if the child widget is not a canvas add it to the list
if (!childWidget.detachFromLayout) {
childWidgets[childId] = true;
} else if (layersProcessQueue) {
//If it is a canvas widget remove the widget from the layer queue to avoid processing it again
delete layersProcessQueue[childId];
}
//if the widget further has nested children call the getAllChildWidgets recursively.
if (childWidget.children && childWidget.children.length > 0) {
childWidgets = {
...childWidgets,
...getAllChildWidgets(childWidget, widgets, layersProcessQueue),
};
}
}
return childWidgets;
}

View File

@ -36,15 +36,13 @@ export const validateResponsiveProp = (
) => data && Object.keys(data)?.length;
export const generateDragStateForAnvilLayout = ({
canvasId,
layoutId,
}: {
canvasId: string;
layoutId: string;
}): SetDraggingStateActionPayload => {
return {
isDragging: true,
dragGroupActualParent: canvasId || "",
dragGroupActualParent: layoutId || "",
draggedOn: layoutId,
};
};

View File

@ -49,6 +49,9 @@ import {
} from "selectors/editorSelectors";
import { getWidgetMinMaxDimensionsInPixel } from "layoutSystems/autolayout/utils/flexWidgetUtils";
import { isFunction } from "lodash";
import type { LayoutComponentProps } from "layoutSystems/anvil/utils/anvilTypes";
import { getLayoutSystemType } from "selectors/layoutSystemSelectors";
import { LayoutSystemTypes } from "layoutSystems/types";
const WidgetTypes = WidgetFactory.widgetTypes;
@ -73,6 +76,7 @@ function* getChildWidgetProps(
widgets: { [widgetId: string]: FlattenedWidgetProps },
) {
const { leftColumn, newWidgetId, topRow, type } = params;
const layoutSystemType: LayoutSystemTypes = yield select(getLayoutSystemType);
let { columns, parentColumnSpace, parentRowSpace, props, rows, widgetName } =
params;
let minHeight = undefined;
@ -108,6 +112,21 @@ function* getChildWidgetProps(
draft.children = [];
}
});
if (
layoutSystemType === LayoutSystemTypes.ANVIL &&
props.layout &&
props.layout.length
) {
props = {
...props,
layout: props.layout.map((each: LayoutComponentProps) => {
return {
...each,
layoutId: generateReactKey(),
};
}),
};
}
}
}