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

LaravelLocalization::getLocalizedURL issues! #47

Closed
nikosv opened this issue Jan 20, 2014 · 26 comments
Closed

LaravelLocalization::getLocalizedURL issues! #47

nikosv opened this issue Jan 20, 2014 · 26 comments

Comments

@nikosv
Copy link
Contributor

nikosv commented Jan 20, 2014

Hello,
I am having some issues with this function which i am not sure if it is a bug or i am missing something. Probably there is something i am missing.

I am trying to use this function to build localized urls for my app like so:

<a href="{{ LaravelLocalization::getLocalizedURL(LaravelLocalization::getCurrentLanguage(), $category->category_slug) }}">whatever</a>

The above example doen't work when in my '/' route. However a similar implementation works perfectly in other routes.

What is more i cannot generate localized urls for my '/' route while trying the following.

LaravelLocalization::getLocalizedURL(LaravelLocalization::getCurrentLanguage(), '/')
@nikosv
Copy link
Contributor Author

nikosv commented Jan 20, 2014

My bad, I dug deeper into the code and looks like the correct way to generate links is as follows:

LaravelLocalization::getLocalizedURL(LaravelLocalization::getCurrentLanguage(), url($category->category_slug))

@nikosv nikosv closed this as completed Jan 20, 2014
@nikosv nikosv reopened this Jan 20, 2014
@nikosv
Copy link
Contributor Author

nikosv commented Jan 20, 2014

However shouldn't the getLocalizedURL() function return a url with no language code for the default language if the 'hideDefaultLanguageInRoute' in config is set to TRUE? I think it makes more sense than redirecting from a localized url to a non-localized for the default language.

@uintaam
Copy link
Contributor

uintaam commented Jan 21, 2014

Your right. I'll create a fix and pull-request shortly.

@uintaam
Copy link
Contributor

uintaam commented Jan 21, 2014

Also, you're initial issue is valid. I'll alter the function to deal with paths.

@uintaam
Copy link
Contributor

uintaam commented Jan 21, 2014

#49 should fix your issues.

@uintaam
Copy link
Contributor

uintaam commented Jan 21, 2014

@nikosv the fixes for this issue have been merged up in to the master branch. Can you confirm if your issue is resolved or not?

@nikosv
Copy link
Contributor Author

nikosv commented Jan 21, 2014

@drewjw81 First of all some changes must be made to languagebar.blade.php which is still using

LaravelLocalization::getCurrentLanguage().

What is more getLocalizedUrl works great for 'hideDefaultLanguageInRoute' => false but behaviour doesn't seem to change for 'hideDefaultLanguageInRoute' => true.

Moreover, even though it is not that important, it would be better if someone could just do

LaravelLocalization::getLocalizedUrl(LaravelLocalization::getCurrentLocale(), 'route_name') 

instead of

LaravelLocalization::getLocalizedURL(LaravelLocalization::getCurrentLocale(), '/route_name')

while it makes sense as it is for the home route.

@uintaam
Copy link
Contributor

uintaam commented Jan 21, 2014

I'll take a look at these tomorrow.

On Tue, Jan 21, 2014 at 10:19 PM, nikosv notifications@github.com wrote:

@drewjw81 First of all some changes must be made to languagebar.blade.php whicj is still using LaravelLocalization::getCurrentLanguage().
What is more getLocalizedUrl works great for 'hideDefaultLanguageInRoute' => false but behaviour doesn't seem to change for 'hideDefaultLanguageInRoute' => true.
Moreover, even though it is not that important, it would be better if someone could just do LaravelLocalization::getLocalizedUrl(LaravelLocalization::getCurrentLocale(), 'route_name') instead of
LaravelLocalization::getLocalizedURL(LaravelLocalization::getCurrentLocale(), '/route_name'

while it makes sense as it is for the home route.

Reply to this email directly or view it on GitHub:
#47 (comment)

@uintaam
Copy link
Contributor

uintaam commented Jan 22, 2014

Fixed the issues mentioned above in #55

Staring work on suggested getLocalizedUrl changes.

@uintaam
Copy link
Contributor

uintaam commented Jan 22, 2014

All the above issues including one new one I discovered should be solved in v0.10.1 coming in pull request #55

@uintaam
Copy link
Contributor

uintaam commented Jan 24, 2014

@nikosv can you confirm if all the issues related to this are completed now? If so please close off the ticket.

@nikosv
Copy link
Contributor Author

nikosv commented Jan 24, 2014

There is still the issue related to

LaravelLocalization::getLocalizedUrl(LaravelLocalization::getCurrentLocale(), 'whatever1/whatever2') 

which produces links with twice the locale in the url like so
en/en/whatever1/whatever2

as i have described in #55

@uintaam
Copy link
Contributor

uintaam commented Feb 17, 2014

Pull request #76 should finally fix this issue.

@uintaam
Copy link
Contributor

uintaam commented Feb 18, 2014

Branch is merged as of yesterday. Please re-test and close this issue if we're good to go.

@nikosv
Copy link
Contributor Author

nikosv commented Feb 18, 2014

@drewjw81 i am afraid we still have a problem with

LaravelLocalization::LocalizeURL('/')

which doesn't seem to work properly.

@uintaam
Copy link
Contributor

uintaam commented Feb 18, 2014

What is the result?

@nikosv
Copy link
Contributor Author

nikosv commented Feb 18, 2014

it produces a url with a slash in the end like
http://localhost/laravel/public/en/
which redirects to
localhost/en
instead of producing http://localhost/laravel/public/en (without the slash in the end)

@uintaam
Copy link
Contributor

uintaam commented Feb 18, 2014

Thanks for catching that. I should always be stripping the trailing slashes. #77 fixes this and one more minor issue.

@nikosv
Copy link
Contributor Author

nikosv commented Feb 18, 2014

i am not sure if this is due to the same issue but there where also problems in my vhost

LaravelLocalization::LocalizeURL('contact') 

instead of "laravel.dev/contact" returns just "contact"

@uintaam
Copy link
Contributor

uintaam commented Feb 18, 2014

That is by design, for localizing relative URLs. If you want "http://laravel.dev/contact/en" you'll need to pass "http://laravel.dev/contact".

LaravelLocalization::LocalizeURL('/contact') will return "/contact/en"
LaravelLocalization::LocalizeURL('/contact/') will return "/contact/en" (v0.13.2)
LaravelLocalization::LocalizeURL('http://laravel.dev/contact') will return "http://laravel.dev/contact/en"
LaravelLocalization::LocalizeURL('http://laravel.dev/contact/') will return "http://laravel.dev/contact/en" (v0.13.2)

@uintaam
Copy link
Contributor

uintaam commented Feb 18, 2014

Or LaravelLocalization::LocalizeURL(Request::getSchemeAndHttpHost() . '/contact') would probably more appropriate if you need a full URL

@nikosv
Copy link
Contributor Author

nikosv commented Feb 22, 2014

ok... i guess everything looks good ... i am closing the issue.

@nikosv nikosv closed this as completed Feb 22, 2014
@nikosv nikosv reopened this Feb 22, 2014
@nikosv
Copy link
Contributor Author

nikosv commented Feb 22, 2014

@drewjw81
i am afraid i have to reopen the issue since the

LaravelLocalization::LocalizeURL('/')

seems to localize the current url instead of the home url for the default language with the language prefix hidden.
any other language which the language prefix is present in the url works like a charm.

@uintaam
Copy link
Contributor

uintaam commented Feb 24, 2014

I'll take a look at this tomorrow and issue a fix.  Time to finish off the test suite for this as well.

On Sat, Feb 22, 2014 at 6:53 PM, nikosv notifications@github.com wrote:

@drewjw81
i am afraid i have to reopen the issue since since the

LaravelLocalization::LocalizeURL('/')

seems to localize the current url.

is this normal behaviour? and how do i generate a localized url to the home page.

Reply to this email directly or view it on GitHub:
#47 (comment)

@uintaam
Copy link
Contributor

uintaam commented Mar 2, 2014

@nikosv I'm not able to replicate this. Can you give any more detail? I've tried with hideDefaultLocaleInURL true and false and with Laravel public as the root or in a subfolder.

@nikosv
Copy link
Contributor Author

nikosv commented Mar 2, 2014

Now i am not able to replicate it either... so i guess it was my mistake...
sorry for the trouble...
issue closed.

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

No branches or pull requests

2 participants