Fix gift card customer select#6513
Conversation
🦋 Changeset detectedLatest commit: d801c97 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6513 +/- ##
==========================================
- Coverage 47.94% 47.93% -0.02%
==========================================
Files 2579 2579
Lines 45736 45748 +12
Branches 10787 10796 +9
==========================================
Hits 21928 21928
- Misses 22560 22571 +11
- Partials 1248 1249 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Fixes behavior of the gift card “Customer” combobox so the selected customer is cleared when the user edits the input after making a selection, and adds unit tests around the customer/email selection behavior.
Changes:
- Clear
selectedCustomerwhen the combobox input changes away from the selected value. - Add RTL tests covering rendering, “Use email” option, selection, and post-selection editing behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/giftCards/GiftCardCreateDialog/GiftCardCustomerSelectField.tsx | Adds logic to clear the selected customer when the input value changes. |
| src/giftCards/GiftCardCreateDialog/GiftCardCustomerSelectField.test.tsx | Adds component-level tests and mocks for search/debounce behavior. |
| onInputValueChange={val => { | ||
| setInputValue(val); | ||
| debouncedSearch(val); | ||
|
|
||
| if (selectedCustomer.email && val !== selectedCustomer.email) { | ||
| setSelectedCustomer({ email: "", name: "" }); | ||
| } |
There was a problem hiding this comment.
The new clearing condition compares the combobox input value only to selectedCustomer.email. DynamicCombobox typically updates the input to the selected option’s label (e.g. full name or "Use email: …"), so after selecting a customer the next onInputValueChange may see val === selectedCustomer.name but val !== selectedCustomer.email and immediately clear the selection again (potentially even on initial render when a value is set). Consider treating both the selected email and the selected label as valid input values (or otherwise distinguishing user typing from selection updates) before clearing selectedCustomer.
Fixes `pnpm install` command failing due to not having PNPM installed Fix behavior of gift card customer selection
lkostrowski
force-pushed
the
lkostrowski/fix-email-dropdown-update
branch
from
c7cfaef to
9394eb1
Compare
| }); | ||
| }} | ||
| > | ||
| Use custom email: {customEmail} |
There was a problem hiding this comment.
The button text "Use custom email: …" is a new user-facing string but it isn’t localized. Please use react-intl (e.g. reuse messages.useEmail or add a new message descriptor) so this label is translated consistently with the rest of the gift card create dialog.
| Use custom email: {customEmail} | |
| {intl.formatMessage( | |
| { | |
| defaultMessage: "Use custom email: {email}", | |
| id: "I4TjC5", | |
| }, | |
| { email: customEmail }, | |
| )} |
| {customEmail && ( | ||
| <Box> | ||
| <Button | ||
| display="inline-block" | ||
| variant="tertiary" | ||
| onClick={() => { | ||
| setInputValue(customEmail); | ||
| setSelectedCustomer({ | ||
| email: customEmail, | ||
| name: customEmail, | ||
| }); | ||
| }} | ||
| > | ||
| Use custom email: {customEmail} | ||
| </Button> | ||
| </Box> | ||
| )} |
There was a problem hiding this comment.
This change introduces new UI state/behavior (showing a separate “custom email” action and selecting it). Consider adding a component-level test that covers: (1) button appears only for a valid, non-matching email; (2) clicking it updates selectedCustomer and hides the button when it matches the selection.
…leor/saleor-dashboard into lkostrowski/fix-email-dropdown-update
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| {customEmail && ( | ||
| <Box> | ||
| <Button | ||
| display="inline-block" | ||
| variant="tertiary" | ||
| onClick={() => { | ||
| setInputValue(customEmail); | ||
| setSelectedCustomer({ | ||
| email: customEmail, | ||
| name: customEmail, | ||
| }); | ||
| }} |
There was a problem hiding this comment.
The combobox respects the disabled prop, but the new "Use email" button remains clickable and can still mutate selectedCustomer while the field is disabled. Pass disabled={disabled} to the Button (and/or hide the button when disabled) to keep the disabled state consistent.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| {customEmail && ( | ||
| <Box> | ||
| <Button | ||
| display="inline-block" | ||
| variant="tertiary" | ||
| onClick={() => { | ||
| setInputValue(customEmail); | ||
| setSelectedCustomer({ | ||
| email: customEmail, | ||
| name: customEmail, | ||
| }); | ||
| }} | ||
| > | ||
| {intl.formatMessage(messages.useEmail)} {customEmail} | ||
| </Button> | ||
| </Box> |
There was a problem hiding this comment.
The disabled prop disables the combobox, but the custom email Button remains clickable. If this field can become disabled after a user typed a value (or if it renders with customEmail already computed), clicking the button would still change selectedCustomer while the UI is supposed to be read-only. Pass disabled={disabled} to the Button (and/or avoid rendering the custom-email action when disabled is true).
Scope of the change