chore: UI package main pane relevant ce changes#39961
Conversation
WalkthroughThis pull request introduces a new IDE type called Changes
Sequence Diagram(s)sequenceDiagram
participant S as Saga Caller
participant W as waitForInitialization
participant G as getIDETypeByUrl
participant P as waitForPackageInitialization
participant A as waitForAppInitialization
S->>W: Invoke waitForInitialization(saga, action)
W->>G: Determine IDE type from URL
G-->>W: Return IDE type
alt IDE_TYPE.UIPackage
W->>P: Call waitForPackageInitialization(saga, action)
else Other IDE type
W->>A: Call waitForAppInitialization(saga, action)
end
Note right of W: Continue with widget selection saga
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 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: 0
🧹 Nitpick comments (2)
app/client/src/ce/sagas/moduleInterfaceSagas.ts (1)
25-30: Placeholder saga for package initializationThis empty implementation provides the correct interface for the EE version to override. The ESLint directives appropriately suppress warnings for the unused parameters.
Would this function be called directly from other parts of the codebase? If so, consider adding a documentation comment explaining its purpose and expected EE behavior, similar to the file-level comment.
app/client/src/sagas/WidgetDeletionSagas.ts (1)
393-393: Confusing conditional logic could be simplified.The double negative in the condition
!(deletableWidgets && deletableWidgets.length !== 1)makes the code harder to read and understand. Consider rewriting this for clarity.-if (!(deletableWidgets && deletableWidgets.length !== 1)) return; +if (!deletableWidgets || deletableWidgets.length <= 1) return;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/client/src/ce/IDE/Interfaces/IDETypes.ts(1 hunks)app/client/src/ce/IDE/constants/routes.ts(1 hunks)app/client/src/ce/pages/Editor/Explorer/helpers.tsx(1 hunks)app/client/src/ce/sagas/NavigationSagas.ts(1 hunks)app/client/src/ce/sagas/moduleInterfaceSagas.ts(2 hunks)app/client/src/pages/Editor/PropertyPane/PropertyPaneView.tsx(3 hunks)app/client/src/sagas/WidgetDeletionSagas.ts(1 hunks)app/client/src/sagas/WidgetSelectionSagas.ts(3 hunks)app/client/src/utils/canvasStructureHelpers.ts(1 hunks)app/client/src/widgets/BaseWidget.tsx(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
app/client/src/sagas/WidgetSelectionSagas.ts (3)
app/client/src/ce/pages/Editor/Explorer/helpers.tsx (2)
isEditorPath(111-113)isUIPackageEditorPath(121-123)app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-5)app/client/src/ce/sagas/moduleInterfaceSagas.ts (1)
waitForPackageInitialization(25-30)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
🔇 Additional comments (14)
app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
4-4: New IDE type added appropriatelyThe addition of the
UIPackagetype in theIDE_TYPEconstant is consistent with the PR objectives to introduce UI package functionality.app/client/src/ce/pages/Editor/Explorer/helpers.tsx (1)
119-123: Placeholder implementation for EE extensionThis function correctly provides a CE implementation that will be extended in EE. The comment and ESLint directive are appropriate for this pattern.
app/client/src/ce/IDE/constants/routes.ts (1)
44-44: Empty path array initialized for UI PackageThe addition of an empty path array for
IDE_TYPE.UIPackageis appropriate as a placeholder. This allows for future path definitions while maintaining type safety in theIDEBasePathsconstant.app/client/src/ce/sagas/moduleInterfaceSagas.ts (1)
10-12: Required imports for new functionThese imports are necessary for the new
waitForPackageInitializationfunction signature.app/client/src/utils/canvasStructureHelpers.ts (1)
30-30: Appropriate export of utility function.Exporting
getCanvasStructureFromDSLmakes it accessible for reuse across the application, improving code modularity. This is a good refactoring practice.app/client/src/widgets/BaseWidget.tsx (1)
530-530: Good addition of widget deletion control.The new
isDeletableproperty enhances the widget framework by allowing certain widgets to be protected from deletion, aligning with the PR's objective of preventing accidental deletions of critical components.app/client/src/pages/Editor/PropertyPane/PropertyPaneView.tsx (1)
219-257: Correctly implements conditional deletion UI based on widget property.The code now checks the
isDeletableproperty before adding the delete button to widget actions, ensuring protected widgets cannot be accidentally deleted from the UI. This is consistent with the behavior defined in BaseWidget.tsx.app/client/src/ce/sagas/NavigationSagas.ts (1)
140-140: Appropriate export of saga function.Exporting
setSelectedWidgetsSagaimproves code modularity and allows for reuse in other sagas or components. This change supports the PR's objective of implementing context-specific widget initialization flows.app/client/src/sagas/WidgetDeletionSagas.ts (2)
389-391: Good addition of the isDeletable filter for widget selection.The implementation correctly filters out widgets that are explicitly marked as non-deletable, preventing their deletion even if they're selected. This is in line with the PR objective of enhancing protection for critical components.
396-396: Good implementation - using filtered widgets for deletion.Using the filtered
deletableWidgetsarray for mapping to ensure only deletable widgets are processed is the correct approach.app/client/src/sagas/WidgetSelectionSagas.ts (4)
57-66: Appropriate imports for supporting UI Package feature.These imports correctly add the necessary dependencies for supporting the new IDE type and initialization flow detection.
87-89: Improved editor path detection with UI Package support.The updated condition now correctly checks for both regular editor paths and UI package editor paths, making the widget selection logic more flexible.
280-288: Well-structured conditional initialization based on IDE type.The new implementation properly determines the IDE type from the URL and calls the appropriate initialization function. This clean separation of concerns supports the PR objective of context-specific widget initialization flows.
290-310: Appropriate extraction of app initialization logic.The
waitForAppInitializationfunction correctly preserves the original initialization logic while allowing for the new UI Package flow. The code maintains the necessary editor and page loading checks.
## Description This PR introduces several changes - Added isDeletable flag to widget canvas props - Modified Property Pane to hide delete buttons for protected widgets - Enhanced deletion sagas to filter out non-deletable widgets - Implemented context-specific widget initialization flows, i.e if in app editor then wait for app initialization else wait for package PR for appsmithorg/appsmith-ee#6805 ## Automation /ok-to-test tags="@tag.All" ### 🔍 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/14123255944> > Commit: 61de5b0 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=14123255944&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Fri, 28 Mar 2025 07:47:43 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** - Introduced support for a new editor package type, expanding available IDE options. - Updated widget configuration to include conditional deletion capabilities, ensuring delete actions appear only for eligible widgets. - **Refactor** - Enhanced navigation and initialization flows for a smoother editor experience. - Refined multi-widget deletion processes to safeguard against accidental removals. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
3 participants
Description
This PR introduces several changes
PR for https://github.com/appsmithorg/appsmith-ee/pull/6805
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/14123255944
Commit: 61de5b0
Cypress dashboard.
Tags:
@tag.AllSpec:
Fri, 28 Mar 2025 07:47:43 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Refactor