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

Dropdown styles for language and year switcher #1214

Merged
merged 72 commits into from
Sep 16, 2020
Merged

Dropdown styles for language and year switcher #1214

merged 72 commits into from
Sep 16, 2020

Conversation

sudheendrachari
Copy link
Member

Fixes: #986
Intent: Redesign of year and language switcher

Major Changes:

  • Introduce two new macros 'styled_lang_dropdown' and 'styled_year_dropdown'
  • Existing DOM structure is retained and logic to generate those have been moved into
    'native_year_dropdown' and 'native_lang_dropdown'
  • Existing select-and-option structure will be used for mobile view where native dropdown picker of the
    OS will be used

UI/UX details:

  • The button which triggers dropdown will look attached to the dropdown that comes up
  • Dropdown list will be aligned to the left edge of button if the entire dropdown content can fit into the
    viewport
  • Else, Right edge of Dropdown list will be aligned to the right edge of button
  • Chevron in the button will animate to a check mark when the list is opened
  • On hover of the items in dropdown menu, its background color and font color will change

Testing:

  • Manually verified the behaviour of both the switchers
  • Verified that options are keyboard focusable (Once menu is open the list items are Tab-able,
    clicking Tab on last option navigates out of the list to the next focusable element)
  • Verified that options are reachable via screen reader also

Demo:
Mouse and Keyboard: https://drive.google.com/file/d/15-BA8r5e8QbaSi5MiuvZ4Q18L0wYzzlE/view
Voice-over: https://drive.google.com/file/d/17wkCvS7lISqwDVHXlBbsZXUDwHdL7LIx/view

Intent: Redesign of year and language switcher

Major Changes:
- Introduce two new macros 'styled_lang_dropdown' and 'styled_year_dropdown'
- Existing DOM structure is retained and logic to generate those have been moved into
'native_year_dropdown' and 'native_lang_dropdown'
- Existing select-and-option structure will be used for mobile view where native dropdown picker of the
OS will be used

UI/UX details:
- The button which triggers dropdown will look attached to the dropdown that comes up
- Dropdown list will be aligned to the left edge of button if the entire dropdown content can fit into the
viewport
- Else, Right edge of Dropdown list will be aligned to the right edge of button
- Chevron in the button  will animate to a check mark when the list is opened
- On hover of the items in dropdown menu, its background color and font color will change

Testing:
- Manually verified the behaviour of both the switchers
- Verified that options are keyboard focusable (Once menu is open the list items are Tab-able,
clicking Tab on last option navigates out of the list to the next focusable element)
- Verified that options are reachable via screen reader also
@rviscomi rviscomi added the development Building the Almanac tech stack label Aug 14, 2020
@rviscomi rviscomi added this to the 2020 Platform Development milestone Aug 14, 2020
@rviscomi rviscomi requested a review from a team August 14, 2020 21:03
Copy link
Member

@rviscomi rviscomi left a comment

Choose a reason for hiding this comment

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

Love this feature! I left some style suggestions in my review.

Also, please test the positioning logic on the footer buttons. Looks like there's some alignment issues:

image

src/static/css/2019.css Outdated Show resolved Hide resolved
src/static/css/2019.css Show resolved Hide resolved
src/static/css/2019.css Show resolved Hide resolved
src/static/css/2019.css Outdated Show resolved Hide resolved
src/static/css/2019.css Outdated Show resolved Hide resolved
src/static/css/2019.css Show resolved Hide resolved
src/static/css/2019.css Show resolved Hide resolved
src/static/css/2019.css Show resolved Hide resolved
src/static/css/2019.css Outdated Show resolved Hide resolved
Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

Very nicely done!

However appears broken on mobile:

Mobile view

Also I feel like Esc, up, down arrow keys and space should also be supported for interacting with the menu - like they are for native select.

src/templates/base/2019/base.html Outdated Show resolved Hide resolved
src/templates/base/2019/base.html Show resolved Hide resolved
Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

This is looking pretty good now. Just a few more changes required.

src/static/css/2019.css Show resolved Hide resolved
src/templates/base/2019/base.html Outdated Show resolved Hide resolved
src/templates/base/2019/base.html Outdated Show resolved Hide resolved
src/templates/base/2019/base.html Outdated Show resolved Hide resolved
src/templates/base/2019/base.html Outdated Show resolved Hide resolved
src/templates/base/2019/base.html Outdated Show resolved Hide resolved
src/templates/base/2019/base.html Outdated Show resolved Hide resolved
Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

This is looking really nice now.

A few things I've spotted (some of these might be in the "nit" category!):

More noticable in Chrome 86 (thanks to the focus-visible change), Firefox and Safari is that the border is split - the button is yellow, but the button drop down is white which breaks the illusion of it being one menu item. Is it possible to make it all yellow when button is in focus?

Yellow and White border

On a similar note there's a small, one-pixel gap between the borders. Is it possible to close this gap?

1 pixel gap

The keyboard focus ring is a bit hidden when first menu item is focused due to z-index changes:

broken focus ring

Also not loving use of z-index to be honest but can understand why you have done it. Not sure if there is a a better way that would solve above and avoid using z-index?

@tunetheweb
Copy link
Member

Maybe we should just accept the white border in the bottom instead of trying to fake this? That would avoid needing to use z-index and so also solve the focus-ring problem, and maybe the yellow/white border difference is less noticeable and so acceptable?

White border in drop down

Another thing I've noticed is that pressing up on the top item cycles right to the bottom. This kind of makes sense but acts weird on Year switcher where there is only one item. Should we include the button (i.e. the current selection) as an option in the arrow key cycle up and down?

@sudheendrachari
Copy link
Member Author

