Skip to content

Commit

Permalink
chore: Anvil only: Automatically delete redundant zones (#34879)
Browse files Browse the repository at this point in the history
## Description
Delete redundant zones when drag last widget out or remove it.


https://www.notion.so/appsmith/Removing-redundant-zones-8a2cf907c97246adb618664940e298b0

Fixes #34854 


https://github.com/user-attachments/assets/686dbee3-d16c-4ea1-bbd1-b9e48c73ea8f


https://github.com/user-attachments/assets/c2fe52b8-6a3e-4dcb-993a-03cb65e9672c


## Automation

/ok-to-test tags="@tag.Anvil"

### 🔍 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/9953698717>
> Commit: d2a5392
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9953698717&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Anvil`
> Spec:
> <hr>Tue, 16 Jul 2024 09:12:41 UTC
<!-- 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

- **New Features**
- Improved widget management by adding new functions to handle deletion
of redundant zones and manage widget children.

- **Bug Fixes**
- Enhanced widget dragging and deletion logic to ensure redundant zones
are properly handled.

- **Tests**
- Added tests for new utility functions to validate their behavior in
different scenarios.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Ilia Znamenskii <ilia@appsmith.com>
  • Loading branch information
znamenskii-ilia and znamenskii-ilia authored Jul 16, 2024
1 parent 3ba2f25 commit 48c782a
Show file tree
Hide file tree
Showing 6 changed files with 411 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { CanvasWidgetsReduxState } from "reducers/entityReducers/canvasWidg
import { all, call, put, select, takeLatest } from "redux-saga/effects";
import type {
AnvilHighlightInfo,
DraggedWidget,
WidgetLayoutProps,
} from "layoutSystems/anvil/utils/anvilTypes";
import { getWidget, getWidgets } from "sagas/selectors";
Expand Down Expand Up @@ -40,7 +41,13 @@ import {
getCreateWidgetPayload,
} from "layoutSystems/anvil/utils/widgetAdditionUtils";
import { updateAndSaveAnvilLayout } from "../../../utils/anvilChecksUtils";
import { moveWidgetsToZone } from "layoutSystems/anvil/utils/layouts/update/zoneUtils";
import {
isRedundantZoneWidget,
isZoneWidget,
moveWidgetsToZone,
} from "layoutSystems/anvil/utils/layouts/update/zoneUtils";
import { severTiesFromParents } from "../../../utils/layouts/update/moveUtils";
import { widgetChildren } from "../../../utils/layouts/widgetUtils";

// Function to retrieve highlighting information for the last row in the main canvas layout
export function* getMainCanvasLastRowHighlight() {
Expand Down Expand Up @@ -316,7 +323,7 @@ export function* moveWidgetsSaga(
const isSection = draggedOn === "SECTION";
const movedWidgetIds = movedWidgets.map((each) => each.widgetId);

const updatedWidgets: CanvasWidgetsReduxState = yield call<
let updatedWidgets: CanvasWidgetsReduxState = yield call<
typeof handleWidgetMovement
>(
handleWidgetMovement,
Expand All @@ -327,6 +334,8 @@ export function* moveWidgetsSaga(
isSection,
);

updatedWidgets = handleDeleteRedundantZones(updatedWidgets, movedWidgets);

yield call(updateAndSaveAnvilLayout, updatedWidgets);
log.debug("Anvil : moving widgets took", performance.now() - start, "ms");
} catch (error) {
Expand Down Expand Up @@ -380,6 +389,36 @@ export function* handleWidgetMovement(
return updatedWidgets;
}

export function handleDeleteRedundantZones(
allWidgets: CanvasWidgetsReduxState,
movedWidgets: DraggedWidget[],
) {
let updatedWidgets: CanvasWidgetsReduxState = { ...allWidgets };
const parentIds = movedWidgets
.map((widget) => widget.parentId)
.filter(Boolean) as string[];

for (const parentId of parentIds) {
const zone = updatedWidgets[parentId];

if (!isZoneWidget(zone) || !zone.parentId) continue;

const parentSection = updatedWidgets[zone.parentId];

if (!parentSection || !isRedundantZoneWidget(zone, parentSection)) continue;

updatedWidgets = severTiesFromParents(updatedWidgets, [zone.widgetId]);
delete updatedWidgets[zone.widgetId];

if (widgetChildren(parentSection).length === 1) {
updatedWidgets = severTiesFromParents(updatedWidgets, [zone.parentId]);
delete updatedWidgets[zone.parentId];
}
}

return updatedWidgets;
}

export default function* anvilDraggingSagas() {
yield all([
takeLatest(AnvilReduxActionTypes.ANVIL_ADD_NEW_WIDGET, addWidgetsSaga),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
import { RenderModes } from "../../../../../constants/WidgetConstants";
import { mockButtonProps } from "../../../../../mocks/widgetProps/button";
import { isRedundantZoneWidget, isZoneWidget } from "./zoneUtils";

describe("isZoneWidget", () => {
it("should return true if the widget is a zone widget", () => {
// TODO: Use factory to generate widget
const widget = {
type: "ZONE_WIDGET",
widgetId: "123",
widgetName: "Button1",
renderMode: RenderModes.CANVAS,
version: 1,
isLoading: false,
parentColumnSpace: 10,
parentRowSpace: 10,
leftColumn: 0,
rightColumn: 10,
topRow: 0,
bottomRow: 4,
};

expect(isZoneWidget(widget)).toBe(true);
});

it("should return false if the widget is not a zone widget", () => {
const widget = mockButtonProps();

expect(isZoneWidget(widget)).toBe(false);
});
});

describe("isRedundantZoneWidget", () => {
it("should return true if the widget is a redundant zone widget", () => {
const widget = {
type: "ZONE_WIDGET",
widgetId: "123",
widgetName: "Zone1",
renderMode: RenderModes.CANVAS,
children: [],
dynamicPropertyPathList: [],
version: 1,
isLoading: false,
parentColumnSpace: 10,
parentRowSpace: 10,
leftColumn: 0,
rightColumn: 10,
topRow: 0,
bottomRow: 4,
};

const parentWidget = {
type: "SECTION_WIDGET",
widgetId: "567",
widgetName: "Section1",
renderMode: RenderModes.CANVAS,
children: ["123"],
version: 1,
isLoading: false,
parentColumnSpace: 10,
parentRowSpace: 10,
leftColumn: 0,
rightColumn: 10,
topRow: 0,
bottomRow: 4,
};

expect(isRedundantZoneWidget(widget, parentWidget)).toBe(true);
});

it("should return false if the widget is not empty", () => {
const widget = {
type: "ZONE_WIDGET",
widgetId: "123",
widgetName: "Zone1",
renderMode: RenderModes.CANVAS,
children: ["000"],
dynamicPropertyPathList: [],
version: 1,
isLoading: false,
parentColumnSpace: 10,
parentRowSpace: 10,
leftColumn: 0,
rightColumn: 10,
topRow: 0,
bottomRow: 4,
};

const parentWidget = {
type: "SECTION_WIDGET",
widgetId: "567",
widgetName: "Section1",
renderMode: RenderModes.CANVAS,
children: ["123"],
version: 1,
isLoading: false,
parentColumnSpace: 10,
parentRowSpace: 10,
leftColumn: 0,
rightColumn: 10,
topRow: 0,
bottomRow: 4,
};

expect(isRedundantZoneWidget(widget, parentWidget)).toBe(false);
});

it("should return false if the widget is not zone", () => {
const widget = {
type: "SECTION_WIDGET",
widgetId: "567",
widgetName: "Section1",
renderMode: RenderModes.CANVAS,
children: ["123"],
version: 1,
isLoading: false,
parentColumnSpace: 10,
parentRowSpace: 10,
leftColumn: 0,
rightColumn: 10,
topRow: 0,
bottomRow: 4,
};

const parentWidget = {
type: "SECTION_WIDGET",
widgetId: "567",
widgetName: "Section1",
renderMode: RenderModes.CANVAS,
children: ["123"],
version: 1,
isLoading: false,
parentColumnSpace: 10,
parentRowSpace: 10,
leftColumn: 0,
rightColumn: 10,
topRow: 0,
bottomRow: 4,
};

expect(isRedundantZoneWidget(widget, parentWidget)).toBe(false);
});

it("should return false if the widget is not the only child", () => {
const widget = {
type: "ZONE_WIDGET",
widgetId: "567",
widgetName: "Section1",
renderMode: RenderModes.CANVAS,
children: ["123"],
version: 1,
isLoading: false,
parentColumnSpace: 10,
parentRowSpace: 10,
leftColumn: 0,
rightColumn: 10,
topRow: 0,
bottomRow: 4,
};

const parentWidget = {
type: "SECTION_WIDGET",
widgetId: "567",
widgetName: "Section1",
renderMode: RenderModes.CANVAS,
children: ["123", "877"],
version: 1,
isLoading: false,
parentColumnSpace: 10,
parentRowSpace: 10,
leftColumn: 0,
rightColumn: 10,
topRow: 0,
bottomRow: 4,
};

expect(isRedundantZoneWidget(widget, parentWidget)).toBe(false);
});

it("should return false if the widget has JS props enabled", () => {
const widget = {
type: "ZONE_WIDGET",
widgetId: "567",
widgetName: "Section1",
renderMode: RenderModes.CANVAS,
children: ["123"],
dynamicPropertyPathList: [{ key: "isVisible" }],
version: 1,
isLoading: false,
parentColumnSpace: 10,
parentRowSpace: 10,
leftColumn: 0,
rightColumn: 10,
topRow: 0,
bottomRow: 4,
};

const parentWidget = {
type: "SECTION_WIDGET",
widgetId: "567",
widgetName: "Section1",
renderMode: RenderModes.CANVAS,
children: ["123"],
version: 1,
isLoading: false,
parentColumnSpace: 10,
parentRowSpace: 10,
leftColumn: 0,
rightColumn: 10,
topRow: 0,
bottomRow: 4,
};

expect(isRedundantZoneWidget(widget, parentWidget)).toBe(false);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ import {
transformMovedWidgets,
} from "./moveUtils";
import type { WidgetProps } from "widgets/BaseWidget";
import {
hasWidgetJsPropertiesEnabled,
isEmptyWidget,
widgetChildren,
} from "../widgetUtils";

export function* createZoneAndAddWidgets(
allWidgets: CanvasWidgetsReduxState,
Expand Down Expand Up @@ -242,3 +247,16 @@ export function* moveWidgetsToZone(
return updatedWidgets;
}
}

export const isZoneWidget = (widget: FlattenedWidgetProps): boolean =>
widget.type === anvilWidgets.ZONE_WIDGET;

export const isRedundantZoneWidget = (
widget: FlattenedWidgetProps,
parentSection: FlattenedWidgetProps,
): boolean =>
isZoneWidget(widget) &&
isEmptyWidget(widget) &&
// Check that the zone is the only child of the parent section.
widgetChildren(parentSection).length === 1 &&
!hasWidgetJsPropertiesEnabled(widget);
Loading

0 comments on commit 48c782a

Please sign in to comment.