chore: EE widgets register and allow onboarding text for non main canvas widgets#39364
chore: EE widgets register and allow onboarding text for non main canvas widgets#39364ashit-rath merged 4 commits intoreleasefrom
Conversation
WalkthroughThis pull request introduces a new constant Changes
Possibly related PRs
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)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
|
ashit-rath
added
Frontend
github-actions
bot
added
the
skip-changelog
ashit-rath
requested review from
a team,
brayn003 and
rahulbarwal
and removed request for
AnaghHegde
![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 (1)
app/client/src/layoutSystems/common/dropTarget/DropTargetComponent.tsx (1)
328-331: Consider using optional chaining for safer null checks.The childWidgets check can be simplified using optional chaining.
- !(childWidgets && childWidgets.length) && + !childWidgets?.length && !isDragging && // If the parentId is not set (true for main canvas), or the showOnboardingText is true (for module canvas) (!props.parentId || props.showOnboardingText);🧰 Tools
🪛 Biome (1.9.4)
[error] 328-328: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/client/src/ee/widgets/index.ts(1 hunks)app/client/src/layoutSystems/common/dropTarget/DropTargetComponent.tsx(2 hunks)app/client/src/layoutSystems/fixedlayout/canvas/FixedLayoutEditorCanvas.tsx(2 hunks)app/client/src/widgets/index.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/client/src/ee/widgets/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/src/layoutSystems/common/dropTarget/DropTargetComponent.tsx
[error] 328-328: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- 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 (4)
app/client/src/layoutSystems/fixedlayout/canvas/FixedLayoutEditorCanvas.tsx (1)
41-41: LGTM! Clean implementation of the showOnboardingText prop.The prop is correctly added to fixedLayoutDropTargetProps and its dependency array.
Also applies to: 51-51
app/client/src/layoutSystems/common/dropTarget/DropTargetComponent.tsx (1)
55-55: LGTM! Type definition is properly extended.The showOnboardingText optional prop is correctly added to DropTargetComponentProps.
app/client/src/widgets/index.ts (2)
93-93: LGTM! Clean import of EE widgets.The import follows the established pattern for widget imports.
200-200:Details
❓ Verification inconclusive
LGTM! Proper integration of EE widgets.
The EE widgets are correctly integrated into the Widgets array, maintaining type safety with BaseWidget.
Let's verify the EE widgets integration:
🏁 Script executed:
#!/bin/bash # Description: Verify EE widgets folder structure and exports # Check if ee/widgets folder exists and contains an index.ts fd -t f "index.ts" -p "ee/widgets" # Check if EEWidgets is properly exported rg -l "export const EEWidgets" "ee/widgets"Length of output: 225
EE Widgets Integration Check Requires Manual Verification
The code at
app/client/src/widgets/index.ts(line 200) correctly uses the spread operator forEEWidgets, which seems to maintain type safety withBaseWidget. However, our automated search for the expectedee/widgetsfolder and the export ofEEWidgetsturned up no results. This suggests thatEEWidgetsmay be defined or imported from a different location than initially assumed.Please verify manually that:
- The
EEWidgetsexport exists in the codebase.- Its source location matches the intended design (or update the verification script if the folder path has changed).
…vas widgets (appsmithorg#39364) ## Description Adds a new folder `widgets` under ee to have EE specific widget and right now the requirement is from UI modules. The other changes is to allow to show `Drag and drop a widget here` text when there are no widgets dropped inside a canvas widget if it has flag `showOnboardingText` to true. Right now it is hardwired for main canvas only. This is a requirement for the UI module; since there would a container like widget as the starting point of a UI module, the empty state text has to be shown for the said container like widget. PR for appsmithorg/appsmith-ee#6272 ## 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/13490984047> > Commit: a9ac967 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13490984047&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Mon, 24 Feb 2025 06:05:12 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** - Expanded widget options with a newly introduced widget collection that broadens the available selections. - Enhanced onboarding display across interface components by adding configurable options for showing onboarding guidance, providing a smoother and more intuitive user experience. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Description
Adds a new folder
widgetsunder ee to have EE specific widget and right now the requirement is from UI modules.The other changes is to allow to show
Drag and drop a widget heretext when there are no widgets dropped inside a canvas widget if it has flagshowOnboardingTextto true. Right now it is hardwired for main canvas only. This is a requirement for the UI module; since there would a container like widget as the starting point of a UI module, the empty state text has to be shown for the said container like widget.PR for https://github.com/appsmithorg/appsmith-ee/pull/6272
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/13490984047
Commit: a9ac967
Cypress dashboard.
Tags:
@tag.AllSpec:
Mon, 24 Feb 2025 06:05:12 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit