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

Make { "text" | t } work in autoroute pattern. #7517

Merged
merged 1 commit into from
Nov 5, 2020

Conversation

Skrypt
Copy link
Contributor

@Skrypt Skrypt commented Nov 5, 2020

Reinsert this feature which was not working before.

See #5626

@Skrypt Skrypt merged commit d67d509 into dev Nov 5, 2020
@delete-merged-branch delete-merged-branch bot deleted the skrypt/autoroute-pattern-loc branch November 5, 2020 06:06
@hishamco
Copy link
Member

hishamco commented Nov 5, 2020

Similar to what I did before, what we are missing before?!!

@Skrypt
Copy link
Contributor Author

Skrypt commented Nov 5, 2020

There's an other part coming later on. The fix was appropriate for the Liquid filter but it still doesn't work because we can't localize a Liquid template.

@Skrypt
Copy link
Contributor Author

Skrypt commented Nov 5, 2020

I looked and we use a IViewLocalizer which inherits from a IHtmlLocalizer which has a method .WithCulture which has been deprecated. They say :

[System.Obsolete("This method is obsolete. Use `CurrentCulture` and `CurrentUICulture` instead.")]
public Microsoft.AspNetCore.Mvc.Localization.IHtmlLocalizer WithCulture (System.Globalization.CultureInfo culture);

So basically what I did in the AutoroutePartHandler by changing the CurrentUICulture before rendering the Liquid Template and setting it back to what it was afterward ; but I think it needs to be done more globally.

Also, while testing I found out that if you add a LocalizationPart to the LiquidPage Content Type for example. It doesn't take the LocalizationPart into consideration. Right now a LiquidPage renders in all cultures and so is a LiquidTemplate. It just takes the CultureInfo.CurrentUICulture all the time. Now I assume that what @sebastienros was saying is to assign the LiquidViewTemplate.Context.CultureInfo before rendering the Liquid template. But nowhere in the code we use this Context.CultureInfo when rendering the Liquid template.

We need to add a distinction in the code between "default : CultureInfo.CurrentUICulture" and when we assign eplicitly a culture to the LiquidViewTemplate.Context which I will continue work on tomorrow.

@Skrypt
Copy link
Contributor Author

Skrypt commented Nov 5, 2020

So basically, in the same request we should be able to render multiple Liquid Templates in a different culture. The current page culture has nothing to do with it. The LiquidTemplate culture should be defined by the CultureAspect but we probably don't call _contentManager.PopulateAsync() on every time because the Liquid template could be also file based.

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

Successfully merging this pull request may close these issues.

2 participants