feat: group identifier for dropdown form controls #39256
Conversation
WalkthroughThe pull request updates the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant D as DropDownControl
participant R as Redux Store
U->>D: Select option
D->>D: Check if appendGroupIdentifierToValue is true
D->>D: Append group identifier to the option value
D->>R: Dispatch Redux action with modified payload
R-->>D: Update store state
D-->>U: Update displayed selection
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
🔇 Additional comments (5)
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
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13312354447. |
|
Deploy-Preview-URL: https://ce-39256.dp.appsmith.com |
When trying to do multi select on the dropdown with the append option enabled, the check for group type identifier was being appended incorrectly. Fixed the same
We are not getting store updates, but the store actions launched are realiable and hence used in the test
github-actions
bot
added
the
Frontend
ayushpahwa
requested review from
jsartisan and
sneha122
and removed request for
jsartisan
github-actions
bot
added
Workflows Pod
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/components/formControls/DropDownControl.test.tsx(3 hunks)app/client/src/components/formControls/DropDownControl.tsx(8 hunks)
🔇 Additional comments (7)
app/client/src/components/formControls/DropDownControl.test.tsx (4)
9-9: LGTM! Test setup changes look good.The changes improve type safety and properly initialize the form state for testing the new group identifier feature.
Also applies to: 225-233
303-369: LGTM! Comprehensive test coverage for group identifier feature.The test case thoroughly verifies:
- Group identifier appending for grouped options
- Handling of ungrouped options (others group)
- Correct Redux action payloads
371-444: LGTM! Multi-select test coverage is thorough.The test case effectively verifies:
- Array handling with group identifiers
- Selection and removal of multiple options
- Correct Redux action payloads for array values
446-515: LGTM! Edge cases are well covered.The test case effectively handles:
- Values containing colons
- Options without group assignments
- Correct Redux action payloads for edge cases
app/client/src/components/formControls/DropDownControl.tsx (3)
294-304: LGTM! Selected options filtering logic is robust.The implementation correctly:
- Handles both grouped and ungrouped options
- Performs appropriate value comparisons based on configuration
355-368: LGTM! Option selection logic handles group identifiers correctly.The implementation properly:
- Appends group identifiers when configured
- Handles ungrouped options with "others" group
- Maintains existing functionality when feature is disabled
402-412: LGTM! Option removal logic handles group identifiers correctly.The implementation effectively:
- Handles group identifiers during removal
- Correctly compares values by splitting on colon
- Maintains array integrity
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/client/src/components/formControls/DropDownControl.tsx (2)
294-308: Optimize the filtering logic.The filtering logic can be simplified using array methods and early returns.
- const selectedOptions = options.filter((opt) => { - const checkGroupIdentifier = - appendGroupIdentfierToValue && optionGroupConfig; - const valueToCompare = checkGroupIdentifier - ? opt.optionGroupType + ":" + opt.value - : opt.value; - - return isMultiSelect - ? (selectedValue as string[]).includes(valueToCompare) - : selectedValue === valueToCompare; - }); + const checkGroupIdentifier = appendGroupIdentfierToValue && optionGroupConfig; + const getValueToCompare = (opt: SelectOptionProps) => + checkGroupIdentifier ? `${opt.optionGroupType || "others"}:${opt.value}` : opt.value; + const selectedOptions = options.filter((opt) => + isMultiSelect + ? (selectedValue as string[]).includes(getValueToCompare(opt)) + : selectedValue === getValueToCompare(opt) + );
411-421: Simplify the removal logic.The removal logic can be simplified by extracting the group identifier parsing into a helper function.
+ function parseGroupedValue(value: string): string { + return appendGroupIdentfierToValue && optionGroupConfig ? value.split(":")[1] : value; + } + const filtered = currentArray.filter((v) => { - let selectedValueToCheck = v; - - if (appendGroupIdentfierToValue && optionGroupConfig) { - selectedValueToCheck = v.split(":")[1]; - } - - return selectedValueToCheck !== optionValueToRemove; + return parseGroupedValue(v) !== optionValueToRemove; });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/components/formControls/DropDownControl.tsx(9 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
🔇 Additional comments (2)
app/client/src/components/formControls/DropDownControl.tsx (2)
120-120: Fix typo in property name.The property name has a typo: "Identfier" should be "Identifier".
458-458: LGTM: Clear button visibility logic is correct.The clear button visibility logic correctly handles both multi-select and single-select cases.
Description
This PR enhances the DropDownControl component to support storing group information along with selected values. When enabled, this feature allows consumers to track which group an option belongs to in the stored value, making it easier to handle cases where different groups might have options with identical values.
Key Changes
appendGroupIdentfierToValueto control the featuregroupType:valuewhen enabledValue Format
When
appendGroupIdentfierToValueis enabled:"groupType:optionValue"["groupType1:optionValue1", "groupType2:optionValue2", ...]Example Usage
Notes
appendGroupIdentfierToValuepropoptionGroupConfigis not provided, behavior remains unchangedappendGroupIdentfierToValueis true ANDoptionGroupConfigis providedFixes #39259
Automation
/ok-to-test tags="@tag.Sanity, @tag.IDE"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13315296499
Commit: 7a0184b
Cypress dashboard.
Tags:
@tag.Sanity, @tag.IDESpec:
Thu, 13 Feb 2025 20:07:54 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Tests