mcamara / laravel-localization

Easy localization for Laravel
MIT License
3.38k stars 515 forks source link

getLocalizedURL doesn't translate routes #448

Closed francoisdusautoir closed 5 years ago

francoisdusautoir commented 7 years ago

Hello.

Laravel-localization works well on my application. When I am on the homepage if I switch language and navigate on the application, all is running well, routes are translated as expected in all links, menus etc.

The problem is when I am on a page, if I click on the switch languages this is what I have :

- http://app.dev/fr/titres/mon-titre
- http://app.dev/en/titres/mon-titre
- http://app.dev/es/titres/mon-titre

This is what I should have :

- http://app.dev/fr/titres/mon-titre
- http://app.dev/en/titles/mon-titre
- http://app.dev/es/titulos/mon-titre

On my header.blade.php, this is what I have :


@foreach(Localization::getSupportedLocales() as $lang => $properties)
    <a
        href="{{Localization::getLocalizedURL($lang)}}"
        @if(Localization::getCurrentLocale() == $lang) class="active" @endif
    >
        {{$lang}}
    </a>
@endforeach

Insofar as all is working as expected for the language when we are on a good URL ... what is the issue with that problem ? I saw many posts about similar problems with the getLocalizedURL() method, I tried many "issues" without success ... :/

quaidesbalises commented 7 years ago

Any help ... ?

kazak71 commented 7 years ago

