Skip to content

Commit

Permalink
fix: Avoid pasting list widget inside another list widget (appsmithor…
Browse files Browse the repository at this point in the history
…g#13719)

* fix for stopping list widget from copying onto itself

* remove duplicate comment

* verify selected widgets are not inside list widget if list widget is to be pasted
  • Loading branch information
rahulramesha committed May 11, 2022
1 parent 89c4bbe commit 27d5ae8
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,27 @@ describe("Widget Copy paste", function() {
.find(widgetsPage.listWidget)
.should("have.length", 0);
});

it("should not be able to paste list widget inside another list widget, when widget inside the list widget are selected", function() {
cy.get(`#div-selection-0`).click({
force: true,
});

// Select widget inside the list widget
cy.get(widgetsPage.listWidget)
.eq(0)
.find(".positioned-widget")
.eq(0)
.click({
ctrlKey: true,
});

//paste
cy.get("body").type(`{${modifierKey}}{v}`);
cy.get(widgetsPage.listWidget).should("have.length", 3);
cy.get(widgetsPage.listWidget)
.eq(0)
.find(widgetsPage.listWidget)
.should("have.length", 0);
});
});
21 changes: 12 additions & 9 deletions app/client/src/sagas/WidgetOperationSagas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
select,
takeEvery,
takeLatest,
takeLeading,
} from "redux-saga/effects";
import { convertToString } from "utils/AppsmithUtils";
import {
Expand Down Expand Up @@ -109,11 +110,10 @@ import {
getPastePositionMapFromMousePointer,
getBoundariesFromSelectedWidgets,
WIDGET_PASTE_PADDING,
getWidgetsFromIds,
getDefaultCanvas,
isDropTarget,
checkIfPastingListWidgetOntoItself,
getValueFromTree,
getVerifiedSelectedWidgets,
} from "./WidgetOperationUtils";
import { getSelectedWidgets } from "selectors/ui";
import { widgetSelectionSagas } from "./WidgetSelectionSagas";
Expand Down Expand Up @@ -861,7 +861,14 @@ const getNewPositions = function*(

const selectedWidgetIDs: string[] = yield select(getSelectedWidgets);
const canvasWidgets: CanvasWidgetsReduxState = yield select(getWidgets);
const selectedWidgets = getWidgetsFromIds(selectedWidgetIDs, canvasWidgets);
const {
isListWidgetPastingOnItself,
selectedWidgets,
} = getVerifiedSelectedWidgets(
selectedWidgetIDs,
copiedWidgetGroups,
canvasWidgets,
);

//if the copied widget is a modal widget, then it has to paste on the main container
if (
Expand All @@ -877,11 +884,7 @@ const getNewPositions = function*(
!(
selectedWidgets.length === 1 &&
isDropTarget(selectedWidgets[0].type, true) &&
!checkIfPastingListWidgetOntoItself(
canvasWidgets,
selectedWidgets[0],
copiedWidgetGroups,
)
!isListWidgetPastingOnItself
) &&
selectedWidgets.length > 0
) {
Expand Down Expand Up @@ -1677,7 +1680,7 @@ export default function* widgetOperationSagas() {
),
takeLatest(ReduxActionTypes.UPDATE_CANVAS_SIZE, updateCanvasSize),
takeLatest(ReduxActionTypes.COPY_SELECTED_WIDGET_INIT, copyWidgetSaga),
takeEvery(ReduxActionTypes.PASTE_COPIED_WIDGET_INIT, pasteWidgetSaga),
takeLeading(ReduxActionTypes.PASTE_COPIED_WIDGET_INIT, pasteWidgetSaga),
takeEvery(ReduxActionTypes.CUT_SELECTED_WIDGET, cutWidgetSaga),
takeEvery(ReduxActionTypes.GROUP_WIDGETS_INIT, groupWidgetsSaga),
]);
Expand Down
49 changes: 4 additions & 45 deletions app/client/src/sagas/WidgetOperationUtils.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { OccupiedSpace } from "constants/CanvasEditorConstants";
import { RenderModes } from "constants/WidgetConstants";
import { get } from "lodash";
import { WidgetProps } from "widgets/BaseWidget";
import { FlattenedWidgetProps } from "widgets/constants";
Expand All @@ -8,7 +7,7 @@ import {
handleSpecificCasesWhilePasting,
doesTriggerPathsContainPropertyPath,
getSelectedWidgetIfPastingIntoListWidget,
checkIfPastingListWidgetOntoItself,
checkForListWidgetInCopiedWidgets,
updateListWidgetPropertiesOnChildDelete,
purgeOrphanedDynamicPaths,
getBoundariesFromSelectedWidgets,
Expand Down Expand Up @@ -981,50 +980,10 @@ describe("WidgetOperationSaga", () => {
},
]);
});
it("should test checkIfPastingListWidgetOntoItself", () => {
const canvasWidgets = {
list2: {
widgetId: "list2",
type: "LIST_WIDGET",
widgetName: "List2",
parentId: "0",
renderMode: RenderModes.CANVAS,
parentColumnSpace: 2,
parentRowSpace: 3,
leftColumn: 2,
rightColumn: 3,
topRow: 1,
bottomRow: 3,
isLoading: false,
listData: [],
version: 16,
disablePropertyPane: false,
template: {},
children: [],
},
};
const selectedWidget = {
widgetId: "list2",
type: "LIST_WIDGET",
widgetName: "List2",
parentId: "0",
renderMode: RenderModes.CANVAS,
parentColumnSpace: 2,
parentRowSpace: 3,
leftColumn: 2,
rightColumn: 3,
topRow: 1,
bottomRow: 3,
isLoading: false,
listData: [],
version: 16,
disablePropertyPane: false,
template: {},
children: [],
};
it("should test checkForListWidgetInCopiedWidgets", () => {
//if copying list widget onto list widget
expect(
checkIfPastingListWidgetOntoItself(canvasWidgets, selectedWidget, [
checkForListWidgetInCopiedWidgets([
{
widgetId: "list2",
parentId: "0",
Expand Down Expand Up @@ -1054,7 +1013,7 @@ describe("WidgetOperationSaga", () => {

//if copying container widget onto list widget
expect(
checkIfPastingListWidgetOntoItself(canvasWidgets, selectedWidget, [
checkForListWidgetInCopiedWidgets([
{
widgetId: "container",
parentId: "0",
Expand Down
73 changes: 59 additions & 14 deletions app/client/src/sagas/WidgetOperationUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,26 +384,71 @@ export const getSelectedWidgetIfPastingIntoListWidget = function(
};

/**
* checks if the selectedWidget is a list widget and copiedWidgetsGroups contains list widget
* get selected widgets that are verified to make sure that we are not pasting list widget onto another list widget
* also return a boolean to indicate if the list widget is pasting into a list widget
*
* @param selectedWidgetIDs
* @param copiedWidgetGroups
* @param canvasWidgets
* @param selectedWidget
* @param copiedWidgetsGroups
* @returns
*/
export const checkIfPastingListWidgetOntoItself = function(
export function getVerifiedSelectedWidgets(
selectedWidgetIDs: string[],
copiedWidgetGroups: CopiedWidgetGroup[],
canvasWidgets: CanvasWidgetsReduxState,
selectedWidget: FlattenedWidgetProps | undefined,
copiedWidgetsGroups: CopiedWidgetGroup[],
) {
return (
getSelectedWidgetIfPastingIntoListWidget(
canvasWidgets,
selectedWidget,
copiedWidgetsGroups,
)?.type === "LIST_WIDGET"
);
};
const selectedWidgets = getWidgetsFromIds(selectedWidgetIDs, canvasWidgets);

//if there is no list widget in the copied widgets then return selected Widgets
if (
!checkForListWidgetInCopiedWidgets(copiedWidgetGroups) ||
selectedWidgets.length === 0
)
return { selectedWidgets };

//if the selected widget is a list widgets the return isListWidgetPastingOnItself as true
if (selectedWidgets.length === 1 && selectedWidgets[0].type === "LIST_WIDGET")
return { selectedWidgets, isListWidgetPastingOnItself: true };

//get list widget ancestor of selected widget if it has a list widget ancestor
const parentListWidgetId = document
.querySelector(
`.${POSITIONED_WIDGET}.${getBaseWidgetClassName(
selectedWidgets[0].widgetId,
)}`,
)
?.closest(".t--widget-listwidget")?.id;

//if the selected widgets do have a list widget ancestor then,
// return that list widget as selected widgets and isListWidgetPastingOnItself as true
if (parentListWidgetId && canvasWidgets[parentListWidgetId])
return {
selectedWidgets: [canvasWidgets[parentListWidgetId]],
isListWidgetPastingOnItself: true,
};

return { selectedWidgets };
}

/**
* returns true if list widget is among the copied widgets
*
* @param copiedWidgetGroups
* @returns boolean
*/
export function checkForListWidgetInCopiedWidgets(
copiedWidgetGroups: CopiedWidgetGroup[],
) {
for (let i = 0; i < copiedWidgetGroups.length; i++) {
const copiedWidget = copiedWidgetGroups[i].list[0];

if (copiedWidget?.type === "LIST_WIDGET") {
return true;
}
}

return false;
}

/**
* get top, left, right, bottom most widgets and totalWidth from copied groups when pasting
Expand Down

0 comments on commit 27d5ae8

Please sign in to comment.