Feature/EPMGCIP-168/implement contacts form #29
Conversation
usavkov-epam
requested review from
gordoney,
DmitryLukyanov and
Dzmitry-Yaniuk
| const [captchaToken, setCaptchaToken] = useState<string | null>(null); | ||
| const [isCaptchaLoaded, setIsCaptchaLoaded] = useState(false); | ||
| const [isSubmitting, setIsSubmitting] = useState(false); | ||
| const reCaptchaRef: any = useRef(); |
There was a problem hiding this comment.
Is it possible to avoid 'any'?
postcss.config.cjs
Outdated
| 'postcss-preset-mantine': {}, | ||
| 'postcss-simple-vars': { | ||
| variables: { | ||
| 'mantine-breakpoint-xs': '36em', |
There was a problem hiding this comment.
We have some breakpoints in src\variables.scss. So, we have to make a single source of truth for them. I guess, we can:
- at least remove breakpoints in src\variables.scss and update a few related .scss files
- OR, honestly, I am a not big fan of the idea to be to so tighly connected to mantine's provider, so maybe you know other options
There was a problem hiding this comment.
Updated, use SCSS as a source of truth
| } | ||
|
|
||
| .reCaptcha { | ||
| min-height: 5.25rem; |
There was a problem hiding this comment.
Could you convert 'gap' and 'min-height' to 'px'?
src/hooks/useShowNotification.ts
Outdated
| import { MessageKeys, NestedKeyOf, useTranslations } from "next-intl"; | ||
| import { useCallback } from "react"; | ||
|
|
||
| type NotificationType = "success" | "error"; |
There was a problem hiding this comment.
What do you think about creating global enum and using it for calling 'showNotification'? I noticed that you pass 'type' as string, maybe enum will be more appropriate.
There was a problem hiding this comment.
Updated. Types extracted to enum
src/actions/sendContactForm.ts
Outdated
| token: string; | ||
| } | ||
|
|
||
| export async function sendContactForm({ name, email, subject, message, token }: SendEmailProps) { |
There was a problem hiding this comment.
I suppose, we need runtime validation here. Maybe Zod or something like that. In addition, we can reuse validation schema on frontend
There was a problem hiding this comment.
Updated. Used zod for validation on UI and BE via sharing the schema
| @@ -0,0 +1 @@ | |||
| export { default } from "./ContactForm"; | |||
There was a problem hiding this comment.
If you don't mind, could you remove barrel 'index' files?
| label={t("contactForm.fields.name.label")} | ||
| placeholder={t("contactForm.fields.name.placeholder")} | ||
| key={form.key("name")} | ||
| {...form.getInputProps("name")} |
There was a problem hiding this comment.
Just to confirm: is it safe to put spread operator for form.getInputProps("name")? Won't it accidentally override some props you declared earlier? Perhaps for safety it is better to put it in the beginning
There was a problem hiding this comment.
All the props, specified above ...form.get... will be safely passed to components. I don't think it should be in the beginning, since devs are who can accidentally erase the necessary props for the form :)
src/variables.scss
Outdated
| md: 992px, | ||
| lg: 1200px, | ||
| xl: 1400px, | ||
| tablet: 834px, |
There was a problem hiding this comment.
I propose to remove 'tablet' and use 'sm' instead of that. I even don't remember why we chose 834px for tablets
src/components/forms/index.tsx
Outdated
| @@ -1 +1 @@ | |||
| export { default as ContactForm } from "./ContactForm"; | |||
| export { default as ContactForm } from "./ContactForm/ContactForm"; | |||
There was a problem hiding this comment.
What do you think about removing this index.tsx as well?
There was a problem hiding this comment.
I think using index files simplifies imports, hides folder structure details, and centralizes exports, making the codebase cleaner, easier to manage, and more scalable.
Purpose
JIRA
UI Form: https://jira.epam.com/jira/browse/EPMGCIP-168
Server action: https://jira.epam.com/jira/browse/EPMGCIP-174
Approach
Utilize the Mantine library as the UI kit to create the form component. Additionally, leverage Mantine's plugin for implementing a UI notification system. For form handling, adopt Next.js's approach using Server Actions. Use the
nodemailerlibrary to handle email sending. Include a CAPTCHA to verify that the user is human.Screenshot
EPM-PR.mp4