Skip to content

Update language selection menu#2879

Merged
chrisgarrity merged 5 commits intoscratchfoundation:developfrom
chrisgarrity:issue/2658-lang-select
Aug 20, 2018
Merged

Update language selection menu#2879
chrisgarrity merged 5 commits intoscratchfoundation:developfrom
chrisgarrity:issue/2658-lang-select

Conversation

@chrisgarrity
Copy link
Contributor

Resolves

Proposed Changes

Initial update of language selection menu to avoid the menu within a menu. The language list is still a select, so the list of languages is styled by the browser being used. At some point we probably want to replace the select to have a consistent look for the languages across platforms.
/cc @carljbowman

Also removes the coming soon wrapper as it's no longer needed.

Test Coverage

Try it:
https://chrisgarrity.github.io/scratch-gui/issue/2658-lang-select/

Updated language integration test to use the new menu.

Browser Coverage

Check the OS/browser combinations tested (At least 2)

Mac

  • Chrome
  • Firefox
  • Safari

Windows

  • Chrome
  • Firefox
  • Edge

Chromebook

  • Chrome

iPad

  • Safari

Android Tablet

  • Chrome

fsih
fsih previously requested changes Aug 16, 2018
Copy link
Contributor

@fsih fsih left a comment

Choose a reason for hiding this comment

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

  1. It looks like the arrow next to the button moved down a little (before/after):
    image

  2. The behavior is a bit different on Edge; the selector that appears covers the menu bar. Does that behavior seem ok?:
    image

  3. The selector isn't working on Firefox on Windows. When the opacity is set to 100, it looks like this (it's not covering the button):
    image

  4. Should the aria label change to be on the selector, since that's the relevant button to interact with now?

Overlay a transparent `select` over the language icon. When a user clicks there the options pops up. There’s no styling of the options, other than font and font size.
@chrisgarrity chrisgarrity force-pushed the issue/2658-lang-select branch from d76c700 to 165d730 Compare August 20, 2018 12:00
@chrisgarrity chrisgarrity force-pushed the issue/2658-lang-select branch from 165d730 to 523bb04 Compare August 20, 2018 12:02
@chrisgarrity
Copy link
Contributor Author

@fsih The changes should address 1, 3 & 4. The language options pop up over the menubar in all browsers, so that's expected (although the edge version is particularly ugly)

@chrisgarrity
Copy link
Contributor Author

chrisgarrity commented Aug 20, 2018

Interesting - I just checked the language select on my PC at home (Edge, Chrome, Firefox). Chrome and Firefox do not overlap the menu but Edge does. I was surprised because on Mac Chrome does overlap the menubar....
screen shot 2018-08-20 at 8 17 38 am

In any case this behavior is completely browser specific (while you can style the select tag, it's nigh on impossible to do anything to the option tag), so we'll live with the inconsistencies until we have a better component to replace the select entirely.

border: 1px solid $motion-primary;
position: absolute;
width: 3rem;
height: 3rem;

This comment was marked as abuse.

Copy link
Contributor

@fsih fsih left a comment

Choose a reason for hiding this comment

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

LG besides the one change with the constants

Copy link
Contributor

@fsih fsih left a comment

Choose a reason for hiding this comment

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

Looks good! I tried it out on Windows Chrome, Firefox and Edge

@chrisgarrity chrisgarrity merged commit b0118cd into scratchfoundation:develop Aug 20, 2018
@chrisgarrity chrisgarrity deleted the issue/2658-lang-select branch August 20, 2018 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments