feat: implements onPageUnload functionality for the edit app mode page selector (#41074)
## Description <ins>Problem</ins> onPageUnload functionality was not consistently triggered during all types of page navigation in edit mode, leading to potential missed cleanup or actions when navigating between pages via different UI elements or programmatic flows. <ins>Root cause</ins> Navigation logic was fragmented across multiple components and methods (button clicks, navigation tabs, page navigator), and direct history manipulation bypassed centralized handling, preventing reliable invocation of onPageUnload actions. <ins>Solution</ins> This PR handles the integration of onPageUnload functionality with all page navigation flows in edit mode by centralizing navigation logic through the navigateToAnotherPage action, enhancing type safety, and ensuring onPageUnload actions are filtered and executed based on the current page context. Fixes #40998 _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.JS, @tag.Sanity" ### 🔍 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/16042398132> > Commit: 8ea04e6bb1312d9f468ed3d74ccc080ed6e9bac9 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=16042398132&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.JS, @tag.Sanity` > Spec: > <hr>Thu, 03 Jul 2025 06:44:33 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Enhanced page unload actions to only trigger for the current page, improving accuracy and reliability. * **Bug Fixes** * Improved navigation consistency by updating the page switching mechanism to use a unified action. * **Tests** * Added tests to ensure correct filtering of JavaScript actions executed on page unload. * **Refactor** * Streamlined selector logic for better maintainability and performance. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Rahul Barwal
GitHub
parent
b4d5685d21
commit
3421f8bfd1
|
|
@ -31,6 +31,7 @@ import type { ApiResponse } from "api/ApiResponses";
|
|||
import type { EvaluationReduxAction } from "./EvaluationReduxActionTypes";
|
||||
import { appsmithTelemetry } from "instrumentation";
|
||||
import type { NavigateToAnotherPagePayload } from "sagas/ActionExecution/NavigateActionSaga/types";
|
||||
import type { Path } from "history";
|
||||
|
||||
export interface FetchPageListPayload {
|
||||
applicationId: string;
|
||||
|
|
@ -699,7 +700,7 @@ export const setupPublishedPage = (
|
|||
});
|
||||
|
||||
export const navigateToAnotherPage = (
|
||||
payload: NavigateToAnotherPagePayload,
|
||||
payload: NavigateToAnotherPagePayload | Path,
|
||||
) => ({
|
||||
type: ReduxActionTypes.NAVIGATE_TO_ANOTHER_PAGE,
|
||||
payload,
|
||||
|
|
|
|||
|
|
@ -15,11 +15,10 @@ import {
|
|||
import { PERMISSION_TYPE, isPermitted } from "ee/utils/permissionHelpers";
|
||||
import { getCurrentApplication } from "ee/selectors/applicationSelectors";
|
||||
import type { DefaultRootState } from "react-redux";
|
||||
import { updatePageAction } from "actions/pageActions";
|
||||
import { navigateToAnotherPage, updatePageAction } from "actions/pageActions";
|
||||
import { useGetPageFocusUrl } from "./hooks/useGetPageFocusUrl";
|
||||
import AnalyticsUtil from "ee/utils/AnalyticsUtil";
|
||||
import { toggleInOnboardingWidgetSelection } from "actions/onboardingActions";
|
||||
import history, { NavigationMethod } from "utils/history";
|
||||
import { EntityItem } from "@appsmith/ads";
|
||||
import { useNameEditorState } from "IDE/hooks/useNameEditorState";
|
||||
import { useValidateEntityName } from "IDE";
|
||||
|
|
@ -90,9 +89,7 @@ export const PageEntity = ({
|
|||
toUrl: navigateToUrl,
|
||||
});
|
||||
dispatch(toggleInOnboardingWidgetSelection(true));
|
||||
history.push(navigateToUrl, {
|
||||
invokedBy: NavigationMethod.EntityExplorer,
|
||||
});
|
||||
dispatch(navigateToAnotherPage(navigateToUrl));
|
||||
|
||||
if (onClick) {
|
||||
onClick();
|
||||
|
|
|
|||
|
|
@ -1702,14 +1702,18 @@ export function* executePageUnloadActionsSaga() {
|
|||
const span = startRootSpan("executePageUnloadActionsSaga");
|
||||
|
||||
try {
|
||||
const pageActions: Action[] = yield select(getLayoutOnUnloadActions);
|
||||
const actionCount = pageActions.length;
|
||||
const pageOnUnloadActions: Action[] = yield select(
|
||||
getLayoutOnUnloadActions,
|
||||
);
|
||||
const actionCount = pageOnUnloadActions.length;
|
||||
|
||||
setAttributesToSpan(span, { numActions: actionCount });
|
||||
|
||||
// Execute unload actions in parallel batches
|
||||
yield all(
|
||||
pageActions.map((action) => call(executeOnPageUnloadJSAction, action)),
|
||||
pageOnUnloadActions.map((action) =>
|
||||
call(executeOnPageUnloadJSAction, action),
|
||||
),
|
||||
);
|
||||
|
||||
// Publish success event after all actions are executed
|
||||
|
|
|
|||
|
|
@ -0,0 +1,71 @@
|
|||
import type { DefaultRootState } from "react-redux";
|
||||
import { getLayoutOnUnloadActions } from "selectors/editorSelectors";
|
||||
|
||||
describe("getLayoutOnUnloadActions", () => {
|
||||
it("should filter actions by current page ID", () => {
|
||||
const state = {
|
||||
entities: {
|
||||
pageList: { currentPageId: "page1" },
|
||||
jsActions: [
|
||||
{
|
||||
isLoading: false,
|
||||
config: {
|
||||
id: "collection1",
|
||||
pageId: "page1",
|
||||
actions: [
|
||||
{
|
||||
id: "action1",
|
||||
pageId: "page1",
|
||||
runBehaviour: "ON_PAGE_UNLOAD",
|
||||
name: "myFun1",
|
||||
fullyQualifiedName: "JSObject1.myFun1",
|
||||
collectionId: "collection1",
|
||||
},
|
||||
{
|
||||
id: "action3",
|
||||
pageId: "page1",
|
||||
runBehaviour: "ON_PAGE_LOAD",
|
||||
name: "myFun2",
|
||||
fullyQualifiedName: "JSObject1.myFun2",
|
||||
collectionId: "collection1",
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
{
|
||||
isLoading: false,
|
||||
config: {
|
||||
id: "collection2",
|
||||
pageId: "page2",
|
||||
actions: [
|
||||
{
|
||||
id: "action2",
|
||||
pageId: "page2",
|
||||
runBehaviour: "ON_PAGE_UNLOAD",
|
||||
name: "myFun1",
|
||||
fullyQualifiedName: "JSObject2.myFun1",
|
||||
collectionId: "collection2",
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
|
||||
const result = getLayoutOnUnloadActions(
|
||||
state as unknown as DefaultRootState,
|
||||
);
|
||||
|
||||
expect(result).toEqual([
|
||||
{
|
||||
id: "action1",
|
||||
pageId: "page1",
|
||||
runBehaviour: "ON_PAGE_UNLOAD",
|
||||
name: "myFun1",
|
||||
fullyQualifiedName: "JSObject1.myFun1",
|
||||
collectionId: "collection1",
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
@ -125,16 +125,21 @@ export const getPageSavingError = (state: DefaultRootState) => {
|
|||
return state.ui.editor.loadingStates.savingError;
|
||||
};
|
||||
|
||||
export const getCurrentPageId = (state: DefaultRootState) =>
|
||||
state.entities.pageList.currentPageId;
|
||||
|
||||
export const getLayoutOnLoadActions = (state: DefaultRootState) =>
|
||||
state.ui.editor.pageActions || [];
|
||||
|
||||
export const getLayoutOnUnloadActions = createSelector(
|
||||
getCurrentPageId,
|
||||
getAllJSCollectionActions,
|
||||
(jsActions) => {
|
||||
return jsActions.filter((action) => {
|
||||
return action.runBehaviour === ActionRunBehaviour.ON_PAGE_UNLOAD;
|
||||
});
|
||||
},
|
||||
(currentPageId, jsActions) =>
|
||||
jsActions.filter(
|
||||
(action) =>
|
||||
action.runBehaviour === ActionRunBehaviour.ON_PAGE_UNLOAD &&
|
||||
action.pageId === currentPageId,
|
||||
),
|
||||
);
|
||||
|
||||
export const getLayoutOnLoadIssues = (state: DefaultRootState) => {
|
||||
|
|
@ -167,9 +172,6 @@ export const getPageByBaseId = (basePageId: string) =>
|
|||
pages.find((page) => page.basePageId === basePageId),
|
||||
);
|
||||
|
||||
export const getCurrentPageId = (state: DefaultRootState) =>
|
||||
state.entities.pageList.currentPageId;
|
||||
|
||||
export const getCurrentBasePageId = (state: DefaultRootState) =>
|
||||
state.entities.pageList.currentBasePageId;
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user