-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
fix: wds widgets #34387
fix: wds widgets #34387
Conversation
WalkthroughRecent updates focus on refining UI components, enhancing user guidance, and simplifying code related to button variants, validation processes, and event handling within various widgets. Notable changes include renaming properties, removing redundant validations and events, and refining placeholders to offer clearer instructions. Changes
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 Configration File (
|
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Commits
Files that changed from the base of the PR and between 6ef4d2a and e32296ecb2940939fef68d8720500ef2b4a8d326.
Files selected for processing (9)
- app/client/src/widgets/wds/WDSStatsWidget/component/index.tsx (2 hunks)
- app/client/src/widgets/wds/WDSStatsWidget/config/propertyPaneConfig/contentConfig.ts (1 hunks)
- app/client/src/widgets/wds/WDSStatsWidget/widget/index.tsx (2 hunks)
- app/client/src/widgets/wds/WDSSwitchGroupWidget/config/autocompleteConfig.ts (1 hunks)
- app/client/src/widgets/wds/WDSSwitchGroupWidget/config/defaultsConfig.ts (1 hunks)
- app/client/src/widgets/wds/WDSSwitchGroupWidget/config/propertyPaneConfig/contentConfig.ts (1 hunks)
- app/client/src/widgets/wds/WDSSwitchGroupWidget/config/settersConfig.ts (1 hunks)
- app/client/src/widgets/wds/WDSSwitchGroupWidget/widget/index.tsx (4 hunks)
- app/client/src/widgets/wds/WDSSwitchGroupWidget/widget/types.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- app/client/src/widgets/wds/WDSStatsWidget/config/propertyPaneConfig/contentConfig.ts
- app/client/src/widgets/wds/WDSSwitchGroupWidget/config/settersConfig.ts
Additional comments not posted (7)
app/client/src/widgets/wds/WDSSwitchGroupWidget/config/autocompleteConfig.ts (1)
Line range hint
4-10
: Approved removal of validation properties.The changes in the
autocompleteConfig
align with the PR's objectives to simplify the widget by removing unnecessary validation properties.app/client/src/widgets/wds/WDSSwitchGroupWidget/widget/types.ts (1)
Line range hint
5-15
: Approved removal of unnecessary properties from interface.The changes in the
SwitchGroupWidgetProps
interface correctly reflect the simplification of the widget's capabilities by removing unnecessary validation properties.app/client/src/widgets/wds/WDSSwitchGroupWidget/config/defaultsConfig.ts (1)
Line range hint
5-19
: Approved simplification of default configurations.The changes in the
defaultsConfig
reflect the PR's objectives to simplify the widget by removing unnecessary default properties.app/client/src/widgets/wds/WDSStatsWidget/component/index.tsx (1)
Line range hint
5-37
: Approved removal of onClick event handler.The removal of the
onClick
event handler from theStatsComponent
aligns with the PR's objectives and resolves the issue where the event could be triggered without UI indication.app/client/src/widgets/wds/WDSStatsWidget/widget/index.tsx (1)
59-59
: Approved: Removal of onClick prop from StatsComponentThe removal of the onClick prop aligns with the PR's objective to address the issue where a click event was triggerable without a visible UI indication. Ensure that this change does not inadvertently affect other functionalities or events associated with the
StatsComponent
.#!/bin/bash # Description: Verify that removing onClick does not affect other functionalities. # Test: Search for any remaining references to `onClick` in the widget. rg --type typescript $'onClick' 'app/client/src/widgets/wds/WDSStatsWidget'app/client/src/widgets/wds/WDSSwitchGroupWidget/widget/index.tsx (1)
Line range hint
1-1
: Check for consistency with configuration changesEnsure that the
WDSSwitchGroupWidget
class is consistent with the removals made in the configuration files such asautocompleteConfig
andpropertyPaneContentConfig
. Specifically, verify that removed properties likeisValid
andisRequired
are not referenced.#!/bin/bash # Description: Verify that removed properties are not referenced in the widget. # Test: Search for the removed properties in the widget. rg --type typescript $'isValid|isRequired' 'app/client/src/widgets/wds/WDSSwitchGroupWidget'app/client/src/widgets/wds/WDSSwitchGroupWidget/config/propertyPaneConfig/contentConfig.ts (1)
Line range hint
1-1
: Approved: Removal of the "Validations" section from property pane configurationThe changes in the property pane configuration are consistent with the PR's objectives to simplify the widget by removing unnecessary validations. Ensure that the removal does not affect the widget's functionality in unexpected ways.
#!/bin/bash # Description: Verify that the "Validations" section is completely removed. # Test: Search for any remnants of the "Validations" section in the configuration. rg --type typescript $'Validations' 'app/client/src/widgets/wds/WDSSwitchGroupWidget/config'
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: 1
Outside diff range and nitpick comments (1)
app/client/src/components/propertyControls/ButtonListControl.tsx (1)
Line range hint
139-139
: Refactor: Simplify visibility toggle expression.The use of a ternary operator here is unnecessary. You can directly assign the negated value of
isVisible
to simplify the code.- const isVisible = menuItems[index].isVisible === true ? false : true; + const isVisible = !menuItems[index].isVisible;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Commits
Files that changed from the base of the PR and between e32296ecb2940939fef68d8720500ef2b4a8d326 and 417b79a3f056eb9e637ce6ccac9e4fea76c77da2.
Files selected for processing (1)
- app/client/src/components/propertyControls/ButtonListControl.tsx (1 hunks)
Additional context used
Biome
app/client/src/components/propertyControls/ButtonListControl.tsx
[error] 139-139: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Commits
Files that changed from the base of the PR and between 417b79a3f056eb9e637ce6ccac9e4fea76c77da2 and d6f30da1c1f12d99e97c28d22117bc41f3030cee.
Files selected for processing (1)
- app/client/src/widgets/wds/WDSParagraphWidget/config/defaultsConfig.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/widgets/wds/WDSParagraphWidget/config/defaultsConfig.ts
9ce20cb
to
15420c1
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.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
app/client/src/widgets/wds/WDSStatsWidget/widget/index.tsx (1)
Line range hint
14-16
: Remove unnecessary constructor.The constructor in this class is redundant as it only calls
super(props)
without any additional logic. Removing it simplifies the class.- constructor(props: StatsWidgetProps) { - super(props); - }app/client/src/components/propertyControls/ButtonListControl.tsx (1)
Line range hint
139-139
: Simplify conditional expression.The use of a ternary operator here is unnecessary. Directly use the logical NOT operator for clarity and conciseness.
- const isVisible = menuItems[index].isVisible === true ? false : true; + const isVisible = !menuItems[index].isVisible;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Commits
Files that changed from the base of the PR and between d6f30da1c1f12d99e97c28d22117bc41f3030cee and 15420c1.
Files selected for processing (12)
- app/client/src/components/propertyControls/ButtonListControl.tsx (1 hunks)
- app/client/src/widgets/wds/WDSBaseInputWidget/config/contentConfig.ts (3 hunks)
- app/client/src/widgets/wds/WDSParagraphWidget/config/defaultsConfig.ts (1 hunks)
- app/client/src/widgets/wds/WDSStatsWidget/component/index.tsx (2 hunks)
- app/client/src/widgets/wds/WDSStatsWidget/config/propertyPaneConfig/contentConfig.ts (1 hunks)
- app/client/src/widgets/wds/WDSStatsWidget/widget/index.tsx (2 hunks)
- app/client/src/widgets/wds/WDSSwitchGroupWidget/config/autocompleteConfig.ts (1 hunks)
- app/client/src/widgets/wds/WDSSwitchGroupWidget/config/defaultsConfig.ts (1 hunks)
- app/client/src/widgets/wds/WDSSwitchGroupWidget/config/propertyPaneConfig/contentConfig.ts (1 hunks)
- app/client/src/widgets/wds/WDSSwitchGroupWidget/config/settersConfig.ts (1 hunks)
- app/client/src/widgets/wds/WDSSwitchGroupWidget/widget/index.tsx (4 hunks)
- app/client/src/widgets/wds/WDSSwitchGroupWidget/widget/types.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/widgets/wds/WDSBaseInputWidget/config/contentConfig.ts
Files skipped from review as they are similar to previous changes (9)
- app/client/src/widgets/wds/WDSParagraphWidget/config/defaultsConfig.ts
- app/client/src/widgets/wds/WDSStatsWidget/component/index.tsx
- app/client/src/widgets/wds/WDSStatsWidget/config/propertyPaneConfig/contentConfig.ts
- app/client/src/widgets/wds/WDSSwitchGroupWidget/config/autocompleteConfig.ts
- app/client/src/widgets/wds/WDSSwitchGroupWidget/config/defaultsConfig.ts
- app/client/src/widgets/wds/WDSSwitchGroupWidget/config/propertyPaneConfig/contentConfig.ts
- app/client/src/widgets/wds/WDSSwitchGroupWidget/config/settersConfig.ts
- app/client/src/widgets/wds/WDSSwitchGroupWidget/widget/index.tsx
- app/client/src/widgets/wds/WDSSwitchGroupWidget/widget/types.ts
Additional context used
Biome
app/client/src/widgets/wds/WDSStatsWidget/widget/index.tsx
[error] 14-16: This constructor is unnecessary. (lint/complexity/noUselessConstructor)
Unsafe fix: Remove the unnecessary constructor.
app/client/src/components/propertyControls/ButtonListControl.tsx
[error] 139-139: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
Additional comments not posted (1)
app/client/src/widgets/wds/WDSStatsWidget/widget/index.tsx (1)
59-59
: Verify integration ofStatsComponent
after prop removal.Ensure that the removal of the
onClick
prop fromStatsComponent
does not affect other functionalities of the widget. It is important to test this thoroughly, especially in user interactions that might have depended on this prop.
@@ -195,7 +195,7 @@ class ButtonListControl extends BaseControl< | |||
itemType: isSeparator ? "SEPARATOR" : "BUTTON", | |||
isSeparator, | |||
isVisible: true, | |||
buttonVariant: "filled", | |||
variant: "filled", |
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.
Refactor: Optimize default property assignment.
The current implementation sets the variant
property to "filled"
directly. Consider using a default prop or a configuration object to make this more flexible and maintainable.
- variant: "filled",
+ variant: this.defaultProps.variant || "filled",
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
variant: "filled", | |
variant: this.defaultProps.variant || "filled", |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9642818636. |
Deploy-Preview-URL: https://ce-34387.dp.appsmith.com |
## Description - Remove click event from Stats - Remove validation section from SwitchGroup - Fix Inline button variant for added button - Fix paragraph default text - Fix inputs placeholders Fixes: appsmithorg#34189 appsmithorg#34002 appsmithorg#33753 appsmithorg#33242 appsmithorg#33243 appsmithorg#33186 appsmithorg#32509 ## Automation /ok-to-test tags="@tag.Anvil" ### 🔍 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/9615117095> > Commit: 15420c1 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9615117095&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Anvil` <!-- 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** - Updated the placeholder text for input fields to provide more concise and specific guidance. - **Bug Fixes** - Adjusted button variant naming for better consistency. - Improved text in `WDSParagraphWidget` to emphasize understanding mysteries. - **Refactor** - Removed unnecessary `onClick` prop from `StatsComponent`. - Simplified `getWidgetView` function in `WDSStatsWidget`. - **Chores** - Cleaned up redundant validation and required properties in `WDSSwitchGroupWidget`. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Fixes:
#34189
#34002
#33753
#33242
#33243
#33186
#32509
Automation
/ok-to-test tags="@tag.Anvil"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9615117095
Commit: 15420c1
Cypress dashboard.
Tags:
@tag.Anvil
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes
WDSParagraphWidget
to emphasize understanding mysteries.Refactor
onClick
prop fromStatsComponent
.getWidgetView
function inWDSStatsWidget
.Chores
WDSSwitchGroupWidget
.