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

Instantiate parent in Page->translatedLanguages() #2709

Closed
wants to merge 2 commits into from

Conversation

OleVik
Copy link
Contributor

@OleVik OleVik commented Oct 27, 2019

Fixes #2163.

@mahagr mahagr self-requested a review October 28, 2019 07:36
@mahagr mahagr self-assigned this Oct 28, 2019
@mahagr
Copy link
Member

mahagr commented Oct 28, 2019

A similar fix is probably needed for Flex Pages.

@mahagr
Copy link
Member

mahagr commented Oct 29, 2019

@OleVik There's a small issue there; parent isn't the correct one as it comes from the current language, not from the same language as the page.

Also include prefix with route.
@OleVik
Copy link
Contributor Author

OleVik commented Oct 30, 2019

@mahagr I added a check for a translated version of the parent, in the same language as the current Page, and set the parent's route to reflect that. I also updated the returned route to include the language-prefix, otherwise all languages return the same route.

@mahagr
Copy link
Member

mahagr commented Oct 31, 2019

It still won't work as you can change slug from the parent based on the language. So the URL is basically going to be wrong: /en/dogs/intro -> /fi/koirat/esittely. With your change, the resulting URL would be: /fi/dogs/esittely which is still wrong.

The only way to get it right, is to go up on a tree and initialize all the translated parent pages and get route from them.

@OleVik
Copy link
Contributor Author

OleVik commented Oct 31, 2019

That's derived from aliases, right? Wouldn't the router then apply /fi/koirat/esittely if /fi/dogs/intro was requested? I'm trying to avoid recursion upwards, assigning new routes along the way, if possible - to not impact performance as much.

@mahagr
Copy link
Member

mahagr commented Nov 4, 2019

I think the best thing though, would be to get the rawRoute() instead of trying trying to build translated parent route (it won't work with your code). If you use rawRoute(), it will never be translated (it uses folders in the filesystem to name the route) and that route is always added to the router and known to work.

The issue with your approach is that it'll not build the nested route right way causing 404 in many cases as the route cannot be found.

@mahagr
Copy link
Member

mahagr commented Nov 5, 2019

@OleVik I checked this and, unfortunately no, your suggestion won't work like that as Grav currently keeps record only from the current translation. This means that all the other slugs are coming from the wrong language.

@OleVik
Copy link
Contributor Author

OleVik commented Nov 8, 2019

What associates /en/dogs/intro with /fi/koirat/esittely though? Seemingly the slug, per Multi-Language Routing. I probably need to expand my test-case, I initially just needed a fix for $this->grav['page']->translatedLanguages(true) - which just returns empty values right now.

My assumption about the aliases stemmed from Language Alias Routes, which with rawRoute() is supposed to process the page accordingly - from /fi/dogs/intro yielding /fi/koirat/esittely.

@rhukster
Copy link
Member

rhukster commented Nov 8, 2019

Each language, no matter the language specific slugs, are associated by Lang and raw route.

@OleVik
Copy link
Contributor Author

OleVik commented Nov 8, 2019

But do they then belong to different folders, or does the Page FrontMatter just declare the translation? I haven't really had need to use multi-language that much in Grav before, so my test-cases were always rather simple.

@mahagr mahagr closed this May 20, 2021
@mahagr mahagr deleted the branch getgrav:1.7 May 20, 2021 14:03
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.

3 participants