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

Localize AutoroutePart pattern #5626

Closed
wants to merge 5 commits into from

Conversation

hishamco
Copy link
Member

Fixes #5599

@hishamco hishamco requested a review from Skrypt February 24, 2020 22:21
Copy link
Contributor

@Skrypt Skrypt left a comment

Choose a reason for hiding this comment

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

LGTM

@hishamco
Copy link
Member Author

FYI I tried this

var orchardHelper = LiquidViewTemplate.Context.Services.GetService<IOrchardHelper>();
var contentItemCulture = await orchardHelper.GetContentCultureAsync(model.ContentItem);

LiquidViewTemplate.Context.CultureInfo = contentItemCulture;

it doesn't work as expected

@sebastienros
Copy link
Member

I did the change, please check

@hishamco
Copy link
Member Author

Hope to check it ASAP, also I will check why the build is fail

@Skrypt
Copy link
Contributor

Skrypt commented Feb 25, 2020

The code looks fine but it doesn't work.

@Skrypt Skrypt self-requested a review February 25, 2020 17:29
@hishamco
Copy link
Member Author

The code looks fine but it doesn't work.

That's why I asked before when I used GetContentAspectAsync() too

Copy link
Contributor

@Skrypt Skrypt left a comment

Choose a reason for hiding this comment

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

Doesn't work. I've tested this and the pipe t liquid filter doesn't translate anything. I don't know why but when I approved the changes it was working. I'm looking at the previous commits and the original commit from @hishamco and at that point it was working ; now not anymore.

LiquidTemplateView.Context.Culture doesn't affect the parsing of the Liquid template.
The only thing working so far is changing the CultureInfo.CurrentUICulture and moving it back to what it was afterward.

@hishamco
Copy link
Member Author

hishamco commented Feb 25, 2020

If you tried the original commit it works perfectly

@hishamco
Copy link
Member Author

@Skrypt did you remove the permlink and add it again after the build?

@Skrypt
Copy link
Contributor

Skrypt commented Feb 26, 2020

I'm using the refresh permalink checkbox option of the AutoroutePart. So I've set it to be updatable. I would be surprised that it makes any difference.

@Skrypt
Copy link
Contributor

Skrypt commented Feb 26, 2020

orchard

@Skrypt
Copy link
Contributor

Skrypt commented Feb 26, 2020

And here it works when I change the CultureInfo.CurrentUICulture :

orchard

Copy link
Contributor

@Skrypt Skrypt left a comment

Choose a reason for hiding this comment

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

Don't merge

@hishamco
Copy link
Member Author

I'm using the refresh permalink checkbox option of the AutoroutePart

I didn't know there's a checkbox, but when I remove and add the permlink again it works with my me at the first time before the other commits have been added

@Skrypt
Copy link
Contributor

Skrypt commented Feb 26, 2020

@hishamco I tried also by clearing the permalink input of course. Does the same.
And I also changed the code to your inital first commit to test if it work and it does not either.

orchard

@Skrypt
Copy link
Contributor

Skrypt commented Feb 26, 2020

Also tried the previous post code with removing this line in LiquidViewTemplate.
And it doesn't work either. When I'm debugging this line I can clearly see that the LiquidViewTemplate.Context.Culture is "fr" but it doesn't apply it.

image

@hishamco
Copy link
Member Author

Strange!! Could you please clean the solution and be sure that the translations are there, I can try this too if you want

@Skrypt
Copy link
Contributor

Skrypt commented Feb 26, 2020

Translations are there don't worry about that. You can see it's working on a previous gif.

@hishamco
Copy link
Member Author

I will try this again with m initial commit with IContentManager injection

@Skrypt
Copy link
Contributor

Skrypt commented Feb 26, 2020

Cleaned up the solution, same result.

@hishamco
Copy link
Member Author

Nooo .. let me try it now

@hishamco
Copy link
Member Author

Oops, I'm getting an exception whenever I create or edit content item .. I'm checking why this happing, I didn't change anything since then

@Skrypt Skrypt closed this Feb 27, 2020
@hishamco
Copy link
Member Author

hishamco commented Feb 27, 2020

Why this is closed? I will test it again once I fixed the exception I got

@sebastienros sebastienros deleted the hishamco/autoroute-pattern-localization branch April 6, 2021 17:50
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.

Autoroute Pattern and {{ | t }} filter
3 participants