Skip to content

Add prototype of language selector design #1562

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

Closed
wants to merge 2 commits into from

Conversation

catarak
Copy link
Member

@catarak catarak commented Aug 19, 2020

✨ Requesting feedback ✨ from @andrewn and @oruburos

Yesterday, @ghalestrilo and I talked about integrating the translations with the mobile design (#1559). In trying to come up a design for how to select languages on mobile, I was messing around with the desktop design to see if that would generate ideas and I came up with this:

Screen Shot 2020-08-19 at 2 58 17 PM

Screen Shot 2020-08-19 at 2 58 12 PM

Screen Shot 2020-08-19 at 3 12 48 PM

I don't want to step on your toes so feel free to ignore this, change this, etc.—do what you think is best! I figured I would just put it out there 😄

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • is from a uniquely-named feature branch and has been rebased on top of the latest master. (If I was asked to make more changes, I have made sure to rebase onto master then too)
  • is descriptively named and links to an issue number, i.e. Fixes #123

@andrewn
Copy link
Member

andrewn commented Aug 20, 2020

Yeah, I think this looks good.

The two key ideas here are:

  • aligning the menu to the right, next to the account selection
  • displaying the current language in the menu instead of "language"

@oruburos What do you think?

@oruburos
Copy link
Collaborator

I like the idea. Clearly is cleaner, my "design" is just floating in the middle of nowhere. Also, displaying the actual language instead of some generic top-level label for the dropdown is a nice touch.

@catarak
Copy link
Member Author

catarak commented Aug 20, 2020

@oruburos okay awesome! I'll let you take the lead on this one and do what you would like with this PR—feel free to add on top of it or make your own.

@oruburos
Copy link
Collaborator

ok @catarak , I'm pulling this feature to check why the test is failing (probably Nav_test) and will upload after that.

@catarak
Copy link
Member Author

catarak commented Aug 21, 2020

@oruburos since I made changes to the Nav component it's probably that the snapshot needs to be updated!

@oruburos
Copy link
Collaborator

@catarak Yes, I tried that approach, but I think that also requires PR #1568, (at least, when I used a local branch with both changes everything went smooth, which was not the case without that patch) so once that we merged that PR #1568 I will create the PR.

@oruburos oruburos mentioned this pull request Aug 26, 2020
3 tasks
@oruburos
Copy link
Collaborator

This change is the main component of PR #1582, I suggest to close this PR.

@andrewn
Copy link
Member

andrewn commented Aug 27, 2020

Closing this PR in favour of #1582.

@andrewn andrewn closed this Aug 27, 2020
@andrewn andrewn deleted the feature/language-selection-design branch August 27, 2020 08:46
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