Emailer: Load HTML templates from S3 #334
Conversation
Bumps [@testing-library/react](https://github.com/testing-library/react-testing-library) from 11.2.7 to 12.1.5. - [Release notes](https://github.com/testing-library/react-testing-library/releases) - [Changelog](https://github.com/testing-library/react-testing-library/blob/main/CHANGELOG.md) - [Commits](testing-library/react-testing-library@v11.2.7...v12.1.5) --- updated-dependencies: - dependency-name: "@testing-library/react" dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.15.2 to 1.15.4. - [Release notes](https://github.com/follow-redirects/follow-redirects/releases) - [Commits](follow-redirects/follow-redirects@v1.15.2...v1.15.4) --- updated-dependencies: - dependency-name: follow-redirects dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [@types/react-csv](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/react-csv) from 1.1.3 to 1.1.10. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/react-csv) --- updated-dependencies: - dependency-name: "@types/react-csv" dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
| emails, | ||
| sendEmailForm, | ||
| }) => { | ||
| const SendEmailForm: React.FC<SendEmailFormProps> = ({ emails }) => { |
There was a problem hiding this comment.
⛏️ Looks like you have some errors (from merging) - make sure to get rid of this extra declaration of the SendEmailForm component!
There was a problem hiding this comment.
Also make sure to remove line 50:
const [sendEmailForm] = Form.useForm();
since we're passing the form instance as a prop now (I can't comment on it since it's an unchanged line 😞)
| name="emailContent" | ||
| rows={6} | ||
| placeholder={'Email Body'} | ||
| /> | ||
| <Input.TextArea rows={8} placeholder={t('body_placeholder')} /> |
There was a problem hiding this comment.
⛏️ Can we also remove this extra TextArea? There's two text areas here:
| setTemplateNames(res.templates); | ||
| }) | ||
| .catch((err) => { | ||
| setTemplateNames([defaultTemplate]); |
There was a problem hiding this comment.
⛏️ Can we add an error message here in the catch (same for fetchTemplateData)? Something like the message.error() call in this function would do - if something fails on our end, it's better to explicitly tell the user that an error happened than to fail silently and have them guess at why something's not working
tested error message when loading empty html
|
There was a problem hiding this comment.
lgtm, nice work! A couple extra notes:
- You might need to just re-run the build checks to get them to pass
- When you merge, I suggest choosing the "squash and merge" option (see pic) from the dropdown - if you're not familiar with this, it'll basically combine all of your commits on this branch into a single commit that gets merged into master. This way the commit history doesn't get clogged up with your commits from while you were working
Checklist
yarn run checkyarn run testWhy
Resolves #259
This allows the user to choose the email template and apply it to the email they're sending. The user can make edits to the template.
This PR
We passed in a form instance to sendEmailForm/index.tsx from email/index.tsx.
Screenshots
Verification Steps
Choose the template from the select template dropdown and a template in html should appear in the email body.