fix(provider): Remove sender name field and make boolean type a switch in nodemailer #3365
Conversation
| @@ -181,7 +181,7 @@ export const nodemailerConfig: IConfigCredentials[] = [ | |||
| type: 'string', | |||
| required: false, | |||
| }, | |||
| ...mailConfigBase, | |||
| mailConfigBase[0], | |||
There was a problem hiding this comment.
I consider we shouldn't remove the option to set the sender name from Nodemailer. If it is not used in the provider package we should implement it. Probably the change for the credentials would be to make the sender name optional instead of mandatory as it is right now.
There was a problem hiding this comment.
alright, I'll add it to the config in the provider file
| if (credential.type === 'boolean') { | ||
| return ( | ||
| <Switch | ||
| styles={() => ({ | ||
| root: { | ||
| display: 'block !important', | ||
| maxWidth: '100% !important', | ||
| }, | ||
| })} | ||
| label={credential.displayName} | ||
| required={credential.required} | ||
| placeholder={credential.displayName} | ||
| description={credential.description ?? ''} | ||
| data-test-id={credential.key} | ||
| error={errors[credential.key]?.message} | ||
| {...register(credential.key)} | ||
| checked={field.value} | ||
| onChange={field.onChange} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
This change will make all boolean values for all integrations to become a Switch. We should review all the providers to see nothing breaks in the functionality.
Adding screenshots of how they would look like now would be also helpful for the PR reviewers.
There was a problem hiding this comment.
yes, although, it makes sense for a boolean to be a toggle rather than an input field. I know I have been confused by that when creating documentation for (sparkpost).
The only providers affected are Sparkpost and Nodemailer (I checked). The others have a type of switch, which frankly I don't understand why it is so, boolean sounds like a better option since it's only true or false. (Note that this switch type is only included in the nodemailer config as well so it's the same behaviour as the boolean.
|
@peoray could you do a |
…u into nodemailer-integration-fields
|
@p-fernandez done |
What change does this PR introduce?
Why was this change needed?
Closes #1753
Other information (Screenshots)