Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Customisation of 'Action Trouble Text' #7

Closed
fridolin-koch opened this issue May 13, 2017 · 5 comments
Closed

Customisation of 'Action Trouble Text' #7

fridolin-koch opened this issue May 13, 2017 · 5 comments
Assignees

Comments

@fridolin-koch
Copy link
Contributor

Currently I can't find any possibility to change the 'action trouble text' inside the footer:

If you’re having trouble with the button 'Hier Anmelden', copy and paste the URL below into your web browser.

This makes it impossible to fully customise the email template (e.g. i18n).

I would propose the following changes: fridolin-koch@44be377

I've you are happy with the proposed solution, I can sent a PR.

Best Regards,
Frido

@matcornic
Copy link
Owner

Hey @fridolin-koch ,

Thanks for your message, it's a nice idea ! Implementation looks great to me, I see a little edge case though.
Code does not check that user set a single %s when he overrides his troubletext string. The user may get weird behaviour without knowing it, because the email template expects a parameter to the printf function.

On top of that, we would just need more additions for the PR, could you:

  • Update README.md with the new feature
  • Update CONTRIBUTING.md with the new feature

Thank you :)

@fridolin-koch
Copy link
Contributor Author

I agree, I can think of two possible solutions.

  1. Check if the string contains %s exactly once, but this may also lead to unexpected behaviour in case the string contains %d %b etc.

  2. Define a custom placeholder like {ACTION} and replace with the action button text.

I think solution 2. would be less error prone. What do you think?

@matcornic
Copy link
Owner

I agree, let's go for the second solution ;)

@fridolin-koch
Copy link
Contributor Author

I've created a PR (#8), let me know if anything is missing :)

@matcornic
Copy link
Owner

closed with #8

mkodrats added a commit to mkodrats/hermes that referenced this issue Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants