chore: Fix layout issues for UI package editor #40349
Conversation
WalkthroughThis set of changes primarily refactors how the Changes
Sequence Diagram(s)sequenceDiagram
participant UI_Module
participant Widget
participant Redux_Store
UI_Module->>Widget: Render Widget with overrideRenderMode (optional)
Widget->>Redux_Store: Select renderMode (if no override)
Widget->>Widget: Use overrideRenderMode if provided
Widget->>UI_Module: Render with determined renderMode
sequenceDiagram
participant Sidebar
participant ParentComponent
ParentComponent->>Sidebar: Render with emptyMessage prop
Sidebar->>Sidebar: Display emptyMessage when no results
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/client/src/ce/reducers/index.tsx(1 hunks)app/client/src/ce/reducers/uiReducers/index.tsx(1 hunks)app/client/src/ce/reducers/uiReducers/mainCanvasReducer.ts(2 hunks)app/client/src/ce/sagas/PageSagas.tsx(1 hunks)app/client/src/ee/reducers/uiReducers/mainCanvasReducer.ts(1 hunks)app/client/src/layoutSystems/CanvasFactory.tsx(1 hunks)app/client/src/pages/AppIDE/components/WidgetAdd/WidgetsList.tsx(2 hunks)app/client/src/pages/Editor/utils.tsx(1 hunks)app/client/src/pages/Editor/widgetSidebar/UIEntitySidebar.tsx(2 hunks)app/client/src/sagas/AutoLayoutUpdateSagas.tsx(1 hunks)app/client/src/sagas/CanvasSagas/DraggingCanvasSagas.ts(1 hunks)app/client/src/selectors/editorSelectors.tsx(1 hunks)app/client/src/widgets/withWidgetProps.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
app/client/src/pages/AppIDE/components/WidgetAdd/WidgetsList.tsx (1)
app/client/src/ce/constants/messages.ts (1)
WIDGET_PANEL_EMPTY_MESSAGE(2523-2524)
app/client/src/ce/reducers/uiReducers/mainCanvasReducer.ts (2)
app/client/src/ce/constants/ReduxActionConstants.tsx (1)
ReduxActionTypes(1282-1325)app/client/src/utils/ReducerUtils.ts (1)
createImmerReducer(23-52)
app/client/src/layoutSystems/CanvasFactory.tsx (2)
app/client/src/selectors/editorSelectors.tsx (1)
getRenderMode(233-237)app/client/src/ce/selectors/applicationSelectors.tsx (1)
getAppThemeSettings(196-200)
app/client/src/widgets/withWidgetProps.tsx (1)
app/client/src/selectors/editorSelectors.tsx (1)
getRenderMode(233-237)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (19)
app/client/src/selectors/editorSelectors.tsx (1)
28-28: Import path updated to reference the enterprise edition module.The import for
MainCanvasReduxStatehas been updated to use the enterprise edition path. This aligns with the broader refactoring where the canvas reducer was moved from CE to EE.app/client/src/ce/sagas/PageSagas.tsx (1)
129-129: Import path updated for enterprise edition consistency.The import for
MainCanvasReduxStatenow correctly points to the enterprise edition reducer, maintaining consistency with the architectural refactoring.app/client/src/sagas/CanvasSagas/DraggingCanvasSagas.ts (1)
29-29: Import path updated for mainCanvasReducer type.Updated the import path for
MainCanvasReduxStateto use the enterprise edition module, consistent with other import path updates in the codebase.app/client/src/pages/AppIDE/components/WidgetAdd/WidgetsList.tsx (2)
8-8: Added import for empty message constant.Added the
WIDGET_PANEL_EMPTY_MESSAGEconstant import to support customizing the empty state message.
21-21: Added customizable empty message to UI sidebar.Added the
emptyMessageprop to theUIEntitySidebarcomponent, improving the user experience by providing a contextual message when no widgets match the search criteria.app/client/src/sagas/AutoLayoutUpdateSagas.tsx (1)
41-41: Import path correctly updated to reflect architectural changes.The import path for
MainCanvasReduxStatenow points to the enterprise edition module, consistent with the architectural improvements mentioned in the PR objectives.app/client/src/ce/reducers/uiReducers/index.tsx (1)
36-36: Import path updated correctly for architectural consistency.The import path for
mainCanvasReducerhas been properly updated to use the enterprise edition module, aligning with the architectural refactoring described in the PR objectives.app/client/src/ce/reducers/index.tsx (1)
51-51: Import path correctly updated for architectural consistency.The import path for
MainCanvasReduxStatenow references the enterprise edition module, maintaining consistency with the architectural changes in this PR.app/client/src/ee/reducers/uiReducers/mainCanvasReducer.ts (1)
1-3: LGTM: Clean proxy implementation for mainCanvasReducerThe implementation correctly re-exports all exports from the CE module and the default export with a clear alias. This supports the architectural migration from CE to EE structure while maintaining backward compatibility.
app/client/src/layoutSystems/CanvasFactory.tsx (1)
19-30: Implementation of render mode override looks goodThe change properly allows for conditional override of the render mode, which helps address the widget behavior inconsistencies in edit mode. The detailed comments explain the rationale behind this change and potential future improvements.
app/client/src/widgets/withWidgetProps.tsx (1)
85-93: Consistent implementation of render mode overrideThis change mirrors the implementation in CanvasFactory.tsx, providing a consistent pattern for overriding render mode. The comments clearly explain the purpose and context.
app/client/src/pages/Editor/widgetSidebar/UIEntitySidebar.tsx (3)
22-22: Added customizable empty message propertyAdding the optional emptyMessage prop to the interface enhances component flexibility and reusability.
27-27: Correctly added emptyMessage to component parametersThe component parameter list has been properly updated to include the new prop.
134-134: Implemented custom empty message displayThe implementation correctly uses the provided emptyMessage prop, concatenated with the current search value, for a more customizable user experience.
app/client/src/ce/reducers/uiReducers/mainCanvasReducer.ts (5)
10-10: Good addition of the klona library for deep cloning.The klona package is a lightweight and efficient deep cloning utility that's perfect for the use case below.
12-17: Proper export of initialState.Exporting the initialState is a good practice for maintainability and testability, especially since you're moving the reducer to EE.
19-45: Well-structured extraction of handlers.Extracting handlers into a separate exported object improves modularity and makes the code more maintainable. This approach will allow for better testing and potential extension in the EE version.
🧰 Tools
🪛 Biome (1.9.4)
[error] 27-28: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
47-47: Clean reducer implementation.The refactored reducer creation is concise and follows best practices by clearly separating the initial state and handlers.
42-44:❓ Verification inconclusive
Good implementation of reset functionality.
The reset handler provides a clean way to restore the initial state. Using klona ensures a proper deep copy without reference issues.
One consideration: verify that all consuming components handle this reset properly, especially if they depend on specific canvas state.
🏁 Script executed:
#!/bin/bash # Search for components that might be affected by canvas reset echo "Searching for components that consume mainCanvasReducer state..." rg -l "useSelector.*(\s*state\s*=>\s*state\.entities\.canvas|\s*selectMainCanvasProps|MainCanvasReduxState)" --type ts --type tsxLength of output: 298
🏁 Script executed:
#!/bin/bash # Searching for components that consume mainCanvasReducer state across .ts and .tsx files echo "Searching for useSelector patterns and selectors..." rg -l "useSelector.*\bstate\s*=>\s*state\.entities\.canvas" -g "*.ts" -g "*.tsx" rg -l "selectMainCanvasProps" -g "*.ts" -g "*.tsx" rg -l "MainCanvasReduxState" -g "*.ts" -g "*.tsx"Length of output: 622
🏁 Script executed:
#!/bin/bash # 1. Determine where mainCanvasReducer is mounted in the root reducer echo "Locating combineReducers in root reducer..." rg -n -C2 "combineReducers" -g "app/client/src/ce/reducers/index.tsx" # 2. Search for any useSelector calls that reference 'canvas' in TS/TSX files echo "Searching for useSelector patterns referencing 'canvas'..." rg -n "useSelector.*canvas" -g "*.ts" -g "*.tsx"Length of output: 348
Verify canvas reset integration in root reducer and sagas
Good implementation of reset functionality using
klonato deep‑copyinitialState.Before approving, please manually confirm:
Root reducer mounting
EnsuremainCanvasReduceris combined under the correct key (e.g.entities.canvas) in
app/client/src/ce/reducers/index.tsxSaga handlers and selectors
Sagas and selectors that referenceMainCanvasReduxStatemay assume a populated canvas. After reset they should handle the default shape without errors. Check in:
•app/client/src/sagas/AutoLayoutUpdateSagas.tsx
•app/client/src/sagas/WidgetDeletionSagas.ts
•app/client/src/sagas/CanvasSagas/DraggingCanvasSagas.ts
•app/client/src/ce/sagas/PageSagas.tsx
•app/client/src/selectors/editorSelectors.tsx
| } else { | ||
| groupedCards[tag] = [item]; | ||
| } |
There was a problem hiding this comment.
Bug fix for handling non-predefined widget tags.
This change correctly handles the case where a widget has tags that aren't predefined in the WIDGET_TAGS constant, ensuring all tags are properly processed regardless of whether they're in the predefined list.
There was a problem hiding this comment.
What is the issue?
There was a problem hiding this comment.
@ashit-rath The issue is in the groupWidgetCardsByTags function. Before this fix, if a widget had a tag that wasn't predefined in WIDGET_TAGS, the code would ignore that tag without creating an entry for it in groupedCards.
Consider a scenario where a widget has a custom tag "newFeature" that isn't in WIDGET_TAGS. Without the fix, groupedCards["newFeature"] would remain undefined, potentially causing errors when accessing it elsewhere in the code.
The fix ensures that even non-predefined tags get properly initialized with an array containing the widget, preventing potential undefined access errors and making sure all tagged widgets are properly grouped regardless of whether their tags are predefined.
There was a problem hiding this comment.
Yes that's what we want
There was a problem hiding this comment.
@ashit-rath Got it! Thanks for confirming that the change to handle non-predefined widget tags is intentional and desired.
ashit-rath
added
Frontend
Description
🏗 Architecture
🐛 Bug Fixes
PR for https://github.com/appsmithorg/appsmith-ee/pull/7245
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/14610885832
Commit: 54916f7
Cypress dashboard.
Tags:
@tag.AllSpec:
Wed, 23 Apr 2025 07:02:40 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Refactor