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

feat(Localized Templates): ability to register user and set language,… #22452

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

rbochenski
Copy link

… ability to have separate templates for different languages;

This is proposition to discussion:
#8239

Scope

What's changed:

  • Added ability to injext user language during registrations
  • Added ability to have separate templates for different languages

Review Notes / Questions

  • Right now this is just a proposition

Fixes #[8239]

… ability to have separate templates for different languages;
Copy link

changeset-bot bot commented May 9, 2024

⚠️ No Changeset found

Latest commit: 20b4957

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

@DanielBiegler DanielBiegler left a comment

Choose a reason for hiding this comment

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

Hey there! Thank you and congratulations on your first contribution here!! 🎊 🚀

This will need some changes before we can merge it and if you'd like you can expand upon this idea, for example adding UI for selecting a language when inviting users.

Yesterday Kevin, Jonathan and Me did an episode on this exact topic with more infos in our Request Review Show on DirectusTV - It isnt online yet but will be soon so feel free to check that out later for more infos if youd like and theres more infos in the original discussion over here.

Comment on lines +102 to +104
const localizedTemplatePath = language
? path.resolve(env['EMAIL_TEMPLATES_PATH'] as string, template + `.${language}.liquid`)
: null;
Copy link
Contributor

Choose a reason for hiding this comment

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

This implementation would mean that we would have to manually translate every single email template into every single language that we support in raw HTML. Imagine then needing to update a small thing like the styling - we would have to go through each template individually and update them all for all languages.

It'd be better if we could reuse our existing community translation integration so everyone benefits from our diverse community and would enable us to change styles/layouts without touching every single language. :)

That has been, amongst other things, mentioned yesterday here and another approach here.

Copy link
Author

Choose a reason for hiding this comment

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

Hi, thanks for response.
I understand that yet it might happen that for different languages especially for right to left you would like to have a but different views.

So with such solution we can have both, what you propose and different templates for different languages.

Also consider situation where for some languages you might want to have different colors of buttons or so.

Proposed implementation is not conflicting with your idea, just gives additional possibilities.

This comment was marked as duplicate.

@DanielBiegler DanielBiegler marked this pull request as draft May 10, 2024 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

None yet

2 participants