chore: Test external contribution PR #35173
Conversation
…t-to-show-error-message-#4653' into external-contri/fix/radio-button-widget-to-show-error-message-#4653
github-actions
bot
added
Widgets Product
|
Important Review skippedAuto reviews are limited to specific labels. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe recent changes enhance the user experience in the Changes
Assessment against linked issues
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
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: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
app/client/src/assets/icons/alert/warning-error.svgis excluded by!**/*.svg
Files selected for processing (2)
- app/client/src/components/propertyControls/KeyValueComponent.tsx (9 hunks)
- app/client/src/widgets/RadioGroupWidget/widget/index.tsx (1 hunks)
Additional comments not posted (12)
app/client/src/components/propertyControls/KeyValueComponent.tsx (11)
9-9: Good job adding the necessary import!The import statement for
WarningErrorIconis correctly added and will be useful for displaying error icons in the validation logic.
42-46: Nice use of FlexBox for layout!The
FlexBoxstyled component is well-defined and will help in organizing the input fields and error messages.
48-57: Great job styling the error messages!The
ErrorMessageBoxstyled component is well-defined and will help in displaying error messages clearly.
63-81: Excellent use of conditional styling!The
StyledInputGroupstyled component is well-defined and the conditional error styling is correctly implemented.
103-103: Good use of state to manage error messages!The
errorMessagesstate variable is correctly added and will help in tracking validation errors for each pair of input fields.
159-166: Great job on the validation logic!The
validatePairsfunction is well-implemented and correctly generates error messages for empty labels and values.
141-141: Good job integrating validation intoupdateKey!The
updateKeyfunction now correctly callsvalidatePairsafter updating the key, ensuring validation is performed.
156-157: Good job integrating validation intoupdateValue!The
updateValuefunction now correctly callsvalidatePairsafter updating the value, ensuring validation is performed.
177-177: Good job integrating validation intodeletePair!The
deletePairfunction now correctly callsvalidatePairsafter deleting a pair, ensuring validation is performed.
211-211: Good job integrating validation intoaddPair!The
addPairfunction now correctly callsvalidatePairsafter adding a new pair, ensuring validation is performed.
226-271: Excellent job on the error message display!The render logic correctly displays error messages below each input field, providing clear feedback to the user.
app/client/src/widgets/RadioGroupWidget/widget/index.tsx (1)
73-81: Great job enhancing the validation logic!The new validation condition in the
optionsCustomValidationfunction correctly checks if bothlabelandvalueare empty and sets the_isValidflag tofalseif true.
| function validatePairs(pairs: SegmentedControlOptionWithKey[]) { | ||
| const newErrorMessages = pairs.map((pair) => { | ||
| if (!pair.label && !pair.value) { | ||
| return "Both Name and Value can't be empty"; | ||
| } | ||
| return ""; | ||
| }); | ||
| setErrorMessages(newErrorMessages); |
There was a problem hiding this comment.
Consider adding unit tests for validation logic.
To ensure the robustness of the validatePairs function, consider adding unit tests to cover various scenarios.
Do you want me to help you write the unit tests for this function?
| if (!label && !value) { | ||
| _isValid = false; | ||
| message = { | ||
| name: "ValidationError", | ||
| message: | ||
| "Both Name and Value can't be empty", | ||
| }; | ||
| break; | ||
| } |
There was a problem hiding this comment.
Consider adding unit tests for the new validation condition.
To ensure the robustness of the optionsCustomValidation function, consider adding unit tests to cover various scenarios, including the new validation condition.
Do you want me to help you write the unit tests for this function?
…o external-contri/fix/radio-button-widget-to-show-error-message-#4653
riodeuno
added
ok-to-test
|
Hello @riodeuno , Will look into this failed cypress spec and update the PR. |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
|
This PR has been closed because of inactivity. |
riodeuno
deleted the
external-contri/fix/radio-button-widget-to-show-error-message-#4653
branch
2 participants
Description
Fixes #4653
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Caution
🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10112601666
Commit: 251e6b7
Cypress dashboard.
Tags: @tag.All
Spec:
The following are new failures, please fix them before merging the PR:
- cypress/e2e/Regression/ClientSide/IDE/Canvas_Context_Bug_Fixes.js
List of identified flaky tests.Fri, 26 Jul 2024 15:01:45 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes