Skip to content
This repository has been archived by the owner on Feb 24, 2024. It is now read-only.

Implement #723 - Localized views #771

Merged
merged 7 commits into from
Nov 27, 2017
Merged

Implement #723 - Localized views #771

merged 7 commits into from
Nov 27, 2017

Conversation

stanislas-m
Copy link
Member

  • Enable view suffixes support (+ fallback on non-suffixed version).
  • Enable localized views support on i18n middleware, which uses view suffixes support. Opt-in option.

@stanislas-m stanislas-m added the enhancement New feature or request label Nov 23, 2017
@stanislas-m stanislas-m added this to the 0.10.2 milestone Nov 23, 2017
* When the default language is a higher preference user choice, use the non-suff
ixed template version
@markbates
Copy link
Member

So this looks a little too complicated, and requires a bit too much effort for end-users, it also changes some APIs, so I'm not sure this is the correct implementation.

What I was thinking was something like this:

If the i18n middleware is configured is a slice of accepted languages on the context.

So, let's assume it has the following slice: []string{"en-uk", "en-us", "en"}

In the action you have this: c.Render(200, r.HTML("foo.html")

The templating system would look for files in this order:

  • foo.en-uk.html
  • foo.en-us.html
  • foo.en.html
  • foo.html

If there are no languages set on the context, it should fall down to the last file on the list automatically.

This would mean that to get internationalized views you need to enable the middleware and name your files accordingly.

With this approach we don't have to change any of the APIs, everyone's apps will continue to work as they do today. Those that want localized views get them easy, those that don't, don't have to do anything.

Thoughts?

@stanislas-m
Copy link
Member Author

So this looks a little too complicated, and requires a bit too much effort for end-users, it also changes some APIs, so I'm not sure this is the correct implementation.

I don't really understand why it requires too much effort for end-users: if they want to use this feature, they just have to set LocalizedViews to true.

What I was thinking was something like this:

If the i18n middleware is configured is a slice of accepted languages on the context.

So, let's assume it has the following slice: []string{"en-uk", "en-us", "en"}

In the action you have this: c.Render(200, r.HTML("foo.html")

The templating system would look for files in this order:

foo.en-uk.html
foo.en-us.html
foo.en.html
foo.html

If there are no languages set on the context, it should fall down to the last file on the list automatically.

This would mean that to get internationalized views you need to enable the middleware and name your files accordingly.

So the changes you propose are the following, am I right?

  • Enable the localized views feature on i18n middleware presence, not using a new option.
  • Remove the suffixed views handling to use a specific version for localized views.
  • Use the languages directly in the render engine, instead of templateSuffixes.

@stanislas-m
Copy link
Member Author

@markbates Another thing, the .{lang}.html will conflict with the multiple template engine layers feature, don't you think? That's why I suggested _{lang}.html

@markbates
Copy link
Member

@stanislas-m I don't see why it should conflict, but I'm open to suggestions. perhaps name-{lang}.html?

@stanislas-m
Copy link
Member Author

@markbates That's because of exts in template.go, due to the dot, it will interpret it as an extension, and then the render loop will fail (since no render engine will be registered for that ext). It should work if I use a copy of the var for the MustBytes part, so the rest will use the normal one to work.

@stanislas-m
Copy link
Member Author

@markbates Anything else?

@markbates
Copy link
Member

Love it! I just tried it on an existing app. Everything worked as expected. I then added some localized views, changed my accept header and it found them. Perfect! This is a great feature. Thanks!

@markbates markbates merged commit ba44ab7 into development Nov 27, 2017
@markbates markbates deleted the localized-views branch November 27, 2017 22:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants