fix: drop of bbs inside container causing reflow (#33947)
## Description Current way of dropping building blocks already knows the widget id where it is going to be pasted. `getNewPositionsBasedOnMousePositions` did not take that into account and used to recalculate the canvas and container widgets. - This led to reflow and BBs getting reflowed further down(along with its container) This PR adds a fix to that ensuring no extra calculation and subsequently no further reflow. Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Widget, @tag.Templates, @tag.IDE" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9379360946> > Commit: ba9c447fc662d2c1386b4caadd6dc04355c60082 > Cypress dashboard url: <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9379360946&attempt=1" target="_blank">Click here!</a> <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved error handling when pasting building block widgets. - Enhanced logic for determining container widget when pasting into a specific widget. - **Tests** - Updated test descriptions for better readability. - Added parameter handling in test cases for widget pasting scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
a7dd0921e9
commit
326b6174e9
|
|
@ -284,6 +284,7 @@ export function* loadBuildingBlocksIntoApplication(
|
|||
}
|
||||
}
|
||||
} catch (error) {
|
||||
log.error("Error loading building blocks into application", error);
|
||||
yield put({
|
||||
type: WidgetReduxActionTypes.WIDGET_SINGLE_DELETE,
|
||||
payload: {
|
||||
|
|
@ -420,8 +421,6 @@ export function* pasteBuildingBlockWidgetsSaga(
|
|||
const mainCanvasWidth: number = yield select(getCanvasWidth);
|
||||
|
||||
try {
|
||||
const isThereACollision = false;
|
||||
|
||||
if (
|
||||
// to avoid invoking old way of copied widgets implementaion
|
||||
!Array.isArray(copiedWidgetGroups) ||
|
||||
|
|
@ -458,6 +457,7 @@ export function* pasteBuildingBlockWidgetsSaga(
|
|||
topMostWidget.topRow,
|
||||
leftMostWidget.leftColumn,
|
||||
{ gridPosition },
|
||||
pastingIntoWidgetId,
|
||||
);
|
||||
for (const widgetGroup of copiedWidgetGroups) {
|
||||
//This is required when you cut the widget as CanvasWidgetState doesn't have the widget anymore
|
||||
|
|
@ -507,7 +507,7 @@ export function* pasteBuildingBlockWidgetsSaga(
|
|||
nextAvailableRow,
|
||||
newPastingPositionMap,
|
||||
true,
|
||||
isThereACollision,
|
||||
false,
|
||||
false,
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@ type GeneratorType = Generator<
|
|||
|
||||
describe("pasteBuildingBlockWidgetsSaga", () => {
|
||||
const copiedWidgetsResponse = { widgets: copiedWidgets, flexLayers };
|
||||
it("should handle pasting into a valid parent widget", () => {
|
||||
it("1. should handle pasting into a valid parent widget", () => {
|
||||
const generator: GeneratorType = pasteBuildingBlockWidgetsSaga(
|
||||
gridPosition,
|
||||
parentWidgetId,
|
||||
|
|
@ -101,6 +101,7 @@ describe("pasteBuildingBlockWidgetsSaga", () => {
|
|||
boundaryWidgets.topMostWidget.topRow,
|
||||
boundaryWidgets.leftMostWidget.leftColumn,
|
||||
{ gridPosition },
|
||||
parentWidgetId,
|
||||
),
|
||||
);
|
||||
|
||||
|
|
@ -136,7 +137,7 @@ describe("pasteBuildingBlockWidgetsSaga", () => {
|
|||
expect(result.done).toBe(true);
|
||||
});
|
||||
|
||||
it("should handle errors gracefully", () => {
|
||||
it("2. should handle errors gracefully", () => {
|
||||
const generator: GeneratorType = pasteBuildingBlockWidgetsSaga(
|
||||
{ left: 0, top: 0 },
|
||||
"testParentId",
|
||||
|
|
|
|||
|
|
@ -1,10 +1,13 @@
|
|||
import type { EitherMouseLocationORGridPosition } from "constants/WidgetConstants";
|
||||
import { GridDefaults } from "constants/WidgetConstants";
|
||||
import {
|
||||
GridDefaults,
|
||||
MAIN_CONTAINER_WIDGET_ID,
|
||||
} from "constants/WidgetConstants";
|
||||
import type { CanvasWidgetsReduxState } from "reducers/entityReducers/canvasWidgetsReducer";
|
||||
import { call, select } from "redux-saga/effects";
|
||||
import { getContainerWidgetSpacesSelector } from "selectors/editorSelectors";
|
||||
import type { WidgetProps } from "widgets/BaseWidget";
|
||||
import { getWidgets } from "../selectors";
|
||||
import { getWidget, getWidgets } from "../selectors";
|
||||
|
||||
import type { FlattenedWidgetProps } from "WidgetProvider/constants";
|
||||
import type { WidgetSpace } from "constants/CanvasEditorConstants";
|
||||
|
|
@ -48,6 +51,7 @@ export /**
|
|||
* @param copiedTopMostRow top row of the top most copied widget
|
||||
* @param copiedLeftMostColumn left column of the left most copied widget
|
||||
* @param gridPosition left and top canvas grid position values
|
||||
* @param pastingIntoWidgetId - The ID of the widget where the new widgets should be pasted into.
|
||||
* @returns
|
||||
*/
|
||||
const getNewPositions = function* (
|
||||
|
|
@ -56,6 +60,7 @@ const getNewPositions = function* (
|
|||
copiedTopMostRow: number,
|
||||
copiedLeftMostColumn: number,
|
||||
whereToPasteWidget: EitherMouseLocationORGridPosition,
|
||||
pastingIntoWidgetId?: string,
|
||||
) {
|
||||
const selectedWidgetIDs: string[] = yield select(getSelectedWidgets);
|
||||
const canvasWidgets: CanvasWidgetsReduxState = yield select(getWidgets);
|
||||
|
|
@ -107,6 +112,7 @@ const getNewPositions = function* (
|
|||
copiedTopMostRow,
|
||||
copiedLeftMostColumn,
|
||||
whereToPasteWidget,
|
||||
pastingIntoWidgetId,
|
||||
);
|
||||
return newPastingPositionDetails;
|
||||
};
|
||||
|
|
@ -124,6 +130,7 @@ const getNewPositions = function* (
|
|||
* @param copiedTopMostRow top row of the top most copied widget
|
||||
* @param copiedLeftMostColumn left column of the left most copied widget
|
||||
* @param gridPosition left and top canvas grid position values
|
||||
* @param pastingIntoWidgetId - The ID of the widget where the new widgets should be pasted into. If this is known we don't need to calculate it. In addition, the container widget and dom queries will be different.
|
||||
* @returns
|
||||
*/
|
||||
function* getNewPositionsBasedOnMousePositions(
|
||||
|
|
@ -134,15 +141,33 @@ function* getNewPositionsBasedOnMousePositions(
|
|||
copiedTopMostRow: number,
|
||||
copiedLeftMostColumn: number,
|
||||
whereToPasteWidget: EitherMouseLocationORGridPosition,
|
||||
pastingIntoWidgetId?: string,
|
||||
) {
|
||||
let { canvasDOM, canvasId, containerWidget } =
|
||||
getDefaultCanvas(canvasWidgets);
|
||||
let {
|
||||
canvasDOM,
|
||||
canvasId,
|
||||
containerWidget,
|
||||
}: {
|
||||
canvasDOM: Element | null | undefined;
|
||||
canvasId: string | undefined;
|
||||
containerWidget: FlattenedWidgetProps | undefined;
|
||||
} = getDefaultCanvas(canvasWidgets);
|
||||
|
||||
//if the selected widget is a layout widget then change the pasting canvas.
|
||||
if (selectedWidgets.length === 1 && isDropTarget(selectedWidgets[0].type)) {
|
||||
containerWidget = selectedWidgets[0];
|
||||
({ canvasDOM, canvasId } = getCanvasIdForContainer(containerWidget));
|
||||
}
|
||||
if (pastingIntoWidgetId && pastingIntoWidgetId !== MAIN_CONTAINER_WIDGET_ID) {
|
||||
// For building block we already know the widget where we want to paste the new widgets
|
||||
// no need to calculate it
|
||||
const containerWidgetId = getContainerIdForCanvas(pastingIntoWidgetId);
|
||||
if (!containerWidgetId) return {};
|
||||
containerWidget = yield select(getWidget, containerWidgetId);
|
||||
({ canvasDOM, canvasId } = getCanvasIdForContainer(
|
||||
containerWidget as WidgetProps,
|
||||
));
|
||||
}
|
||||
|
||||
if (!canvasDOM || !containerWidget || !canvasId) return {};
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user