Conversation
WalkthroughThe recent changes enhance the widget ID generation in the application by introducing structured prefixes for different widget types. This improves the organization and identification of widgets, making it easier to manage them. Specifically, zones are prefixed with "zone-" and sections with "section-", allowing for greater clarity and uniqueness in the widget management process. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WidgetCard
participant SectionUtils
participant ZoneUtils
User->>WidgetCard: Drag widget
WidgetCard->>generateReactKey: Generate widgetId with type prefix
alt Zone Widget
generateReactKey-->>WidgetCard: Returns zone widgetId
WidgetCard->>SectionUtils: Add zone widget with zone widgetId
else Other Widget
generateReactKey-->>WidgetCard: Returns component widgetId
WidgetCard->>SectionUtils: Add component widget with component widgetId
end
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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
Outside diff range, codebase verification and nitpick comments (2)
app/client/src/layoutSystems/anvil/utils/sectionOperationUtils.ts (1)
170-170: Attention Needed: UpdategenerateReactKeyFunction CallsThe code changes in
addNewZonesToSectionare approved. However, several instances ofgenerateReactKeyin the codebase do not follow the new signature, which includes an optional object parameter with aprefixkey.Please update the following instances to ensure consistency:
app/client/test/factories/Widgets/DividerFactory.tsapp/client/test/factories/Widgets/DatepickerFactory.tsapp/client/test/factories/Widgets/RadiogroupFactory.tsapp/client/test/factories/Widgets/VideoFactory.tsapp/client/test/factories/Widgets/TabsFactory.tsapp/client/test/factories/Widgets/DropdownFactory.tsapp/client/test/factories/Widgets/RichTextFactory.tsapp/client/test/factories/Widgets/ModalFactory.tsapp/client/test/factories/Widgets/MapFactory.tsapp/client/test/factories/Widgets/InputFactory.tsapp/client/test/factories/Widgets/ImageFactory.tsapp/client/test/factories/Widgets/IconFactory.tsapp/client/test/factories/Widgets/FormButtonFactory.tsapp/client/test/factories/Widgets/FilepickerFactory.tsapp/client/test/factories/Widgets/TextFactory.tsapp/client/test/factories/Widgets/SwitchFactory.tsapp/client/test/factories/Widgets/ChartFactory.tsapp/client/test/factories/Widgets/CanvasFactory.tsapp/client/test/factories/Widgets/SkeletonFactory.tsapp/client/test/factories/Widgets/FormFactory.tsapp/client/test/factories/Widgets/ContainerFactory.tsapp/client/test/factories/Widgets/TableFactory.tsapp/client/test/factories/Widgets/ListFactory.tsapp/client/test/factories/Widgets/CheckboxFactory.tsapp/client/test/factories/Widgets/ButtonFactory.tsapp/client/src/widgets/wds/WDSTableWidget/component/Constants.tsapp/client/src/widgets/TabsMigrator/widget/index.tsxapp/client/src/widgets/TableWidgetV2/component/Constants.tsapp/client/src/widgets/TableWidgetV2/component/header/actions/filter/FilterPaneContent.tsxapp/client/src/widgets/ListWidgetV2/MetaWidgetGenerator.tsapp/client/src/widgets/JSONFormWidget/fields/ArrayField.tsxapp/client/src/widgets/ContainerWidget/widget/index.test.tsxapp/client/src/widgets/ChartWidget/widget/index.tsxapp/client/src/utils/editor/browserTabsTracking.tsapp/client/src/sagas/WidgetBlueprintSagas.tsapp/client/src/sagas/WidgetOperationUtils.tsapp/client/src/sagas/WidgetOperationSagas.tsxapp/client/src/sagas/WidgetAdditionSagas.tsapp/client/src/sagas/ModalSagas.tsapp/client/src/sagas/BuildingBlockSagas/BuildingBlockAdditionSagas.tsapp/client/src/mocks/widgetProps/button.tsapp/client/src/layoutSystems/anvil/layoutComponents/presets/DefaultLayoutPreset.tsxapp/client/src/layoutSystems/anvil/layoutComponents/presets/sectionPreset.tsxapp/client/src/mocks/widgetProps/container.tsapp/client/src/layoutSystems/anvil/layoutComponents/presets/ModalPreset.tsxapp/client/src/layoutSystems/anvil/layoutComponents/presets/FormPreset.tsxapp/client/src/layoutSystems/anvil/layoutComponents/presets/StatboxPreset.tsxapp/client/src/layoutSystems/anvil/layoutComponents/presets/zonePreset.tsxapp/client/src/mocks/widgetProps/input.tsapp/client/src/mocks/widgetProps/canvas.tsapp/client/src/layoutSystems/anvil/utils/AnvilDSLTransformer.tsapp/client/src/layoutSystems/anvil/utils/paste/utils.tsapp/client/src/layoutSystems/anvil/utils/layouts/update/additionUtils.tsapp/client/src/layoutSystems/anvil/utils/layouts/update/sectionUtils.tsapp/client/src/layoutSystems/autolayout/layoutComponents/presets/ModalPreset.tsapp/client/src/layoutSystems/anvil/utils/layouts/update/zoneUtils.tsapp/client/src/layoutSystems/anvil/integrations/sagas/anvilDraggingSagas/anvilDraggingSagas.test.tsapp/client/src/layoutSystems/anvil/integrations/sagas/pasteSagas/pasteSagas.test.tsapp/client/src/mocks/layoutComponents/layoutComponentMock.tsapp/client/src/layoutSystems/fixedlayout/editor/FixedLayoutCanvasArenas/CanvasSelectionArena.test.tsxapp/client/src/pages/Editor/widgetSidebar/WidgetCard.tsxapp/client/src/pages/Editor/QueryEditor/BindDataButton.tsxapp/client/src/pages/Editor/Popper.tsxapp/client/src/pages/Editor/IDE/AppsmithIDE.test.tsxapp/client/src/pages/Editor/Explorer/Actions/helpers.tsxapp/client/src/components/propertyControls/ToolbarButtonListControl.tsxapp/client/src/components/propertyControls/MenuItemsControl.tsxapp/client/src/components/propertyControls/KeyValueComponent.tsxapp/client/src/components/propertyControls/IconSelectControlV2.tsxapp/client/src/components/propertyControls/IconSelectControl.tsxapp/client/src/components/propertyControls/ColumnActionSelectorControl.tsxapp/client/src/components/editorComponents/ActionCreator/index.tsxapp/client/src/components/editorComponents/ActionCreator/viewComponents/TextView/index.tsxapp/client/src/components/propertyControls/ChartDataControl.tsxapp/client/src/components/propertyControls/ButtonListControl.tsxapp/client/src/WidgetProvider/factory/index.tsxapp/client/src/WidgetProvider/factory/helpers.tsapp/client/packages/dsl/src/migrate/utils.tsapp/client/packages/dsl/src/migrate/tests/ChartDataFromArrayFromArrayToObject.test.tsapp/client/packages/dsl/src/migrate/migrations/012-migrate-incorrect-dynamic-binding-path-lists.tsapp/client/packages/dsl/src/migrate/migrations/016-migrate-chart-data-from-array-to-object.tsapp/client/packages/dsl/src/migrate/migrations/001-update-containers.tsPlease ensure all these instances are updated to use the new signature of
generateReactKey.Analysis chain
LGTM! But verify the function usage in the codebase.
The code changes are approved. The addition of a prefix to the widget ID generation will help in debugging and managing widgets.
However, ensure that all function calls to
generateReactKeymatch the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `generateReactKey` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ts -A 5 $'generateReactKey'Length of output: 114083
app/client/src/layoutSystems/anvil/utils/layouts/update/sectionUtils.ts (1)
Line range hint
89-144:
Reminder: Address the maxChildLimit issue.The TODO comment indicates a potential issue with the maxChildLimit of 4 for sections. This needs to be addressed to prevent adding too many zones to a section.
Do you want me to help address this issue or open a GitHub issue to track this task?
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- app/client/src/layoutSystems/anvil/utils/layouts/update/sectionUtils.ts (1 hunks)
- app/client/src/layoutSystems/anvil/utils/layouts/update/zoneUtils.ts (1 hunks)
- app/client/src/layoutSystems/anvil/utils/sectionOperationUtils.ts (1 hunks)
- app/client/src/pages/Editor/widgetSidebar/WidgetCard.tsx (1 hunks)
Additional comments not posted (6)
app/client/src/pages/Editor/widgetSidebar/WidgetCard.tsx (1)
131-137: LGTM! But verify the function usage in the codebase.The code changes are approved. The addition of a prefix to the widget ID generation will help in debugging and managing widgets.
However, ensure that all function calls to
generateReactKeymatch the new signature.app/client/src/layoutSystems/anvil/utils/layouts/update/zoneUtils.ts (1)
40-40: LGTM! But verify the function usage in the codebase.The code changes are approved. The addition of a prefix to the widget ID generation will help in debugging and managing widgets.
However, ensure that all function calls to
generateReactKeymatch the new signature.app/client/src/layoutSystems/anvil/utils/layouts/update/sectionUtils.ts (4)
Line range hint
41-52:
FunctionsplitWidgetsis well-implemented.The function correctly splits widgets into zones and non-zones.
Line range hint
54-87:
FunctionaddZoneToSectionis well-implemented.The function correctly handles adding zones to sections, both for new and existing widgets.
Line range hint
146-168:
FunctionmoveWidgetsToSectionis well-implemented.The function correctly handles moving widgets to a section and updating their relationships.
31-33: LGTM! But verify the function usage in the codebase.The change to prefix
widgetIdwith "section-" is a good practice for better identification and debugging. Ensure that all function calls tocreateSectionAndAddWidgethandle the newwidgetIdformat correctly.Verification successful
Let's verify how the
widgetIdis handled in themainCanvasLayoutUtils.tsfile after calling thecreateSectionAndAddWidgetfunction to ensure the new format is correctly managed.
Function usage verified successfully!
The
widgetIdgenerated with the "section-" prefix is being correctly handled in themainCanvasLayoutUtils.tsfile. After callingcreateSectionAndAddWidget, thewidgetIdis accessed and added tomainCanvasWidget.childrenas expected.
mainCanvasLayoutUtils.ts: Proper handling ofwidgetIdafter callingcreateSectionAndAddWidget.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `createSectionAndAddWidget` handle the new widgetId format correctly. # Test: Search for the function usage. Expect: Proper handling of the new widgetId format. rg --type ts -A 5 $'createSectionAndAddWidget'Length of output: 2068
Script:
#!/bin/bash # Description: Verify how `widgetId` is used in `mainCanvasLayoutUtils.ts` after calling `createSectionAndAddWidget`. # Test: Search for the usage of `widgetId` in `mainCanvasLayoutUtils.ts`. Expect: Proper handling of the new widgetId format. rg --type ts -A 10 'createSectionAndAddWidget' app/client/src/layoutSystems/anvil/utils/layouts/update/mainCanvasLayoutUtils.tsLength of output: 863
| * Create Zone widget. | ||
| */ | ||
| const widgetId: string = generateReactKey(); | ||
| const widgetId: string = generateReactKey({ prefix: "zone-" }); |
There was a problem hiding this comment.
Do we need this for the rest of the widgets? 🤔 cc @riodeuno
There was a problem hiding this comment.
I'm talking about other types of widgets, inputs, switches, checkboxes, and so on.
znamenskii-ilia
force-pushed
the
chore/widget-id-prefix
branch
from
fb1cbe8 to
053f81f
Compare
KelvinOm
added
ok-to-test
This reverts commit 1668b04.
## Description This pull request fixes an issue where the error count was not being properly checked when a list widget was dropped. * Reverts a previous commit(#35221). * Include changes to the test file and the widget creation functions to ensure that the error count is correctly checked when a list widget is dragged and dropped. Fixes #35578 _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="@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/10350318730> > Commit: 07f3abc > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10350318730&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Mon, 12 Aug 2024 11:35:20 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Improved drag-and-drop functionality for list widgets. - **Bug Fixes** - Enhanced clarity and organization of test cases for better logical flow. - **Chores** - Simplified widget ID generation across several components, improving consistency in the application. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Description
Add widget type to id in order to simplify debugging. For example
3jgf4p6owp=>section-3jgf4p6owpAutomation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10158529399
Commit: 053f81f
Cypress dashboard.
Tags:
@tag.AllSpec:
Tue, 30 Jul 2024 08:58:49 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
widgetIdgeneration, allowing for better identification of section and zone widgets.widgetIdprefixes.Bug Fixes