Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

feat(menu): add menu item focus styles #286

Merged
merged 1 commit into from
Oct 2, 2018
Merged

feat(menu): add menu item focus styles #286

merged 1 commit into from
Oct 2, 2018

Conversation

bmdalex
Copy link
Collaborator

@bmdalex bmdalex commented Sep 27, 2018

Menu

This PR main purposes:

  • adds keyboard focus styles by leveraging existing functionality;
  • updates existing styles to adhere better to Teams theme;

TODO

  • Conformance test
  • Minimal doc site example
  • Stardust base theme
  • Teams Light theme
  • Teams Dark theme
  • Teams Contrast theme
  • Confirm RTL usage
  • W3 accessibility check
  • Stardust accessibility check
  • Update glossary props table
  • Update the CHANGELOG.md

Description

type="primary"

All primary (type="primary") menus get updated background for active, focus and hover states.
screen shot 2018-09-27 at 02 49 14

underlined

Underlined menus get new styles for active, focus and hover states.

screen shot 2018-09-27 at 02 49 46
screen shot 2018-09-27 at 02 50 02

iconOnly

Menus with only icons as item get new styles for active, focus and hover states.

1. regular:

screen shot 2018-09-27 at 02 50 22

2. inverted (by tweeking variables):

screen shot 2018-09-27 at 02 51 37

3. vertical (with vertical prop):

screen shot 2018-09-27 at 02 51 57

@codecov
Copy link

codecov bot commented Sep 27, 2018

Codecov Report

Merging #286 into master will decrease coverage by 0.28%.
The diff coverage is 68.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #286      +/-   ##
==========================================
- Coverage    89.9%   89.62%   -0.29%     
==========================================
  Files          62       62              
  Lines        1159     1166       +7     
  Branches      150      173      +23     
==========================================
+ Hits         1042     1045       +3     
- Misses        115      119       +4     
  Partials        2        2
Impacted Files Coverage Δ
src/components/RadioGroup/RadioGroupItem.tsx 100% <100%> (ø) ⬆️
src/components/Button/Button.tsx 95.74% <100%> (ø) ⬆️
src/components/Menu/MenuItem.tsx 80% <61.53%> (-8.89%) ⬇️
src/index.ts 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13861f5...e543111. Read the comment docs.

@bmdalex bmdalex changed the title feat(menu): add focus behaviour and improve styles feat(menu): add menu item focus styles Sep 27, 2018
@bmdalex
Copy link
Collaborator Author

bmdalex commented Sep 27, 2018

ready for review:
@miroslavstastny @mnajdova @levithomason @kuzhelov

Copy link
Member

@miroslavstastny miroslavstastny left a comment

Choose a reason for hiding this comment

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

Code looks good, not sure about removing focus ring for all Menu types.

@kuzhelov kuzhelov added the needs author feedback Author's opinion is asked label Oct 1, 2018
@bmdalex bmdalex removed the needs author feedback Author's opinion is asked label Oct 1, 2018
Copy link
Contributor

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Left one comment for consideration, other then that, looks good by me. Please create an issue for the isFromKeyboard service explaining the problem you encountered.

@bmdalex bmdalex merged commit 5bf9300 into master Oct 2, 2018
@bmdalex bmdalex deleted the feat/menu-focus branch October 2, 2018 12:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants