Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: render only the currently open detached widgets. #33040

Merged
merged 2 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/client/src/ce/reducers/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ import type { ActiveField } from "reducers/uiReducers/activeFieldEditorReducer";
import type { SelectedWorkspaceReduxState } from "@appsmith/reducers/uiReducers/selectedWorkspaceReducer";
import type { ConsolidatedPageLoadState } from "reducers/uiReducers/consolidatedPageLoadReducer";
import type { BuildingBlocksReduxState } from "reducers/uiReducers/buildingBlockReducer";
import type { AnvilDetachedWidgetsReduxState } from "layoutSystems/anvil/integrations/reducers/anvilDetachedWidgetsReducer";

export const reducerObject = {
entities: entityReducer,
Expand Down Expand Up @@ -146,6 +147,7 @@ export interface AppState {
oneClickBinding: OneClickBindingState;
activeField: ActiveField;
ide: IDEState;
anvilDetachedWidgets: AnvilDetachedWidgetsReduxState;
};
entities: {
canvasWidgetsStructure: CanvasWidgetStructure;
Expand Down
2 changes: 2 additions & 0 deletions app/client/src/ce/reducers/uiReducers/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import activeFieldReducer from "reducers/uiReducers/activeFieldEditorReducer";
import selectedWorkspaceReducer from "@appsmith/reducers/uiReducers/selectedWorkspaceReducer";
import ideReducer from "../../../reducers/uiReducers/ideReducer";
import consolidatedPageLoadReducer from "reducers/uiReducers/consolidatedPageLoadReducer";
import anvilDetachedWidgetsReducer from "layoutSystems/anvil/integrations/reducers/anvilDetachedWidgetsReducer";

export const uiReducerObject = {
analytics: analyticsReducer,
Expand Down Expand Up @@ -104,4 +105,5 @@ export const uiReducerObject = {
activeField: activeFieldReducer,
ide: ideReducer,
consolidatedPageLoad: consolidatedPageLoadReducer,
anvilDetachedWidgets: anvilDetachedWidgetsReducer,
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,12 @@ import { useDispatch, useSelector } from "react-redux";
import { useWidgetSelection } from "utils/hooks/useWidgetSelection";
import { combinedPreviewModeSelector } from "selectors/editorSelectors";
import { SELECT_ANVIL_WIDGET_CUSTOM_EVENT } from "layoutSystems/anvil/utils/constants";
import { renderChildWidget } from "layoutSystems/common/utils/canvasUtils";
import type { WidgetProps } from "widgets/BaseWidget";
import { getRenderMode } from "selectors/editorSelectors";
import { denormalize } from "utils/canvasStructureHelpers";
import type { CanvasWidgetStructure } from "WidgetProvider/constants";
import { getWidgets } from "sagas/selectors";
import log from "loglevel";
import { useEffect, useMemo } from "react";
import { getAnvilWidgetDOMId } from "layoutSystems/common/utils/LayoutElementPositionsObserver/utils";
import {
MAIN_CONTAINER_WIDGET_ID,
type RenderModes,
} from "constants/WidgetConstants";
import { getCurrentlyOpenAnvilDetachedWidgets } from "layoutSystems/anvil/integrations/modalSelectors";
import { getCanvasWidgetsStructure } from "@appsmith/selectors/entitiesSelector";
import type { CanvasWidgetStructure } from "WidgetProvider/constants";
/**
* This hook is used to select and focus on a detached widget
* As detached widgets are outside of the layout flow, we need to access the correct element in the DOM
Expand Down Expand Up @@ -114,48 +107,25 @@ export function useAddBordersToDetachedWidgets(widgetId: string) {
* @param children
* @returns
*/
function useDetachedChildren(children: CanvasWidgetStructure[]) {
export function useDetachedChildren() {
const start = performance.now();
// Get all widgets
const widgets = useSelector(getWidgets);
const widgets = useSelector(getCanvasWidgetsStructure);
const currentlyOpenWidgets = useSelector(
getCurrentlyOpenAnvilDetachedWidgets,
);
// Filter out the detached children and denormalise each of the detached widgets to generate
// a DSL like hierarchy
const detachedChildren = useMemo(() => {
return children
.map((child) => widgets[child.widgetId])
.filter((child) => child.detachFromLayout === true)
.map((child) => {
return denormalize(child.widgetId, widgets);
});
}, [children, widgets]);
const allChildren = currentlyOpenWidgets.map((widgetId) => {
return (
widgets.children &&
widgets.children.find((each) => each.widgetId === widgetId)
);
});
return allChildren.filter((child) => !!child) as CanvasWidgetStructure[];
}, [currentlyOpenWidgets, widgets]);
const end = performance.now();
log.debug("### Computing detached children took:", end - start, "ms");
return detachedChildren;
}

export function useRenderDetachedChildren(
widgetId: string,
children: CanvasWidgetStructure[],
) {
const renderMode: RenderModes = useSelector(getRenderMode);
// Get the detached children to render on the canvas
const detachedChildren = useDetachedChildren(children);
let renderDetachedChildren = null;
if (widgetId === MAIN_CONTAINER_WIDGET_ID) {
renderDetachedChildren = detachedChildren.map((child) =>
renderChildWidget({
childWidgetData: child as WidgetProps,
defaultWidgetProps: {
className: `${getAnvilWidgetDOMId(child.widgetId)}`,
},
noPad: false,
// Adding these properties as the type insists on providing this
// while it is not required for detached children
layoutSystemProps: { parentColumnSpace: 1, parentRowSpace: 1 },
renderMode,
widgetId: MAIN_CONTAINER_WIDGET_ID,
}),
);
}
return renderDetachedChildren;
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,7 @@ export enum AnvilReduxActionTypes {
ANVIL_SPACE_DISTRIBUTION_STOP = "ANVIL_SPACE_DISTRIBUTION_STOP",
ANVIL_SET_HIGHLIGHT_SHOWN = "ANVIL_SET_HIGHLIGHT_SHOWN",
ANVIL_WIDGET_SELECTION_CLICK = "ANVIL_WIDGET_SELECTION_CLICK",
SHOW_DETACHED_WIDGET = "SHOW_DETACHED_WIDGET",
HIDE_DETACHED_WIDGET = "HIDE_DETACHED_WIDGET",
RESET_DETACHED_WIDGETS = "RESET_DETACHED_WIDGETS",
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { AnvilReduxActionTypes } from "./actionTypes";

export const showDetachedWidgetAction = (widgetId: string) => {
return {
type: AnvilReduxActionTypes.SHOW_DETACHED_WIDGET,
payload: widgetId,
};
};

export const hideDetachedWidgetAction = (widgetId: string) => {
return {
type: AnvilReduxActionTypes.HIDE_DETACHED_WIDGET,
payload: widgetId,
};
};

export const resetDetachedWidgetsAction = () => {
return {
type: AnvilReduxActionTypes.RESET_DETACHED_WIDGETS,
};
};
15 changes: 2 additions & 13 deletions app/client/src/layoutSystems/anvil/integrations/modalSelectors.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,5 @@
import type { AppState } from "@appsmith/reducers";
import { getWidgetIdsByType, getWidgetsMeta } from "sagas/selectors";
import { WDSModalWidget } from "widgets/wds/WDSModalWidget";

export const getCurrentlyOpenAnvilModal = (state: AppState) => {
const allExistingModals = getWidgetIdsByType(state, WDSModalWidget.type);
if (allExistingModals.length === 0) {
return;
}
const metaWidgets = getWidgetsMeta(state);
const currentlyOpenModal = allExistingModals.find((modalId) => {
const modal = metaWidgets[modalId];
return modal && modal.isVisible;
});
return currentlyOpenModal;
export const getCurrentlyOpenAnvilDetachedWidgets = (state: AppState) => {
return state.ui.anvilDetachedWidgets.currentlyOpenDetachedWidgets;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { createImmerReducer } from "utils/ReducerUtils";
import {
AnvilReduxActionTypes,
type AnvilReduxAction,
} from "../actions/actionTypes";

export interface AnvilDetachedWidgetsReduxState {
currentlyOpenDetachedWidgets: string[];
}
const initialState: AnvilDetachedWidgetsReduxState = {
currentlyOpenDetachedWidgets: [],
};
const anvilDetachedWidgetsReducer = createImmerReducer(initialState, {
[AnvilReduxActionTypes.SHOW_DETACHED_WIDGET]: (
state: AnvilDetachedWidgetsReduxState,
action: AnvilReduxAction<string>,
) => {
state.currentlyOpenDetachedWidgets.push(action.payload);
},
[AnvilReduxActionTypes.HIDE_DETACHED_WIDGET]: (
state: AnvilDetachedWidgetsReduxState,
action: AnvilReduxAction<string>,
) => {
state.currentlyOpenDetachedWidgets =
state.currentlyOpenDetachedWidgets.filter(
(widgetId) => widgetId !== action.payload,
);
},
[AnvilReduxActionTypes.RESET_DETACHED_WIDGETS]: (
state: AnvilDetachedWidgetsReduxState,
) => {
state.currentlyOpenDetachedWidgets = [];
},
});

export default anvilDetachedWidgetsReducer;
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import type { ReduxAction } from "@appsmith/constants/ReduxActionConstants";
import { ReduxActionTypes } from "@appsmith/constants/ReduxActionConstants";
import { all, put, select, takeEvery, takeLatest } from "redux-saga/effects";
import { callSagaOnlyForAnvil } from "./utils";
import {
hideDetachedWidgetAction,
resetDetachedWidgetsAction,
showDetachedWidgetAction,
} from "../actions/detachedWidgetActions";
import { getWidgetByName } from "sagas/selectors";
import type { FlattenedWidgetProps } from "WidgetProvider/constants";

function* closeAnvilModalSaga(action: ReduxAction<{ modalName?: string }>) {
const { modalName } = action.payload;

if (modalName) {
const widget: FlattenedWidgetProps | undefined = yield select(
getWidgetByName,
modalName,
);
if (widget) {
hideDetachedWidgetAction(widget.widgetId);
}
} else {
yield put(resetDetachedWidgetsAction());
}
}

function* showAnvilModalSaga(action: ReduxAction<{ modalId: string }>) {
yield put(showDetachedWidgetAction(action.payload.modalId));
}

export default function* anvilDetachedWidgetSagas() {
yield all([
takeEvery(
ReduxActionTypes.CLOSE_MODAL,
callSagaOnlyForAnvil,
closeAnvilModalSaga,
),
takeLatest(
ReduxActionTypes.SHOW_MODAL,
callSagaOnlyForAnvil,
showAnvilModalSaga,
),
]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import anvilSectionSagas from "./sectionSagas";
import anvilSpaceDistributionSagas from "./anvilSpaceDistributionSagas";
import anvilWidgetSelectionSaga from "./anvilWidgetSelectionSaga";
import pasteSagas from "./pasteSagas";
import anvilDetachedWidgetSagas from "./anvilDetachedWidgetSagas";

export default function* anvilSagas() {
yield fork(LayoutElementPositionsSaga);
Expand All @@ -13,4 +14,5 @@ export default function* anvilSagas() {
yield fork(anvilSpaceDistributionSagas);
yield fork(anvilWidgetSelectionSaga);
yield fork(pasteSagas);
yield fork(anvilDetachedWidgetSagas);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { updateAndSaveAnvilLayout } from "../../utils/anvilChecksUtils";
import { builderURL } from "@appsmith/RouteBuilder";
import { getCurrentPageId } from "selectors/editorSelectors";
import {
type ReduxAction,
ReduxActionErrorTypes,
ReduxActionTypes,
} from "@appsmith/constants/ReduxActionConstants";
Expand All @@ -24,7 +23,7 @@ import { getDestinedParent } from "layoutSystems/anvil/utils/paste/destinationUt
import { pasteWidgetsIntoMainCanvas } from "layoutSystems/anvil/utils/paste/mainCanvasPasteUtils";
import { MAIN_CONTAINER_WIDGET_ID } from "constants/WidgetConstants";
import WidgetFactory from "WidgetProvider/factory";
import { getIsAnvilLayout } from "../selectors";
import { callSagaOnlyForAnvil } from "./utils";

function* pasteWidgetSagas() {
try {
Expand Down Expand Up @@ -122,18 +121,11 @@ function* pasteWidgetSagas() {
}
}

function* shouldCallSaga(saga: any, action: ReduxAction<unknown>) {
const isAnvilLayout: boolean = yield select(getIsAnvilLayout);
if (isAnvilLayout) {
yield call(saga, action);
}
}

export default function* pasteSagas() {
yield all([
takeLeading(
ReduxActionTypes.PASTE_COPIED_WIDGET_INIT,
shouldCallSaga,
callSagaOnlyForAnvil,
pasteWidgetSagas,
),
]);
Expand Down
10 changes: 10 additions & 0 deletions app/client/src/layoutSystems/anvil/integrations/sagas/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import type { ReduxAction } from "@appsmith/constants/ReduxActionConstants";
import { call, select } from "redux-saga/effects";
import { getIsAnvilLayout } from "../selectors";

export function* callSagaOnlyForAnvil(saga: any, action: ReduxAction<unknown>) {
const isAnvilLayout: boolean = yield select(getIsAnvilLayout);
if (isAnvilLayout) {
yield call(saga, action);
}
}
Comment on lines +5 to +10
Copy link
Contributor

@coderabbitai coderabbitai bot Apr 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a type definition for the saga parameter to enhance type safety and maintainability.

- export function* callSagaOnlyForAnvil(saga: any, action: ReduxAction<unknown>) {
+ export function* callSagaOnlyForAnvil(saga: GeneratorFunction, action: ReduxAction<unknown>) {

Committable suggestion

鈥硷笍 IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
export function* callSagaOnlyForAnvil(saga: any, action: ReduxAction<unknown>) {
const isAnvilLayout: boolean = yield select(getIsAnvilLayout);
if (isAnvilLayout) {
yield call(saga, action);
}
}
export function* callSagaOnlyForAnvil(saga: GeneratorFunction, action: ReduxAction<unknown>) {
const isAnvilLayout: boolean = yield select(getIsAnvilLayout);
if (isAnvilLayout) {
yield call(saga, action);
}
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

takeLeading is having type issues if I try this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marks0351, it seems like the type GeneratorFunction might not be directly compatible with what takeLeading expects for its saga parameter. To resolve the type issues, I recommend checking the type definition of takeLeading in your project's version of redux-saga. This will help you identify a compatible type for the saga parameter.

If you can provide the specific error message or a snippet of how you're using takeLeading with callSagaOnlyForAnvil, I can offer more targeted advice. Alternatively, you might consider using a type assertion if you're confident about the types being handled, or adjusting the type to something more specific that matches the expected input for takeLeading.

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { MAIN_CONTAINER_WIDGET_ID } from "constants/WidgetConstants";
import { useDetachedChildren } from "layoutSystems/anvil/common/hooks/detachedWidgetHooks";
import { getAnvilWidgetDOMId } from "layoutSystems/common/utils/LayoutElementPositionsObserver/utils";
import { renderChildWidget } from "layoutSystems/common/utils/canvasUtils";
import React from "react";
import { useSelector } from "react-redux";
import { getRenderMode } from "selectors/editorSelectors";
import type { WidgetProps } from "widgets/BaseWidget";

export const AnvilDetachedWidgets = () => {
const detachedChildren = useDetachedChildren();
const renderMode = useSelector(getRenderMode);
return (
<>
{detachedChildren.map((child) =>
renderChildWidget({
childWidgetData: child as WidgetProps,
defaultWidgetProps: {
className: `${getAnvilWidgetDOMId(child.widgetId)}`,
},
noPad: false,
// Adding these properties as the type insists on providing this
// while it is not required for detached children
layoutSystemProps: {
layoutId: "",
rowIndex: 0,
},
renderMode,
widgetId: MAIN_CONTAINER_WIDGET_ID,
}),
)}
</>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,18 @@ import React, { useMemo } from "react";
import styles from "./styles.module.css";
import type { BaseWidgetProps } from "widgets/BaseWidgetHOC/withBaseWidgetHOC";
import { getAnvilCanvasId } from "./utils";
import { useRenderDetachedChildren } from "layoutSystems/anvil/common/hooks/detachedWidgetHooks";
import { LayoutProvider } from "layoutSystems/anvil/layoutComponents/LayoutProvider";
import { AnvilDetachedWidgets } from "./AnvilDetachedWidgets";
export const AnvilViewerCanvas = React.forwardRef(
(props: BaseWidgetProps, ref: React.ForwardedRef<HTMLDivElement>) => {
const className: string = useMemo(
() => `${props.classList?.join(" ")} ${styles["anvil-canvas"]}`,
[props.classList],
);

const renderDetachedChildren = useRenderDetachedChildren(
props.widgetId,
props.children,
);

return (
<>
{renderDetachedChildren}
<AnvilDetachedWidgets />
<div
className={className}
id={getAnvilCanvasId(props.widgetId)}
Expand Down