Dropdown to change locale #102

JulienItard · 2015-07-15T17:49:31Z

Someone should add comments in routing.yml because my english it not good enough :)

bocharsky-bw · 2015-07-15T18:04:38Z

app/config/routing.yml

+    requirements: 
+        _locale: en|fr|de|es|cs|ru|uk| 
+    defaults:
+        _locale: en


I think we should use _locale: "%locale%" in order to use locale parameter from parameters.yml

weaverryan · 2015-07-15T18:22:27Z

@JulienItard I like the flags, but where are they from? Also, I think we should only add the flags that we're using from now (especially if we know the source, and can find other flags if we need them)

weaverryan · 2015-07-15T18:22:48Z

app/config/routing.yml

@@ -7,6 +7,11 @@
 app:
    resource: @AppBundle/Controller/
    type:     annotation
+    prefix:   /{_locale}
+    requirements: 
+        _locale: en|fr|de|es|cs|ru|uk| 


One too many | at the end

I think we should move accepted locales to parameters.yml too, for example:

# parameters.yml parameters: locales: en|fr|de|es|cs|ru|uk|

So we could easily refers to it in other routes in future.
What do you think?

Actually, it should go into a parameters key in config.yml in that case (since it doesn't change on a machine-by-machine basis). I think that's a good idea (we'll add a comment explaining it)

@weaverryan The last | means that locale could be an empty string. We want to use routes without locale by default for en lang

javiereguiluz · 2015-07-15T18:25:17Z

@JulienItard thanks for this. I like it.

My only concern are the flags. I think we should get rid of them. This is one of the classic usability problems, as explained here. Take for the example the Spanish language: it's official in 21 different countries, but we only show the flag for one specific country.

JulienItard · 2015-07-15T18:39:00Z

@weaverryan I use these flags https://dribbble.com/shots/1211759-Free-195-Flat-Flags

@javiereguiluz yes, but i use the country where the langage originally comes from

javiereguiluz · 2015-07-15T20:10:20Z

@JulienItard I'm afraid that's not a good enough reason. Flags cannot be shown in a language selector because countries and languages have nothing to do. In you look around, you'll only see flags when a website lets you select a country, not a language.

stof · 2015-07-16T08:13:56Z

please revert the permission changes. Templates and config files have no reason to be executable

JulienItard · 2015-07-17T14:44:55Z

Everything is ok now?

javiereguiluz · 2015-07-17T14:49:16Z

app/Resources/views/base.html.twig

+                                        <li><a href="{{ path('blog_index', {_locale:'ro'}) }}">Român</a></li>
+                                        <li><a href="{{ path('blog_index', {_locale:'ru'}) }}">Pусский</a></li>
+                                        <li><a href="{{ path('blog_index', {_locale:'uk'}) }}">Yкраїнська</a></li>
+                                        <li><a href="{{ path('blog_index', {_locale:'cs'}) }}">česká republika</a></li>


I guess česká republika means Czech Republic (the country) instead of Czech (the language). Is it Česko more appropriate?

Česky would be the correct one.

Thanks for pointing out @javiereguiluz !

Could you please add Português Brasileiro (pt_BR) to the dropdown?

@javiereguiluz @TomasVotruba Done.
@davialexandre My bad. Done.

javiereguiluz · 2015-07-17T15:06:15Z

I've tested it and from a code point of view, everything looks correct to me.

However, I'd like to suggest some design changes. This is the current design:

The first issue I see is that the icon represents a flag, which has nothing to do with languages ;) The second issue, is that I think it's better to display the current language name. The last issue is that this selector is usually dispalyed at the rightmost part of the page. Something like this:

It would be nice too if the current language is displayed in bold letters in the language list:

Lastly, if you don't like the fa-language icon, another common alternative is to use fa-globe, which FontAwesome also relates to "translation":

bocharsky-bw · 2015-07-17T15:07:38Z

app/Resources/views/base.html.twig

+                                        <li><a href="{{ path('blog_index', {_locale:'es'}) }}">Español</a></li>
+                                        <li><a href="{{ path('blog_index', {_locale:'fr'}) }}">Français</a></li>
+                                        <li><a href="{{ path('blog_index', {_locale:'ro'}) }}">Român</a></li>
+                                        <li><a href="{{ path('blog_index', {_locale:'ru'}) }}">Pусский</a></li>


The first letter in this word is from English alphabet now. Could you replace (copy/paste) it with "Русский"?

bocharsky-bw · 2015-07-17T15:11:54Z

I vote for fa-language icon 👍

JulienItard · 2015-07-17T15:30:59Z

I vote for fa-globe :)

JulienItard · 2015-07-19T12:59:07Z

Up

bocharsky-bw · 2015-07-19T14:04:00Z

Yeah, it's a bit overhead. What about to add Twig function to return locale_code => locale_name array of all supported locales?

javiereguiluz · 2015-07-19T15:28:50Z

Yes, it would be better to create a Twig global variable from the locales parameter and use something like: for locale in locales|split('|')

voronkovich · 2015-07-19T15:49:42Z

Parameters do not need to be flat strings, they can also contain array values: http://symfony.com/doc/current/components/dependency_injection/parameters.html#array-parameters. You don't need to use the "split" function.

javiereguiluz · 2015-07-19T15:53:08Z

@voronkovich yes, but we want to define the locales in one single parameter and then, define the global Twig variable based on that parameter:

parameters:
    locales: en|fr|de|es|cs|ru|uk|ro|pt_BR

# ...

twig:
    globals:
        locales: %locales%

bocharsky-bw · 2015-07-19T16:41:16Z

What about locale human readable names? Maybe somethink like:

twig:
    globals:
        languages:
            en: English
            ru: Russian
            # ... other

voronkovich · 2015-07-19T16:48:05Z

@bocharsky-bw, I agree with you. In the template we will need languages names, not only codes.

javiereguiluz · 2015-07-19T17:36:18Z

I was hoping we can define just the locale codes and Symfony will do the magic of displaying the name of each language in its own language (English, Français, Español, etc.)

voronkovich · 2015-07-19T17:58:09Z

@javiereguiluz, are you meaning this? http://symfony.com/doc/current/components/intl.html#country-names

bocharsky-bw · 2015-07-19T17:59:31Z

Yep, Intl component could help us, but I think we need locales not countries

JulienItard · 2015-07-19T20:30:48Z

Good ideas, i'll do it tomorrow.

JulienItard · 2015-07-20T16:27:37Z

What do you think about it ?

bocharsky-bw · 2015-07-20T16:37:27Z

I think locale menu relates to whole project, not only to Blog, so this method in BlogController a bit confused me. Maybe better to use custom Twig function in order to render locale menu?

JulienItard · 2015-07-20T17:59:41Z

Better ?

bocharsky-bw · 2015-07-20T18:29:34Z

I like it more 👍

javiereguiluz · 2015-07-20T18:37:41Z

src/AppBundle/Twig/LocaleExtension.php

+        $array = explode('|', $this->container->getParameter('locales'));
+
+        foreach ($array as $locale) {
+            $locales[] = ['code' => $locale, 'name' => Intl::getLocaleBundle()->getLocaleName($locale, $locale)];


We need to use the long array syntax because we still target PHP 5.3. Sorry.

javiereguiluz · 2015-07-21T06:38:01Z

@JulienItard thanks a lot for implementing this feature, for your patience during the review process and for removing the flags even if you didn't want to do it at first :)

bocharsky-bw · 2015-07-21T07:05:07Z

@JulienItard Thanks a lot!

JulienItard added 2 commits July 15, 2015 18:45

Change locale HTML/CSS + flags images

eb00375

Add _locale parameters in all routes

f1fb951

JulienItard changed the title ~~Dropdown to change locale #83~~ Dropdown to change locale Jul 15, 2015

Add dropdown locale in backend

3c66568

bocharsky-bw reviewed Jul 15, 2015
View reviewed changes

JulienItard added 2 commits July 15, 2015 20:09

Fix typo russian

ac741a6

use %locale%

f673822

weaverryan reviewed Jul 15, 2015
View reviewed changes

JulienItard added 4 commits July 15, 2015 20:39

Remove useless pipe

9ec62be

Remove unused flags

ebc986d

Add ro

b8239c3

Add locales parameter in config

1cd1def

JulienItard added 2 commits July 16, 2015 09:27

Remove flags :(

bd8c9f8

Add missing capital

03658cf

755 -> 644 permissions

2319db1

javiereguiluz reviewed Jul 17, 2015
View reviewed changes

bocharsky-bw reviewed Jul 17, 2015
View reviewed changes

Use good capital

8c8f8cb

Add pt_BR + fix Česky

846ad91

Use render controller to generate locales dropdown

b5eaddc

Change icon flag to globe

29ada5f

Use Twig Extension instead of render controller

9cd16c6

JulienItard added 2 commits July 20, 2015 20:00

Remove useless file

f711537

Add argument type

c106b99

javiereguiluz reviewed Jul 20, 2015
View reviewed changes

JulienItard added 3 commits July 20, 2015 21:07

Use long array syntax

bebb8df

Remove Container injection

05339a1

Rename locales to app_locales

b4f0e1a

javiereguiluz closed this in de5d5ef Jul 21, 2015

Dropdown to change locale #102

Dropdown to change locale #102

Conversation

JulienItard commented Jul 15, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weaverryan commented Jul 15, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javiereguiluz commented Jul 15, 2015

JulienItard commented Jul 15, 2015

javiereguiluz commented Jul 15, 2015

stof commented Jul 16, 2015

JulienItard commented Jul 17, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javiereguiluz commented Jul 17, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bocharsky-bw commented Jul 17, 2015

JulienItard commented Jul 17, 2015

JulienItard commented Jul 19, 2015

bocharsky-bw commented Jul 19, 2015

javiereguiluz commented Jul 19, 2015

voronkovich commented Jul 19, 2015

javiereguiluz commented Jul 19, 2015

bocharsky-bw commented Jul 19, 2015

voronkovich commented Jul 19, 2015

javiereguiluz commented Jul 19, 2015

voronkovich commented Jul 19, 2015

bocharsky-bw commented Jul 19, 2015

JulienItard commented Jul 19, 2015

JulienItard commented Jul 20, 2015

bocharsky-bw commented Jul 20, 2015

JulienItard commented Jul 20, 2015

bocharsky-bw commented Jul 20, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javiereguiluz commented Jul 21, 2015

bocharsky-bw commented Jul 21, 2015