-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat: hide ai chat 2 #36773
feat: hide ai chat 2 #36773
Conversation
WalkthroughThis pull request introduces several structural changes, including an updated import path for Changes
Possibly related PRs
Suggested labels
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
|
19e62d4
to
4adab52
Compare
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.
Caution
Inline review comments failed to post
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (2)
app/client/src/ce/modules/ui-builder/ui/wds/constants.ts (1)
Action Required: Remove Remaining AI Chat References
It appears that while the
WDS_AI_CHAT_WIDGET
entry has been removed, there are still references toAIChatThumbnail
andAIChatIcon
in the following files:
app/client/packages/icons/src/stories/Thumbnails.mdx
app/client/packages/icons/src/stories/Icons.mdx
Please remove these imports and component usages to ensure the AI Chat feature is fully hidden and does not cause any unintended side effects.
🔗 Analysis chain
Line range hint
29-65
: Students, let's discuss the removal of theWDS_AI_CHAT_WIDGET
entry.The
WDS_AI_CHAT_WIDGET
entry has been removed from ourWDS_V2_WIDGET_MAP
object. This change aligns with our lesson plan to hide the AI Chat feature. However, we must ensure this removal doesn't cause any unintended consequences in our codebase.Let's conduct a thorough search to identify any remaining references:
Let's take another look to ensure we catch any lingering references to the
WDS_AI_CHAT_WIDGET
.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to WDS_AI_CHAT_WIDGET # Test 1: Search for any remaining references to WDS_AI_CHAT_WIDGET rg --type typescript "WDS_AI_CHAT_WIDGET" # Test 2: Check for any imports or uses of AIChat component rg --type typescript "import.*AIChat|<AIChat"Length of output: 159
Script:
#!/bin/bash # Description: Check for any remaining references to WDS_AI_CHAT_WIDGET without specifying file type # Test 1: Search for any remaining references to WDS_AI_CHAT_WIDGET rg "WDS_AI_CHAT_WIDGET" # Test 2: Check for any imports or uses of AIChat component rg "import.*AIChat|<AIChat"Length of output: 439
app/client/src/layoutSystems/anvil/utils/anvilChecksUtils.ts (1)
Line range hint
1-93
: Well done on maintaining the function's integrity!Class, I'm pleased to see that the rest of the
updateAndSaveAnvilLayout
function remains unchanged. This is a good example of making targeted modifications without disrupting the overall functionality.However, let's not forget our golden rule: "Test, test, and test again!"
As your diligent teacher, I must remind you to ensure comprehensive testing of this function, particularly focusing on:
- The filtering of section widgets using
anvilWidgets.SECTION_WIDGET
- The overall behavior of the Anvil layout updates
Remember, a stitch in time saves nine! Thorough testing now can prevent headaches later.
🛑 Comments failed to post (7)
app/client/src/ce/modules/ui-builder/ui/wds/constants.ts (1)
1-1: 💡 Codebase verification
Import Path Consistency Needed
The import statement in
constants.ts
uses a relative path ("../../../../../utils/generators"
), while other files use an absolute path ("utils/generators"
). For better consistency and maintainability, consider updating the import inconstants.ts
to use the absolute path.🔗 Analysis chain
Class, let's examine this import statement change.
The import path for
getCanvasClassName
has been modified. This change suggests an alteration in our project structure. We must ensure that this new path is correct and consistent across our codebase.Let's run a quick check to verify this change:
Let's refine our verification process to address the issue with the
rg
command. We'll adjust the script to search without specifying the file type, which should help us identify any lingering old import paths.[/run_scripts]
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the import path change for getCanvasClassName # Test 1: Check if the new import path exists fd --type f "generators.ts" --exec grep -H "export.*getCanvasClassName" # Test 2: Check for any remaining old import paths rg --type typescript "import.*getCanvasClassName.*from.*utils/generators"Length of output: 270
Script:
#!/bin/bash # Description: Re-verify the import path change for getCanvasClassName without specifying file type # Test 1: Confirm the new import path exists fd --type f "generators.ts" --exec grep -H "export.*getCanvasClassName" # Test 2: Search for any remaining old import paths without specifying file type rg "import.*getCanvasClassName.*from.*utils/generators" -A 2Length of output: 2850
app/client/cypress/support/Pages/Anvil/Locators/index.ts (1)
3-3: 💡 Codebase verification
Import Path Issue Detected
The new import path
../../../../../src/ee/modules/ui-builder/ui/wds/constants
does not point to an existing file. Please verify and correct the import path to ensureAnvilDataAttributes
is properly referenced.🔗 Analysis chain
Class, let's examine this import statement change.
Good job updating the import path for
AnvilDataAttributes
. This change reflects a project restructuring, moving the constants to an enterprise edition folder.However, I'd like you to complete a few homework assignments:
- Double-check that the new import path is correct across all environments.
- Verify that this change doesn't break any non-enterprise edition builds or configurations.
- Ensure that the
AnvilDataAttributes
usage on line 10 still works as expected with this new import.Remember, attention to detail is crucial in software development!
Let's run a quick test to verify the new import path:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the new import path and its usage # Test 1: Check if the new file exists fd --type f --full-path "src/ee/modules/ui-builder/ui/wds/constants.ts" # Test 2: Verify the usage of AnvilDataAttributes in the file rg "AnvilDataAttributes" app/client/cypress/support/Pages/Anvil/Locators/index.tsLength of output: 317
app/client/src/layoutSystems/anvil/utils/layouts/update/mainCanvasLayoutUtils.ts (1)
14-14: 🛠️ Refactor suggestion
⚠️ Potential issueCarefully consider dependencies on the 'ee' module
This line introduces an import from the Enterprise Edition module:
import { anvilWidgets } from "ee/modules/ui-builder/ui/wds/constants";According to the PR objectives, we aim to hide the AI Chat feature without making modifications to the
ee
folder. Importing from theee
module may introduce unintended dependencies and could affect the open-source version of the application.Let's explore alternative solutions to access
anvilWidgets
without directly importing from theee
module. One approach is to moveanvilWidgets
to a common module that is accessible by both the open-source and enterprise editions. This way, we maintain the modularity and avoid cross-dependencies between editions.app/client/src/ce/widgets/index.ts (2)
190-193: 🛠️ Refactor suggestion
Consider reordering the
Widgets
array for clarityIn lines 190-193, the
Widgets
array is constructed by mergingWDSWidgets
,DeprecatedWidgets
, andLegacyWidgets
in that order. SinceDeprecatedWidgets
are no longer recommended for use, placing them afterLegacyWidgets
might help prevent accidental usage and improve code clarity.
1-196:
⚠️ Potential issueEnsure alignment with the PR objectives
The goal of this pull request is to hide the AI Chat feature without modifying the
"ee"
folder. However, the changes in this file don't appear to address the hiding of the AI Chat feature. Please double-check that all necessary modifications to hide AI Chat are included and that they align with the PR objectives.app/client/src/WidgetProvider/factory/helpers.ts (1)
20-20:
⚠️ Potential issueBe cautious when importing from 'ee' modules to prevent unintended dependencies
On line 20, the import statement has been updated to:
import { DEFAULT_WIDGET_ON_CANVAS_UI } from "ee/modules/ui-builder/ui/wds/constants";While this change may address immediate needs, importing directly from the
ee
(Enterprise Edition) modules can introduce unintended dependencies into the Community Edition codebase. According to the PR objectives, the goal is to hide the AI Chat feature without modifying the "ee" folder or affecting enterprise code.To maintain a clear separation between editions and avoid potential build or runtime issues, consider one of the following approaches:
Import from the non-enterprise module: Check if
DEFAULT_WIDGET_ON_CANVAS_UI
is available in the standard modules and update the import path accordingly.Abstract the constant to a common module: If the constant is needed in both editions, refactor it into a shared module that can be safely imported without crossing edition boundaries.
This will help ensure that the codebase remains modular and that functionalities specific to the Enterprise Edition do not inadvertently impact the Community Edition.
app/client/src/layoutSystems/anvil/editor/canvasArenas/utils/utils.ts (1)
10-10:
⚠️ Potential issueLet's carefully review the import statement to avoid unintended dependencies
Dear learner, I notice that on line 10, you've changed the import path to
"ee/modules/ui-builder/ui/wds/constants"
. While it's excellent to see you actively working on the code, we need to be cautious about importing modules from the enterprise edition (ee
) into the community edition (ce
) codebase. This can introduce dependencies that may lead to build issues or complicate the separation between the two editions. Could you explore alternative ways to achieve the desired functionality without relying on theee
directory?
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
🧹 Outside diff range and nitpick comments (5)
app/client/src/ce/widgets/index.ts (5)
1-90
: Well done on organizing your imports, class! A gold star for you!Your import statements are neatly arranged, showing a clear distinction between legacy and WDS widgets. However, to make it even better, consider grouping related imports together. For example, you could group all WDS widgets at the end of the import list.
Here's a little homework assignment for you:
- Move all WDS widget imports (lines 60-63 and 66-88) to the end of the import list.
- Keep the BaseWidget import (line 64) where it is, as it's a type import.
- Ensure that related widgets (like Button, IconButton, etc.) are grouped together.
This will make your code even more organized and easier to read. Keep up the good work!
91-142
: Excellent job cataloging your legacy widgets, students! You've earned a shiny apple sticker!Your LegacyWidgets array is a comprehensive collection of your older widget components. It's wonderful to see you keeping track of these!
However, let's think about how we can make this easier to manage as our widget collection grows. Here's a little group project for you:
Consider breaking this large array into smaller, themed sub-arrays. For example:
const InputLegacyWidgets = [InputWidgetV2, CurrencyInputWidget, PhoneInputWidget, /* ... */]; const VisualizationLegacyWidgets = [ChartWidget, MapWidget, MapChartWidget, /* ... */]; // ... more categories ... const LegacyWidgets = [ ...InputLegacyWidgets, ...VisualizationLegacyWidgets, // ... spread other categories ... ];Add comments to explain the purpose of each sub-array.
This approach will make it easier to find and manage related widgets. It's like organizing your crayon box by color - it makes everything neater and easier to use!
Keep up the fantastic work, and remember: organization is the key to success!
144-156
: A round of applause for keeping track of your deprecated widgets! You're showing great responsibility!It's wonderful to see you've separated your deprecated widgets. This is like keeping your old toys in a special box - it helps us remember what we've outgrown!
Let's make this even better with a small writing exercise:
Expand the comment to provide more context. For example:
// Deprecated Widgets: These widgets are no longer recommended for use in new development. // They are kept here for backwards compatibility with existing applications. // Consider using their WDS counterparts or other modern alternatives in new code.If possible, add a comment next to each deprecated widget suggesting its modern alternative. For example:
InputWidget, // Use WDSInputWidget instead DropdownWidget, // Use WDSSelectWidget instead // ... and so onThis will help your fellow developers understand why these widgets are deprecated and what they should use instead. It's like leaving a note in your lunchbox for your friend, telling them where to find the tastier snacks!
Keep up the excellent work in maintaining your widget collection!
158-188
: Bravo on embracing the Widget Design System! You're at the top of the class!Your WDSWidgets array is a shining example of modern widget organization. It's like having a brand new set of colored pencils - everything is sharp and ready to create beautiful designs!
Let's add some extra sparkle to this already stellar work:
Enhance the comment to provide more context:
// WDS Widgets: These widgets conform to our Widget Design System standards. // They represent the latest in our UI component library and should be preferred // for new development and when refactoring legacy code.Consider grouping related WDS widgets together within the array. For example:
// Input Widgets WDSInputWidget, WDSCurrencyInputWidget, WDSPhoneInputWidget, WDSEmailInputWidget, WDSPasswordInputWidget, WDSNumberInputWidget, WDSMultilineInputWidget, // Button Widgets WDSButtonWidget, WDSIconButtonWidget, WDSToolbarButtonsWidget, WDSInlineButtonsWidget, // ... other groups ...Add brief comments to describe each group.
This organization will make it easier for you and your classmates to find the right widget for your project, just like how organizing your school supplies makes it easier to find what you need for each subject!
Keep up the fantastic work in building a modern, consistent UI library!
190-196
: Excellent work on bringing all your widgets together! You've created a beautiful widget rainbow!Your consolidated Widgets array is like a grand finale performance, bringing together all the stars of your widget show. The typing as
(typeof BaseWidget)[]
ensures that all your widgets play by the same rules - bravo!To make this finale even more spectacular, let's add a little explanatory note:
// Consolidated Widgets array: Combines all widget types (WDS, Deprecated, and Legacy) // in order of preference. WDS widgets are prioritized, followed by Deprecated and Legacy. // This array is used as the single source of truth for available widgets in the application. const Widgets = [ ...WDSWidgets, ...DeprecatedWidgets, ...LegacyWidgets, ] as (typeof BaseWidget)[]; export default Widgets;This comment helps explain the purpose and structure of the Widgets array, making it easier for your fellow developers to understand its importance.
Remember, clear communication in code is like writing neat notes on the blackboard - it helps everyone understand the lesson better!
Keep up the fantastic work in organizing and documenting your widget library!
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (32)
- app/client/packages/design-system/widgets/src/components/AIChat/index.ts (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/AIChat.tsx (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/ChatTitle/ChatTitle.tsx (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/ChatTitle/index.ts (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/ChatTitle/styles.module.css (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/ChatTitle/types.ts (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/ThreadMessage/ThreadMessage.tsx (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/ThreadMessage/index.ts (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/ThreadMessage/styles.module.css (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/ThreadMessage/types.ts (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/UserAvatar/UserAvatar.tsx (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/UserAvatar/index.ts (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/UserAvatar/styles.module.css (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/UserAvatar/types.ts (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/index.ts (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/styles.module.css (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/src/types.ts (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/stories/AIChat.stories.tsx (0 hunks)
- app/client/packages/design-system/widgets/src/components/AIChat/tests/AIChat.test.tsx (0 hunks)
- app/client/packages/design-system/widgets/src/index.ts (0 hunks)
- app/client/src/ce/widgets/index.ts (1 hunks)
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/index.tsx (0 hunks)
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/anvilConfig.ts (0 hunks)
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/autocompleteConfig.ts (0 hunks)
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/defaultConfig.ts (0 hunks)
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/index.ts (0 hunks)
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/metaConfig.ts (0 hunks)
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/methodsConfig.ts (0 hunks)
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/propertyPaneContent.ts (0 hunks)
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/propertyPaneStyle.ts (0 hunks)
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/index.tsx (0 hunks)
- app/client/src/widgets/index.ts (0 hunks)
💤 Files with no reviewable changes (31)
- app/client/packages/design-system/widgets/src/components/AIChat/index.ts
- app/client/packages/design-system/widgets/src/components/AIChat/src/AIChat.tsx
- app/client/packages/design-system/widgets/src/components/AIChat/src/ChatTitle/ChatTitle.tsx
- app/client/packages/design-system/widgets/src/components/AIChat/src/ChatTitle/index.ts
- app/client/packages/design-system/widgets/src/components/AIChat/src/ChatTitle/styles.module.css
- app/client/packages/design-system/widgets/src/components/AIChat/src/ChatTitle/types.ts
- app/client/packages/design-system/widgets/src/components/AIChat/src/ThreadMessage/ThreadMessage.tsx
- app/client/packages/design-system/widgets/src/components/AIChat/src/ThreadMessage/index.ts
- app/client/packages/design-system/widgets/src/components/AIChat/src/ThreadMessage/styles.module.css
- app/client/packages/design-system/widgets/src/components/AIChat/src/ThreadMessage/types.ts
- app/client/packages/design-system/widgets/src/components/AIChat/src/UserAvatar/UserAvatar.tsx
- app/client/packages/design-system/widgets/src/components/AIChat/src/UserAvatar/index.ts
- app/client/packages/design-system/widgets/src/components/AIChat/src/UserAvatar/styles.module.css
- app/client/packages/design-system/widgets/src/components/AIChat/src/UserAvatar/types.ts
- app/client/packages/design-system/widgets/src/components/AIChat/src/index.ts
- app/client/packages/design-system/widgets/src/components/AIChat/src/styles.module.css
- app/client/packages/design-system/widgets/src/components/AIChat/src/types.ts
- app/client/packages/design-system/widgets/src/components/AIChat/stories/AIChat.stories.tsx
- app/client/packages/design-system/widgets/src/components/AIChat/tests/AIChat.test.tsx
- app/client/packages/design-system/widgets/src/index.ts
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/index.tsx
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/anvilConfig.ts
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/autocompleteConfig.ts
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/defaultConfig.ts
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/index.ts
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/metaConfig.ts
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/methodsConfig.ts
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/propertyPaneContent.ts
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/config/propertyPaneStyle.ts
- app/client/src/modules/ui-builder/ui/wds/WDSAIChatWidget/widget/index.tsx
- app/client/src/widgets/index.ts
🧰 Additional context used
I'm closing this PR since it's done here. |
Description
Hide AI Chat without touching ee folder
Fixes #
Issue Number
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=""
🔍 Cypress test results
Warning
Tests have not run on the HEAD 4adab52 yet
Wed, 09 Oct 2024 07:43:21 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
AIChat
component and its related files, streamlining the widget offerings.WDSWidgets
array to exclude theWDSAIChatWidget
.Chores
WDSAIChatWidget
.