The one pixel gap, is because of the way we wanted it to look like a single polygon and do not want white border between button and menu list, and hence the cover up with gray bottom border for button (and z-index is required because list is absolute positioned)
@rviscomi What are your thoughts on this pixel gap and also about Barry's idea about changing border color to all white
@bazzadp I am little unsure if we should include button itself in list where focus is trapped and that is my initial reason to not include that arrow down keydown handler to open from button because even though visually we want them to look like single polygon, a11y wise it might be confusing to jump out of the list and jump into it again. Basically i wanted to trap focus only in the list

@tunetheweb
Copy link
Member

@bazzadp I am little unsure if we should include button itself in list where focus is trapped and that is my initial reason to not include that arrow down keydown handler to open from button because even though visually we want them to look like single polygon, a11y wise it might be confusing to jump out of the list and jump into it again. Basically i wanted to trap focus only in the list

Yeah can see that rational. Ultimately we need to decide if we want this to look and act like a single list or not. At the moment we have a kind of halfway-house. And remember A11Y is not just about blind people - sighted keyboard users could easily be confused if it looks like one list but then doesn't act like one. Personally I think add the white line to clearly differentiate between the menu and the button and solves most of these problems. It's still a small problem for the year selector since it only has one value at the moment - it will be clearer in future when there are more values and maybe we just take the hit until then. Or perhaps add a "Help Contribute" link like the "Help Translate".

Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

Noticed we've got some duplicate ids with this, so suggested a couple of changes to solve this. May want to rename mobile to mobile-header too for consistency.

Could you also merge the latest main branch into this branch as there are a couple of changes since this branch was started that will lint the generated HTML which would have caught this?

src/templates/base/2019/base.html Outdated Show resolved Hide resolved
src/templates/base/2019/base.html Outdated Show resolved Hide resolved
src/templates/base/2019/base.html Outdated Show resolved Hide resolved
@tunetheweb
Copy link
Member

Oh and one more thing I noticed is in the footer when the menu opens upwards instead of downwards the bottom border is white instead of yellow:

Bottom white border on menu open in footer

Sorry for discovering these one by one - it's quite a bit change all in all!

HakaCode and others added 6 commits September 15, 2020 19:39
* Translate-pt contributors.html

* Translate-pt error.html

* Update error.html

* Update src/templates/pt/2019/error.html

Co-authored-by: Barry Pollard <[email protected]>

* Update src/templates/pt/2019/error.html

Co-authored-by: Barry Pollard <[email protected]>

* Update src/templates/pt/2019/contributors.html

Co-authored-by: Barry Pollard <[email protected]>

* Portuguese translation of base template 2020

* Portuguese translation of base template

* Portuguese translation of base template

* Portuguese translation of base template

* Update src/templates/pt/2019/error.html

Co-authored-by: Barry Pollard <[email protected]>

Co-authored-by: Barry Pollard <[email protected]>
Bumps [ejs](https://github.com/mde/ejs) from 3.1.3 to 3.1.5.
- [Release notes](https://github.com/mde/ejs/releases)
- [Changelog](https://github.com/mde/ejs/blob/master/CHANGELOG.md)
- [Commits](mde/ejs@v3.1.3...v3.1.5)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
tunetheweb and others added 19 commits September 15, 2020 19:39
* Lint all files on Super Linter upgrade

* Fix version
* Few small perf fixes for home page

* Optimised images with calibre/image-actions

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Bumps [node-fetch](https://github.com/bitinn/node-fetch) from 2.6.0 to 2.6.1.
- [Release notes](https://github.com/bitinn/node-fetch/releases)
- [Changelog](https://github.com/node-fetch/node-fetch/blob/master/docs/CHANGELOG.md)
- [Commits](node-fetch/node-fetch@v2.6.0...v2.6.1)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [peter-evans/create-pull-request](https://github.com/peter-evans/create-pull-request) from v3.2.0 to v3.3.0.
- [Release notes](https://github.com/peter-evans/create-pull-request/releases)
- [Commits](peter-evans/create-pull-request@v3.2.0...44f76dd)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* CMS 2020

* WIP: Start on the 2020 CMS Queries

* update to 2020-08-01

* Core Web Vitals by CMS

* CWV distribution

* Lighthouse

* add results sheet URL to README

* new query for CMS adoption YoY without client breakdown

* Update from 2019 to 2020

Co-authored-by: Artem Denysov <[email protected]>

Co-authored-by: Greg Brimble <[email protected]>
Co-authored-by: Artem Denysov <[email protected]>
* COUNT(0) and newlines

* Add date columns to dated Almanac tables
Bumps [pytest](https://github.com/pytest-dev/pytest) from 6.0.1 to 6.0.2.
- [Release notes](https://github.com/pytest-dev/pytest/releases)
- [Changelog](https://github.com/pytest-dev/pytest/blob/master/CHANGELOG.rst)
- [Commits](pytest-dev/pytest@6.0.1...6.0.2)

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* pwa candidates

* pwa candidates

* pwa helpers

* pwa readme
@sudheendrachari
Copy link
Member Author

Thanks @bazzadp for helping with the rebase issue for this branch
Have addressed duplicate id issue and added yellow border when list is open

Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

LGTM! Great job and thanks for being so patient through all the feedback!

Copy link
Member

@rviscomi rviscomi left a comment

Choose a reason for hiding this comment

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

This is so well done, thank you @sudheendrachari! 🏆

I just have two small keyboard usability comments about the footer.

src/templates/base/2019/base.html Show resolved Hide resolved
src/templates/base/2019/base.html Show resolved Hide resolved
@rviscomi rviscomi merged commit f055e4a into main Sep 16, 2020
@rviscomi rviscomi deleted the dropdown-styles branch September 16, 2020 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Building the Almanac tech stack
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Style dropdown lists
8 participants