Skip to content

Conversation

aarona
Copy link
Contributor

@aarona aarona commented Nov 25, 2019

Because I pass the email address being confirmed to the response message like so:

message: I18n.t('graphql_devise.confirmations.send_instructions', email: email)

I thought that it should be documented for the user in case they wanted to customize the locale to support it. This is supported by Devise even though our locale setting doesn't. The user would have to override the entire mutation to put the email variable in there otherwise which is why I left it in.

I decided to add this documentation as a subset in the Customizing Email Templates section because I felt that was the best place to put it.

Copy link
Member

@mcelicalderon mcelicalderon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure we should add this to the readme. If we do add something related to locales, I think it should be a more generic section. Check Devise's I18n section. Something that references a file that contains always up to date translations (the reference a wiki, but pointing to our config/locales file will be easier to maintain). If you want to write down something similar or just like Devise in this PR, I think we could add it.

@aarona
Copy link
Contributor Author

aarona commented Nov 27, 2019

Ok, so you think I should move this to an I18n section and put an example like the ones used in the link you posted to show that the user could have something like:

en:
  graphql_devise:
    confirmations:
      send_instructions: "Please check '#{email}' for instructions on how to confirm your account"

Something like that?

@mcelicalderon
Copy link
Member

Yes, something more generic like Devise's, not only for the resend confirmation email. More importantly, provide a link to our file on the repo where they could see ALL the locales they can override

@aarona
Copy link
Contributor Author

aarona commented Nov 28, 2019

Ok, I'll look into this, this weekend and update the PR.

@mcelicalderon mcelicalderon added the documentation Improvements or additions to documentation label Dec 2, 2019
@aarona
Copy link
Contributor Author

aarona commented Jan 8, 2020

Finally got around to adding a separate section for I18n.

@00dav00
Copy link
Contributor

00dav00 commented Jan 11, 2020

Thanks @aarona, this looks like a nice addition.

@00dav00 00dav00 merged commit 4749c34 into graphql-devise:master Jan 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants