upcoming: [UIE-9823] - Implement the Contact Sales Drawer for Marketplace products#13368
upcoming: [UIE-9823] - Implement the Contact Sales Drawer for Marketplace products#13368harsh-akamai wants to merge 9 commits intolinode:developfrom
Conversation
Cloud Manager UI test results🎉 866 passing tests on test run #8 ↗︎
|
|
The form initializes additional email to ['']. API Spec expects: ["email1@akamai.com"] or omit if empty |
|
One observation in api spec doc there is a typo API Spec shows: "parner_name": "Akamai" (typo - missing 't'). Please get this corrected from backend team. |
| ? getAPIErrorOrDefault(error)?.[0].reason | ||
| : "Oops! Something went wrong and we couldn't send your contacts. Please try again in a moment, or refresh the page."; | ||
| enqueueSnackbar(errorMessage, { variant: 'error' }); | ||
| onClose(); |
There was a problem hiding this comment.
On api error do we need to close the drawer?
| return ( | ||
| <Drawer | ||
| data-testid="contact-sales-drawer" | ||
| onClose={() => { |
There was a problem hiding this comment.
Instead of resetting the form onClose which can cause visual glitches if the form resets while the drawer is animating closed. we can use onTransitionExited={() => reset()}
| @@ -0,0 +1,590 @@ | |||
| import { yupResolver } from '@hookform/resolvers/yup'; | |||
There was a problem hiding this comment.
No unit tests for ContactSalesDrawer component
| } from '@linode/ui'; | ||
| import { createPartnerReferralSchema } from '@linode/validation'; | ||
| import { createFilterOptions, Grid } from '@mui/material'; | ||
| import { enqueueSnackbar } from 'notistack'; |
There was a problem hiding this comment.
mostly this pattern is follwed in the codebase import { useSnackbar } from 'notistack';
const { enqueueSnackbar } = useSnackbar(); . Direct import works but is inconsistent with codebase conventions.
| render={({ field }) => { | ||
| const emailErrors = errors.additional_emails; | ||
| return ( | ||
| <MultipleIPInput |
There was a problem hiding this comment.
Using MultipleIPInput for email addresses works but is semantically confusing. looked like the component was designed for IP addresses. Consider:
Adding a comment explaining this reuse
Or see if there is possibility of MultipleEmailInput wrapper
| <MultipleIPInput | ||
| buttonText="Add email address" | ||
| className={ | ||
| field.value?.length === 2 |
There was a problem hiding this comment.
can we use a constant here : MAX_ADDITIONAL_EMAILS = 2
|
|
||
| const { | ||
| control, | ||
| formState: { errors, isSubmitting }, |
There was a problem hiding this comment.
When the form is submitted While isSubmitting is used to disable form fields. Is this is correct submitting state. Can we check with ux on this.
| const { classes } = useStyles(); | ||
| const { onClose, open, partnerName, productName } = props; | ||
|
|
||
| const { data: profile } = useProfile(); |
There was a problem hiding this comment.
Are we sure this data is not asynchronous and will always be available when drawer is opened? Basically do you this we should handle async nature of this data?
| @@ -0,0 +1,590 @@ | |||
| import { yupResolver } from '@hookform/resolvers/yup'; | |||
There was a problem hiding this comment.
Missing Notice for in-form api returned errors
3 participants
Description 📝
Implement the Contact Sales Drawer for Marketplace products
Scope 🚢
Upon production release, changes in this PR will be visible to:
Preview 📷
How to test 🧪
Prerequisites
Verification steps
Author Checklists
As an Author, to speed up the review process, I considered 🤔
👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support
As an Author, before moving this PR from Draft to Open, I confirmed ✅