-
Notifications
You must be signed in to change notification settings - Fork 0
feat(tax-servicing-countries) - create custom component #505
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
base: main
Are you sure you want to change the base?
Conversation
|
Deploy preview for remote-flows ready! ✅ Preview Built with commit 81c63cc. |
|
Deploy preview for remote-flows-example-app ready! ✅ Preview Built with commit 81c63cc. |
|
bugbot run |
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.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| selected={selectedOptions} | ||
| onChange={handleChange} | ||
| {...rest} | ||
| /> |
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.
Form state not updated when using default MultiSelect
High Severity
The default MultiSelect path passes handleChange directly as the onChange handler, but handleChange never calls field.onChange from react-hook-form. This means the form's internal state is never updated when selections change. The UI will visually show the correct selections via local state, but form submission will contain stale or initial values because the form field value was never synchronized.
| const customBasicInformationFields = useMemo(() => { | ||
| const enableCustomTaxServicingComponent = | ||
| taxServicingCountriesFieldPresentation?.enableCustomTaxServicingComponent !== | ||
| false; |
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.
Custom component enabled by default instead of opt-in
Medium Severity
The condition !== false causes enableCustomTaxServicingComponent to default to true when not specified, making the custom component opt-out rather than opt-in. The example code explicitly sets enableCustomTaxServicingComponent: true, which would be redundant if that were the default—suggesting opt-in behavior was intended. This means all users of the basic_information form get the custom TaxServicingCountriesField component even without requesting it, potentially causing unexpected behavior changes.
| selected || | ||
| enhancedOptions.filter((option) => | ||
| field.value?.includes(option.value), | ||
| ); |
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.
Local state ignores external form value changes
Medium Severity
The selected state is initialized as undefined and only falls back to field.value while undefined. After any user interaction, handleChange calls setSelected, setting it to an array. From then on, selectedOptions always uses the local selected state (since even [] is truthy), ignoring any external changes to field.value. This means form.reset() or programmatic value changes won't update the UI - users will see stale selections.


Note
Medium Risk
Changes onboarding form behavior and value mapping for a specific field, which could impact saved/validated payloads and selection UX, but does not touch auth or backend data processing.
Overview
Adds a new
TaxServicingCountriesFieldcomponent that enhances thetax_servicing_countriesmulti-select by supporting Global, region/subregion group selections (expanded into individual countries), and de-duplication of overlapping selections.Wires this component into onboarding’s
basic_informationschema viajsfModifyso it can be enabled/disabled throughpresentation.enableCustomTaxServicingComponent, and updates the example app to enable the new behavior by default.Written by Cursor Bugbot for commit 81c63cc. This will update automatically on new commits. Configure here.