chore: external merge request from Contributor #34958
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes introduce a custom switch field in the JSON Form Widget, allowing for new label positioning options and improved form layout customization. Additionally, the updates include new test scenarios to verify these functionalities and address bugs related to label positioning and text wrapping. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant JSONFormWidget
participant SwitchField
participant LabelPosition
User ->> JSONFormWidget: Add Switch Field
JSONFormWidget ->> SwitchField: Create Switch Field with Default Config
SwitchField ->> LabelPosition: Set label position
LabelPosition -->> SwitchField: Apply label position
SwitchField ->> JSONFormWidget: Render with updated label position
JSONFormWidget ->> User: Display updated Switch Field
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
github-actions
bot
added
the
skip-changelog
|
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9951869306. |
|
🔴 There's new test files in JS, please port to TS and re-trigger Cypress tests:
|
![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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_EditSwitchFied_spec.js (1 hunks)
- app/client/cypress/fixtures/jsonFormDslSwitchWidgetSourceData.json (1 hunks)
- app/client/cypress/locators/commonlocators.json (1 hunks)
- app/client/src/widgets/JSONFormWidget/component/Field.tsx (2 hunks)
- app/client/src/widgets/JSONFormWidget/component/FieldLabel.tsx (2 hunks)
- app/client/src/widgets/JSONFormWidget/fields/SwitchField.tsx (6 hunks)
- app/client/src/widgets/JSONFormWidget/widget/propertyConfig/properties/switch.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/widgets/JSONFormWidget/component/FieldLabel.tsx
Additional context used
Path-based instructions (3)
app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_EditSwitchFied_spec.js (1)
Pattern
app/client/cypress/**/**.*:app/client/cypress/fixtures/jsonFormDslSwitchWidgetSourceData.json (1)
Pattern
app/client/cypress/**/**.*:app/client/cypress/locators/commonlocators.json (1)
Pattern
app/client/cypress/**/**.*:
Additional comments not posted (10)
app/client/src/widgets/JSONFormWidget/widget/propertyConfig/properties/switch.ts (2)
5-5: New import added forLabelPosition.This import is necessary for the new
labelPositionproperty added to the switch configuration. It ensures that the property uses the correct enum values.
27-42: Addition of new property configuration forlabelPosition.This new property allows for better customization of the label's position relative to the switch. The use of
LabelPositionenum ensures consistency and reduces the likelihood of errors due to invalid values.app/client/cypress/e2e/Regression/ClientSide/Widgets/JSONForm/JSONForm_EditSwitchFied_spec.js (2)
1-7: New imports and modifications to support the test scenario.These changes are necessary to set up the environment and dependencies for the new test scenario. They ensure that the necessary modules and data are available for the tests.
9-63: Comprehensive test scenario for custom field functionality in JSON Form Widget.This test validates the addition and configuration of a custom switch field, focusing on the label position functionality. It effectively checks both "row" and "row-reverse" positions to ensure they are applied correctly.
app/client/src/widgets/JSONFormWidget/component/Field.tsx (2)
26-26: UpdatedStyledWrapperPropsinterface to include "row-reverse".This update is crucial for supporting the new "row-reverse" label position, allowing for more flexible layout configurations in the JSON Form Widget.
69-75: Logic to determine the direction of the field based onfieldClassNameandinlineLabel.This logic effectively handles the new "row-reverse" option by checking the
fieldClassNameandinlineLabelproperties. It ensures that the field's layout adapts based on the specified configuration.app/client/src/widgets/JSONFormWidget/fields/SwitchField.tsx (2)
Line range hint
14-26: Updated imports and type declarations to support newlabelPositionproperty.These updates are necessary to integrate the new
labelPositionproperty into theSwitchFieldcomponent, ensuring it can utilize the new label positioning options.
40-40: Updated default values to includelabelPosition.Setting the default value for
labelPositionensures that there is a consistent default behavior unless specified otherwise. This change is aligned with the new functionality introduced in the PR.app/client/cypress/fixtures/jsonFormDslSwitchWidgetSourceData.json (1)
1-172: JSON Structure Review: Valid and Aligns with FunctionalityThe JSON structure is valid and correctly defines properties and children for the JSON Form Widget. This aligns with the PR's description of introducing a new form layout with specific widget configurations for testing purposes.
app/client/cypress/locators/commonlocators.json (1)
237-246: New Locators Added: Check for Correctness and RelevanceThe new locators added (
jsonformFieldCustomFieldSwitchandjsonFieldPrefix) are correctly formatted and relevant to the new functionalities introduced in the PR. These locators are essential for the Cypress tests to interact with the new custom field options in the JSON Form Widget.
…label-position-has-no-effect-label-wraps-text-unnecessarily
…-has-no-effect-label-wraps-text-unnecessarily' of https://github.com/Harshithazemoso/appsmith into external-contri/Issue-33793-fix/Switch-Field-in-JSONForm-label-position-has-no-effect-label-wraps-text-unnecessarily
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9953851641. |
rahulbarwal
added
Widgets & Blocks Pod
ok-to-test
|
Deploy-Preview-URL: https://ce-34958.dp.appsmith.com |
|
Deploy-Preview-URL: https://ce-34958.dp.appsmith.com |
|
/ci-test-limit |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10215951638. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10215951638. |
|
/ci-test-limit runId=10215951638 |
4 similar comments
|
/ci-test-limit runId=10215951638 |
|
/ci-test-limit runId=10215951638 |
|
/ci-test-limit runId=10215951638 |
|
/ci-test-limit runId=10215951638 |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10216341910. |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10216342912. |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10216341694. |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10216344342. |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/10216344671. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10216341694. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10216344342. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10216344671. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10216341910. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10216342912. |
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
…label-position-has-no-effect-label-wraps-text-unnecessarily
…-has-no-effect-label-wraps-text-unnecessarily' of https://github.com/Harshithazemoso/appsmith into external-contri/Issue-33793-fix/Switch-Field-in-JSONForm-label-position-has-no-effect-label-wraps-text-unnecessarily
…o external-contri/Issue-33793-fix/Switch-Field-in-JSONForm-label-position-has-no-effect-label-wraps-text-unnecessarily
|
This PR has increased the number of cyclic dependencies by 2, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
…o external-contri/Issue-33793-fix/Switch-Field-in-JSONForm-label-position-has-no-effect-label-wraps-text-unnecessarily
|
This PR has increased the number of cyclic dependencies by 2, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. |
|
This PR has been closed because of inactivity. |
4 participants
Description
Fixes #34298
Original PR: #34298
EE PR: https://github.com/appsmithorg/appsmith-ee/pull/5232
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
Caution
If you modify the content in this section, you are likely to disrupt the CI result for your PR.
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Enhancements
Warning
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11047220758
Commit: 5be5319
Cypress dashboard.
Tags: @tag.All
Spec:
It seems like no tests ran 😔. We are not able to recognize it, please check workflow here.
Thu, 26 Sep 2024 07:10:32 UTC