Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
jfmath04
left a comment
There was a problem hiding this comment.
Yayy good work!! It looks great and the templates work nicely :) A couple things to review:
- Run Prettier and ESLint!!!!!
- Address the ESLint errors and warnings
- The page seems to jump down to the templates when loaded instead of being at the top of the page. It'd be good to investigate that and fix it.
- Create a folder under
components/MailFormto put the template files you made - Rather than comparing the whole editor text, keep a state to detect whether it has been edited after a template is loaded.
- Remove the types and create a map to store the email templates
components/MailForm/SendForm.tsx
Outdated
| const normalize = (s: string | undefined | null) => | ||
| (s || '') | ||
| .replace(/ /g, ' ') | ||
| .replace(/<[^>]*>/g, '') | ||
| .trim(); | ||
| const hasEditorContent = normalize(body) !== ''; | ||
| const wouldChangeBody = normalize(template.bodyTemplate) !== normalize(body); | ||
|
|
||
| if (hasEditorContent && wouldChangeBody) { | ||
| setPendingTemplate(type); | ||
| setShowReplaceConfirm(true); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Ideally, I think we should change this to have a state detecting whether the user has typed after selecting a template. This makes it so that if they are just browsing the templates without changing them, the confirmation doesn't pop up.
There was a problem hiding this comment.
Rather than creating a type, it might be easier to use a map so that in SendForm.tsx on line 74 we don't need to search the whole list for the correct template. We can just look up the template name and get the corresponding template.
Also, let's move this to a new folder under components/MailForm/Templates or whatever you'd like to call the folder
There was a problem hiding this comment.
Let's move this file to a new folder under components/MailForm/Templates or whatever you'd like to call the folder. Same with emailTemplates.ts.
References
Proposed Changes