Yes, I have the same problem. When I made some deeper debug into the code I found that function getLocalizedURL returns wrong results when routes (in lang routes.php) are declared with parameters. When routes are clean (without parameters) everything is fine and working as desired. I found problem is really in function extractAttributes - it returns wrong results when routes are declared with parameters. WORKAROUND :):

  1. declare ALL routes as NAMED ROUTES in file app/routes/web.php (for laravel 5.3 or 5.4)
  2. instead getLocalizedURL you may use getURLFromRouteNameTranslated if it is possible
  3. change function handle(...) in file LaravelLocalizationRoutes.php like this:

    public function handle($request, Closure $next) { // If the URL of the request is in exceptions. if ($this->shouldIgnore($request)) { return $next($request); } $app = app(); if (app()->router->currentRouteName() !== '') { $routeName = 'routes.'.app()->router->currentRouteName(); } else { $routeName = $app['laravellocalization']->getRouteNameFromAPath($request->getUri()); } $app['laravellocalization']->setRouteName($routeName); return $next($request); }

With these changes getLocalizedURL should be working. I think this will help you! Good Luck!

gerardreches commented 7 years ago

That workaround doesn't work for me, and I'm experiencing the problem without having parameterized routes.

mrcbk commented 7 years ago

I had this issue as well and created a pull request: here

gerardreches commented 7 years ago

@mrcbk Are you using laravel-translation-manager package at the time you found that fix? There was an issue in the other package.

Laravel trans function has 3 parameters, not 4:

public function trans($key, array $replace = [], $locale = null)
{
    return $this->get($key, $replace, $locale);
}

So your pull request is invalid, same as #447.

Issue in vsch/laravel-translation-manager (fork of barryvdh/laravel-translation-manager, both can have this problem)

mrcbk commented 7 years ago

no, actually it's vanilla laravel 5.3: https://github.com/laravel/framework/blob/5.3/src/Illuminate/Translation/Translator.php#L235

So i guess for 5.4 it's invalid. Still if you are on 5.3.

gerardreches commented 7 years ago

Uhm.. Then the package will need a breakpoint between Laravel 5.3 and 5.4.

thisprojectworks commented 7 years ago

I have the same issue in laravel 5.4

andrelegault commented 7 years ago

Experiencing the same issue (5.4)

mtx-z commented 7 years ago

Same issue on Laravel 5.5.

It works with 1 parameter, not with more: only locale is changing.

en/routes.php
'post' => [
        'index' => 'post',
        'show' => 'post/{post}',
    ]

fr/routes.php
'post' => [
        'index' => 'post-fr',
        'show' => 'post-fr/{post}',
    ]

route/web.php
Route::name( 'post.' )
                  ->group( function () {
                      Route::get( LaravelLocalization::transRoute( 'routes.post.index' ), 'PostController@dashboard' )->name( 'index' );
                      Route::get( LaravelLocalization::transRoute( 'routes.post.show' ), 'PostController@show' )->name( 'show' )->where( 'post', '[0-9]+' );
                  } );

(Note that the "post" ("post-fr" in french) segment in english version would normally be implemented as a Route group translated prefix, but it does not work as of #290 - prefix is so directly included in all routes translations values when needed).

getLocalizedURL() is working great for "index" route, but not working for "show" route.

Index route output: EN: http://domain.com/en/post FR: http://domain.com/fr/post-fr

Show route output: Locale is EN -> EN: http://domain.com/en/post/1 FR: http://domain.com/fr/post/1 (should be http://domain.com/fr/post-fr/1)

Locale is FR -> EN: http://domain.com/en/post-fr/1 (should be http://domain.com/en/post/1) FR; http://domain.com/fr/post-fr/1

In the end, we can "tweak" it by setting same naming pattern on Route names and Route translated names. Being able to do something like :

LaravelLocalization::getURLFromRouteNameTranslated($localeCode, 'routes.' . Route::currentRouteName(), Route::current()->parameters, false);

as getURLFromRouteNameTranslated() is working great if you pass the URL parameters to the method. In this case, just be careful with Laravel Route model binding. As the Parameter value will not be the ID (int/string) anymore, but the found model instance (ID based if not overided). I made a quick helper to send back the ID if the model as replaced the real URL parameter value.

This is not perfect and have limitations (translated route naming), but as getLocalizedURL() is "not safe enough", it's a way to achieve a dynamic language switcher.

Related at ARCANEDEV/Localization#68, but not tested again this specific issue route name settings (2 translated parameter all from route translation file, here i have 1 variable wich is the ID).

f-liva commented 7 years ago

Same here, both with Laravel 5.4 and 5.5.

More precisely, it doesn't work only for the last parameter of the route. If my route has two parameterers, only the first is localized. If it has three parameters only first two are translated, etc.

Anyone has found where is located that issue?

To @mtx-z, also LaravelLocalization::getURLFromRouteNameTranslated doesn't work fine for me.

mtx-z commented 7 years ago

What's wrong with getUrlFromRouteTranslated()? You need to provide the route name from the translation file, not the one from laravel router.

f-liva commented 7 years ago

Yes, I use the routes name from the translation file but both getLocalizedURL and getURLFromRouteNameTranslated work as described in my previous comment.

f-liva commented 7 years ago

I found a working workaround:

$routeName = 'routes.' . Route::currentRouteName();
$translatedParameters = current(event('routes.translation', [$localeCode, Route::current()->parameters]));
$translatedUrl = LaravelLocalization::getURLFromRouteNameTranslated($localeCode, $routeName, $translatedParameters);
mtx-z commented 7 years ago

You also matched router route name and translated route name?

meduzen commented 7 years ago

Hi, here’s my (temporary for the project I'm working on) solution.

Please carefully read the commented documentation, and note that my fellow back-end developer colleague still has to review it.

f-liva commented 6 years ago

The workaround I found does not work with routes defined as resources. A small variation is needed:

$routeName = 'routes.' . Route::currentRouteName();
$translatedParameters = current(event('routes.translation', [$localeCode, Route::current()->parameters]));

if (!empty($translatedParameters)) {
    $itemUrl = LaravelLocalization::getURLFromRouteNameTranslated($localeCode, $routeName, $translatedParameters);
} else {
    $itemUrl = LaravelLocalization::getLocalizedURL($localeCode);
}

@mcamara Could we find a definitive solution to this problem?

Grupa-Leonardo commented 6 years ago

The author apparently is no longer interested in further development of the package - it's time to look for something else.

f-liva commented 6 years ago

@Grupa-Leonardo any suggestion?

Grupa-Leonardo commented 6 years ago

@fede91it At this moment - nothing, but for next projects I'll look for somethink different. Now I must make it more 'statically' -.-

kazak71 commented 6 years ago

take a look at this: https://stackoverflow.com/questions/42527880/laravel-5-4-translate-routes I think it may be useful with some changes but idea is clear. You may have to use named routes in route definitions. Good luck!

f-liva commented 6 years ago

Unfortunately it is too minimal a solution, I have more specific needs that currently only mcamara can satisfy.

I found this other project, which looks like an improved mcamara clone:

https://github.com/ARCANEDEV/Localization

kazak71 commented 6 years ago

fede91it, this is great! Thank you!

stefkes commented 6 years ago

Just out of curiosity, how would this package be able to fetch the translation of a slug from the database?

As far as I can tell this package only talks to the routes file and the files that hold the translations of named routes, for example resources/lang/fr/routes.php

Anyone?

mtx-z commented 6 years ago

@stefkes I think you should use another package.

I use myself mcamara/laravel-localization for route translations, and dimsav/laravel-translatable for model translations (slug, and all fields that you want translated).

iwasherefirst2 commented 5 years ago

@francoisdusautoir is this issue still present? getLocalizedURL seems to return the corrected translated urls now, even if you use parameters.

iwasherefirst2 commented 5 years ago

Closing this as its seems to be fixed by now

Kamran53596 commented 9 months ago

Hi. Language switcher works with translated slug like this?