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

Autoroute Pattern and {{ | t }} filter #5599

Closed
Skrypt opened this issue Feb 20, 2020 · 16 comments · Fixed by #7528
Closed

Autoroute Pattern and {{ | t }} filter #5599

Skrypt opened this issue Feb 20, 2020 · 16 comments · Fixed by #7528
Assignees
Milestone

Comments

@Skrypt
Copy link
Contributor

Skrypt commented Feb 20, 2020

We cannot use {{ "something" | t }} in an autoroute pattern because it will always use the CultureInfo.CurrentUICulture. I tried forcing a Culture on the LiquidTemplateContext and it doesn't take it into consideration.

@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 20, 2020

image

I did this ugly patch to fix the issue but we might want eventually to do this better.
Also tried previously to affect the _liquidTemplateManager.Context.Culture directly but it doesn't care. Probably because initially the PortableObjectStringLocalizer doesn't take any culture in param.

        public LocalizedString this[string name]
        {
            get
            {
                if (name == null)
                {
                    throw new ArgumentNullException(nameof(name));
                }

                var translation = GetTranslation(name, _context, CultureInfo.CurrentUICulture, null);

                return new LocalizedString(name, translation ?? name, translation == null);
            }
        }

@hishamco
Copy link
Member

hishamco commented Feb 20, 2020

Probably because initially the PortableObjectStringLocalizer doesn't take any culture in param

This is weird, we could use WithCulture(CultureInfo culture) until 4.0 is shipped, but I need to understand more about this issue, why you need to enforce the culture?

Is it because you need to get the proper content translation?

@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 20, 2020

The translated string is not taken from the content. I'm using a global translated string in my autoroute pattern and I want it to be translated in the culture of the content item. So, here I'm using a LocalizationPart on a content item and I want that my autoroute be saved with some french or english patterns. Example : /{{ "category" | t | slugify }}/{{ "apple" | t | slugify }}/{{ contentitemId }}

fr.po file :

msgid "category"
msgstr "catégorie"

msgid "apple"
msgstr "pomme"

@hishamco
Copy link
Member

In nutshell what you want did is "Slug/ Route Localization"?

@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 20, 2020

Yes AutoroutePart pattern localization. Right now we can't because it's always using the CultureInfo.CurrentUICulture when we parse a Liquid template. It should use the LocalizationPart.Culture to properly translate in the correct culture.

@hishamco
Copy link
Member

Ya, could use WithCulture(CultureInfo culture) for now until 4.0 is shipped, also I will try to think about this one

@hishamco
Copy link
Member

If you want me to try this one, I can start a PR

@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 20, 2020

Go for it have fun 😄

@sebastienros
Copy link
Member

ICultureAspect

@sebastienros
Copy link
Member

You will need to call Render with a custom instance of LiquidTemplateContext (new LiquidTemplateContext(ShellScope.Services)) and then pass it the culture of the content item.

@hishamco
Copy link
Member

hishamco commented Feb 21, 2020

I really forgot CultureAspect ;)

@hishamco hishamco self-assigned this Feb 21, 2020
@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 21, 2020

                var customLiquidTemplateContext = new LiquidTemplateContext(ShellScope.Services)
                {
                    CultureInfo = CultureInfo.CurrentUICulture
                };

                if (part.ContentItem.As<LocalizationPart>() != null)
                {
                    customLiquidTemplateContext.CultureInfo = CultureInfo.GetCultureInfo(part.ContentItem.As<LocalizationPart>().Culture);
                }

                part.Path = await _liquidTemplateManager.RenderAsync(pattern, customLiquidTemplateContext, NullEncoder.Default, model,
                    scope => scope.SetValue("ContentItem", model.ContentItem));
        public Task<string> RenderAsync(string source, LiquidTemplateContext context, TextEncoder encoder, object model, Action<Scope> scopeAction)
        {
            if (String.IsNullOrWhiteSpace(source))
            {
                return Task.FromResult((string)null);
            }

            var result = GetCachedTemplate(source);

            return result.RenderAsync(encoder, context, model, scopeAction);
        }

image

Same result.

@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 22, 2020

Ended up doing this for now which works :

                var currentCulture = CultureInfo.CurrentUICulture;
                if (part.ContentItem.As<LocalizationPart>() != null)
                {
                    CultureInfo.CurrentUICulture = CultureInfo.GetCultureInfo(part.ContentItem.As<LocalizationPart>().Culture);
                }

                part.Path = await _liquidTemplateManager.RenderAsync(pattern, NullEncoder.Default, model,
                    scope => scope.SetValue("ContentItem", model.ContentItem));

                if (part.ContentItem.As<LocalizationPart>() != null)
                {
                    CultureInfo.CurrentUICulture = currentCulture;
                }

@hishamco
Copy link
Member

@Skrypt seems you create a custom RenderAsync() version, which I don't have in the latest bits, is that for a reason? Is the intend to use it in UpdatedAsync()?

@Skrypt
Copy link
Contributor Author

Skrypt commented Feb 24, 2020

No in the latest bits I don't do anything else than the changes I made in my last post. I tried doing an instanciation of the LiquidTemplateContext like @sebastienros suggested but it doesn't take in consideration the LiquidTemplateContext.Culture at all. So it is using the CultureInfo.CurrentUICulture somewhere else down the line. I've found a place where we explicitly set it that way but not sure yet.

@hishamco
Copy link
Member

Ok, I'm working on it ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants