-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat: made changes to expose the App's height and width as part of the appsmith.ui state object #41339
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…tate object. And added reduces, actions and saga to update it dynamically whenever the browser height or width changes.
WalkthroughAdds Redux-backed window dimension tracking: new action, reducer, selector (feature-flag gated), and component mounts dispatch initial dimensions. Data tree includes window dimensions under Changes
Sequence DiagramsequenceDiagram
participant Component as Component\n(AppIDE / AppPage)
participant Redux as Redux\n(Store / Dispatch)
participant Reducer as windowReducer
participant Selector as getWindowDimensions\n(feature-flag check)
participant DataTree as dataTree.appsmith
Component->>Redux: dispatch updateWindowDimensions(height, width) on mount
Redux->>Reducer: ACTION: UPDATE_WINDOW_DIMENSIONS {height, width}
Reducer->>Redux: update state.ui.windowDimensions
Selector->>Redux: read state and check release_window_dimensions_enabled
Selector-->>DataTree: expose windowDimensions as appsmith.ui (when enabled)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/client/src/pages/AppViewer/AppPage/AppPage.tsx (1)
51-67: Consider debouncing the resize event to improve performance.The resize listener dispatches updates on every resize event, which triggers data tree re-evaluation via WindowSaga. This could cause performance issues with rapid resize events.
Apply this diff to add debouncing:
+import { debounce } from "lodash"; + // Set up window resize listener for window dimensions useEffect(() => { - const handleResize = () => { + const handleResize = debounce(() => { dispatch(updateWindowDimensions(window.innerHeight, window.innerWidth)); - }; + }, 150); // Set initial dimensions immediately dispatch(updateWindowDimensions(window.innerHeight, window.innerWidth)); // Add resize listener window.addEventListener("resize", handleResize); // Cleanup return () => { window.removeEventListener("resize", handleResize); + handleResize.cancel(); }; }, [dispatch]);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/client/src/actions/windowActions.ts(1 hunks)app/client/src/ce/constants/ReduxActionConstants.tsx(2 hunks)app/client/src/ce/reducers/uiReducers/index.tsx(2 hunks)app/client/src/ce/sagas/index.tsx(2 hunks)app/client/src/pages/AppIDE/AppIDE.tsx(5 hunks)app/client/src/pages/AppViewer/AppPage/AppPage.tsx(3 hunks)app/client/src/reducers/uiReducers/windowReducer.ts(1 hunks)app/client/src/sagas/WindowSaga.ts(1 hunks)app/client/src/selectors/dataTreeSelectors.ts(4 hunks)app/client/src/selectors/windowSelectors.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
app/client/src/pages/AppViewer/AppPage/AppPage.tsx (1)
app/client/src/actions/windowActions.ts (1)
updateWindowDimensions(3-6)
app/client/src/sagas/WindowSaga.ts (4)
app/client/src/entities/DataTree/dataTreeTypes.ts (1)
unEvalAndConfigTree(9-12)app/client/src/selectors/dataTreeSelectors.ts (1)
getUnevaluatedDataTree(146-200)app/client/src/sagas/EvaluationsSaga.ts (1)
evaluateTreeSaga(353-426)app/client/src/ce/constants/ReduxActionConstants.tsx (1)
ReduxActionTypes(1303-1348)
app/client/src/pages/AppIDE/AppIDE.tsx (2)
app/client/src/ce/constants/routes/appRoutes.ts (1)
BuilderRouteParams(142-145)app/client/src/actions/windowActions.ts (1)
updateWindowDimensions(3-6)
app/client/src/actions/windowActions.ts (1)
app/client/src/ce/constants/ReduxActionConstants.tsx (1)
ReduxActionTypes(1303-1348)
app/client/src/ce/sagas/index.tsx (1)
app/client/src/sagas/WindowSaga.ts (1)
windowSaga(12-17)
app/client/src/selectors/dataTreeSelectors.ts (1)
app/client/src/selectors/windowSelectors.ts (1)
getWindowDimensions(3-5)
app/client/src/reducers/uiReducers/windowReducer.ts (2)
app/client/src/utils/ReducerUtils.ts (1)
createImmerReducer(24-57)app/client/src/actions/ReduxActionTypes.ts (1)
ReduxAction(9-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
🔇 Additional comments (13)
app/client/src/ce/reducers/uiReducers/index.tsx (1)
36-36: LGTM!The windowReducer is properly imported and exposed as
windowDimensionsin the UI reducer object, consistent with the state structure.Also applies to: 89-89
app/client/src/ce/sagas/index.tsx (1)
12-12: LGTM!The windowSaga is properly imported and registered in the sagas array, enabling window dimension update handling.
Also applies to: 99-99
app/client/src/ce/constants/ReduxActionConstants.tsx (1)
1216-1218: LGTM!The WindowActionsTypes constant is properly defined and spread into ReduxActionTypes, following the established pattern in this file.
Also applies to: 1311-1311
app/client/src/selectors/windowSelectors.ts (1)
1-5: LGTM!The selector is straightforward and correctly typed, following the standard pattern for accessing Redux state.
app/client/src/reducers/uiReducers/windowReducer.ts (1)
5-23: LGTM!The reducer is correctly implemented with proper SSR handling (window existence check), clear interface definition, and follows the Immer pattern consistent with other reducers in the codebase.
app/client/src/actions/windowActions.ts (1)
1-6: LGTM!The action creator is straightforward, type-safe, and follows the standard pattern for creating Redux actions.
app/client/src/sagas/WindowSaga.ts (2)
1-4: LGTM!Imports are appropriate for the saga implementation.
12-17: Saga structure is correct.Using
takeLatestproperly ensures pending evaluations are cancelled when new resize events arrive. However, consider the debouncing suggestion above for better performance.app/client/src/pages/AppIDE/AppIDE.tsx (5)
41-41: LGTM!Import is correct and necessary for the window dimension tracking feature.
64-64: LGTM!Type signature correctly matches the action creator interface.
71-71: LGTM!Correctly declares a field to store the resize handler reference for cleanup.
176-180: LGTM!Event listener cleanup is correctly implemented, preventing memory leaks.
240-241: LGTM!Redux action mapping is correctly implemented.
app/client/src/sagas/WindowSaga.ts
Outdated
| export function* handleWindowDimensionsUpdate() { | ||
| const unEvalAndConfigTree = yield select(getUnevaluatedDataTree); | ||
|
|
||
| yield call(evaluateTreeSaga, unEvalAndConfigTree); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add debouncing to prevent excessive tree re-evaluations.
Window resize events fire rapidly during user interaction. Re-evaluating the entire data tree on each event can severely impact performance, especially in applications with complex state. Even with takeLatest, evaluations may start before resizing completes.
Consider adding a debounce pattern. Replace takeLatest in the saga with a debounced approach:
+import { debounce } from "redux-saga/effects";
+
export default function* windowSaga() {
- yield takeLatest(
+ yield debounce(
+ 250, // Wait 250ms after last resize event
ReduxActionTypes.UPDATE_WINDOW_DIMENSIONS,
handleWindowDimensionsUpdate,
);
}This ensures the tree is re-evaluated only after the user has finished resizing, rather than continuously during the resize operation.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/client/src/sagas/WindowSaga.ts around lines 6 to 10, the saga currently
calls evaluateTreeSaga directly on every resize which causes excessive
re-evaluations; replace this with a debounced approach so the tree is evaluated
only after resize noise settles. Implement a debounce by creating a watcher that
collects resize actions and delays execution (e.g., 200–300ms) before calling
evaluateTreeSaga, cancelling/resetting the delay on new resize events (use
redux-saga's debounce helper from redux-saga/effects or implement with a loop
using take, fork, delay and cancel). Ensure the debounced handler still selects
getUnevaluatedDataTree before calling evaluateTreeSaga and that cancellation
logic prevents multiple concurrent evaluations.
| // taking precedence in case the key is the same | ||
| store: appData.store, | ||
| theme, | ||
| ui: windowDimensions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify the structure of appsmith.ui to match the intended API.
The assignment ui: windowDimensions directly exposes the window dimensions object (with height and width properties) as appsmith.ui. Ensure this matches the desired public API structure and doesn't conflict with any existing ui property.
🏁 Script executed:
#!/bin/bash
# Search for existing references to appsmith.ui to verify structure expectations
rg -nP 'appsmith\.ui\b' --type=ts --type=tsx -C3Length of output: 27
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for existing references to appsmith.ui with corrected type flags
rg -nP 'appsmith\.ui\b' --type=typescript -C3
Length of output: 34
🏁 Script executed:
#!/bin/bash
# Also search for ui property assignments and windowDimensions usage in dataTree context
rg -nP '(ui\s*:|windowDimensions)' app/client/src/selectors/dataTreeSelectors.ts -C5Length of output: 691
🏁 Script executed:
#!/bin/bash
# Search for dataTree object construction to understand the structure
rg -nP 'dataTree\s*=\s*\{' --type=typescript -C10Length of output: 34
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the AppsmithEntity type definition
rg -nP 'type AppsmithEntity|interface AppsmithEntity' -C5
Length of output: 2717
🏁 Script executed:
#!/bin/bash
# Search for ui property references in the same file
rg -n 'ui:' app/client/src/selectors/dataTreeSelectors.tsLength of output: 31
🏁 Script executed:
#!/bin/bash
# Find windowDimensions type/definition
rg -nP 'windowDimensions\s*[=:|]' -C3 | head -50Length of output: 613
🏁 Script executed:
#!/bin/bash
# Search for appsmith.ui references without type flags
rg -n 'appsmith\.ui' -C2Length of output: 3987
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the complete AppsmithEntity interface definition
rg -n 'export interface AppsmithEntity' -A20 app/client/src/ce/entities/DataTree/types.ts
Length of output: 451
🏁 Script executed:
#!/bin/bash
# Also check if ui is defined in AppDataState (which AppsmithEntity extends from)
rg -n 'interface AppDataState|type AppDataState' -A15Length of output: 1319
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the complete selector function and type casting context around line 193
rg -n 'as AppsmithEntity' app/client/src/selectors/dataTreeSelectors.ts -B10 -A2
Length of output: 656
🏁 Script executed:
#!/bin/bash
# Search for any type extensions or augmentations of AppsmithEntity that add ui property
rg -n 'AppsmithEntity' app/client/src/ce/entities/DataTree/types.ts -A30Length of output: 906
Add ui property to the AppsmithEntity interface definition.
The assignment of ui: windowDimensions at line 188 is being bypassed by the as AppsmithEntity cast at line 193, but the ui property is not declared in the interface. Production code in ExternalWidget and CustomWidget actively consumes appsmith.ui with a {width, height} structure, confirming this is intentional.
Update app/client/src/ce/entities/DataTree/types.ts to declare the property:
export interface AppsmithEntity extends Omit<AppDataState, "store"> {
ENTITY_TYPE: typeof ENTITY_TYPE.APPSMITH;
store: Record<string, unknown>;
theme: AppTheme["properties"];
ui: { width: number; height: number };
currentPageName: string;
workspaceName: string;
appName: string;
currentEnvironmentName: string;
}🤖 Prompt for AI Agents
In app/client/src/ce/entities/DataTree/types.ts (update the AppsmithEntity
interface), add the missing ui property declaration so AppsmithEntity includes
ui: { width: number; height: number }; keep the rest of the interface as-is
(Omit<AppDataState, "store">, ENTITY_TYPE, store, theme, currentPageName,
workspaceName, appName, currentEnvironmentName) to match runtime usage where
code sets appsmith.ui = windowDimensions; ensure any necessary imports/types
referenced (e.g., AppTheme) remain unchanged.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/18904785141. |
|
Deploy-Preview-URL: https://ce-41339.dp.appsmith.com |
rahulbarwal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I would not open it for everyone, please put this behind a feature flag.
- This can become hazardous for big apps. Please test large apps, I think performance team had one named Xavier.
- Specifically on windows.
…ents to avoid heavy evaluations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/pages/AppViewer/AppPage/AppPage.tsx (1)
54-77: Solid implementation with proper cleanup. Clarify the debounce multiplier.The resize listener implementation follows best practices: immediate initialization, debounced updates, and thorough cleanup. However, the use of
RESIZE_DEBOUNCE_THRESHOLD * 2(line 62) is unexplained. Why double the threshold? Consider either documenting the rationale or using a separate named constant for clarity.Additionally, the AI summary mentions AppIDE has similar logic. If this pattern repeats across multiple components, consider extracting it into a shared custom hook (e.g.,
useWindowDimensions) to reduce duplication.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/pages/AppIDE/AppIDE.tsx(5 hunks)app/client/src/pages/AppViewer/AppPage/AppPage.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/pages/AppIDE/AppIDE.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
app/client/src/pages/AppViewer/AppPage/AppPage.tsx (1)
app/client/src/actions/windowActions.ts (1)
updateWindowDimensions(3-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
🔇 Additional comments (1)
app/client/src/pages/AppViewer/AppPage/AppPage.tsx (1)
4-4: LGTM! Clean integration of Redux dispatch.The imports and dispatch initialization are well-structured for the new window dimension tracking feature.
Also applies to: 11-13, 30-30
…ling the updateWindowDimensions on page load of editor and view mode. If any browser resize happens, user is expected refresh page.
|
@rahulbarwal , i have removed the resize handlers. Now on page load only will the window dimensions be updated. If there is any browser resize, the user would have to refresh the page to see the changes. |
…nly when its enabled will the appsmith state object have a ui object. Currently setting it as true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/ce/entities/FeatureFlag.ts (1)
67-67: Consider defaulting the feature flag tofalsefor controlled rollout.Given that window dimensions only update on page load (not on browser resize per your PR comment) and you've expressed uncertainty about whether this should be behind a feature flag, starting with
falsewould allow for a more gradual rollout and easier rollback if issues arise.Apply this diff to set the default to
false:- release_window_dimensions_enabled: true, + release_window_dimensions_enabled: false,Also applies to: 120-120
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/ce/entities/FeatureFlag.ts(2 hunks)app/client/src/selectors/windowSelectors.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/client/src/selectors/windowSelectors.ts (1)
app/client/src/ce/entities/FeatureFlag.ts (1)
FEATURE_FLAG(2-68)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: client-build / client-build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
🔇 Additional comments (1)
app/client/src/selectors/windowSelectors.ts (1)
5-12: Now I need to search the actual codebase for the WindowDimensionsState type and verify the selector implementation:Add explicit return type to selector and ensure all consumers handle the undefined case.
The
state.ui.windowDimensionsproperty is properly typed asWindowDimensionsStatein the Redux state definition. However, the selector should explicitly declare its return typeWindowDimensionsState | undefinedto make the conditional return value explicit for all consumers. Verify that all code using this selector safely handles theundefinedcase when the feature flag is disabled.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/19136258487. |
|
Deploy-Preview-URL: https://ce-41339.dp.appsmith.com |
Description
Introducing this property as part of appsmith.ui state object, so that it can be used in cases like passing to a custom widget which can help in determining height of the widget with respective to the App's visible height
Example of usage below:
appsmith-ui-dimensions.mp4
Additional Info: Controlling this by a feature flag only at the selector level. Could not do it at the component level as one of the dispatch action occurs from a class based component that does not support hooks like useFeatureFlag
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Fixes #
Issue Numberor
Fixes
Issue URLWarning
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.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/19159166010
Commit: 134a977
Cypress dashboard.
Tags:
@tag.AllSpec:
Fri, 07 Nov 2025 06:39:25 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit