Skip to content

Touch Up Language Dropdown #1171

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

Merged
merged 2 commits into from
Apr 29, 2017
Merged

Touch Up Language Dropdown #1171

merged 2 commits into from
Apr 29, 2017

Conversation

skipjack
Copy link
Collaborator

@skipjack skipjack commented Apr 29, 2017

Some follow up code-style and styling tweaks for #1148.

cc @lcxfs1991

Preview of styling tweaks:

… with the new dropdown

For some reason editorconfig wasn't honored and tabs snuck in. Also
tweaked the logic in the language dropdown and moved it to the right
side of the navigation bar. Made some other minor styling and logic
tweaks for easier maintenance.
@skipjack skipjack requested a review from lcxfs1991 April 29, 2017 05:46
Copy link
Collaborator

@lcxfs1991 lcxfs1991 left a comment

Choose a reason for hiding this comment

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

Looks perfect!

@lcxfs1991 lcxfs1991 merged commit 92bdefe into master Apr 29, 2017
@skipjack skipjack deleted the patch-language-dropdown branch April 29, 2017 07:13
@sprrw
Copy link
Contributor

sprrw commented Jun 15, 2017

Great job on adding multiple languages, but we should not use national flags in language selectors for the following reasons:

  1. It is wrong semantically, as the flags do not represent languages but nations. There's simply no one to one correspondence between flags and languages, neither in injective nor in surjective sense.
  2. It may be offensive to some people because their multilingual country is associated with not their own language, or because their language is not associated with their own country.
  3. Webpack has built itself some reputation, therefore using a wrong pattern will serve as a bad reference for other people.

Best way to represent languages in a limited navbar space is by using abbreviated name of the language in the language itself.

@skipjack
Copy link
Collaborator Author

skipjack commented Jun 16, 2017

@sprrw very good points, would you be interested in submitting a PR? By abbreviated name you mean the two or three letter language code like "En" for example?

@sprrw
Copy link
Contributor

sprrw commented Jun 17, 2017

Yeah, I can do a PR. Let's discuss, how do we want to present the language on the menu.
I think that the best way is just list the language as it's spelled in the language itself. So, for current set of languages, it will be:

  • English
  • 中文

Will this be a good way?

We can add ISO 639 codes as well, but on the second thought I think it'll only create unnecessary clutter without adding any useful additional information.

We can also use the "language icon" (see http://www.languageicon.org/) in place of a flag to indicate there's a language dropdown.

@skipjack
Copy link
Collaborator Author

skipjack commented Jun 18, 2017

So something like this:

[icon]
|_ English
|_ 中文
|_ Español
|_ Other options...

... with the language icon and dropdown arrow in the nav and each language option spelled out in it's own locale? Sounds like a great plan to me 👍

We can add ISO 639 codes as well, but on the second thought I think it'll only create unnecessary clutter without adding any useful additional information.

I was thinking of this more in terms of how it would be displayed in the nav bar not the actual options (e.g. En ↓) but I think it's fine to just always display the language icon.

@sprrw
Copy link
Contributor

sprrw commented Jun 18, 2017

@skipjack Does this look alright to you?

image

@skipjack
Copy link
Collaborator Author

Looks great, I would just remove the "English" section from the actual dropdown for now to conserve space as it may break the nav on smaller screen sizes:

image

In terms of implementation though you could add that bit and just comment it out for now. That way I'll know to add it back in down the road once we've cleaned up our breakpoints a bit.

@sprrw
Copy link
Contributor

sprrw commented Jun 19, 2017

ok, will do a PR then with that bit commented.

@sprrw
Copy link
Contributor

sprrw commented Jun 19, 2017

@skipjack Can you ping the most suitable reviewer for #1310?

@skipjack
Copy link
Collaborator Author

Sure I'll review shortly and ping one other for review as well.

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