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

feat(border): focus styles mechanism #1269

Merged
merged 3 commits into from
May 10, 2019
Merged

Conversation

bmdalex
Copy link
Collaborator

@bmdalex bmdalex commented Apr 25, 2019

feat(border): focus styles mechanism

BREAKING CHANGES MIGRATION STEPS

The following component variables have been removed from:

  • Attachment:
    focusInnerBorderColor, focusOuterBorderColor

  • Button:
    circularRadius, colorActive, borderColorActive, borderColorFocus, borderColorFocusIndicator, borderColorDisabled, borderWidth, primaryColorActive, primaryBorderColorActive, primaryBorderColorHover, primaryBorderColorFocus, primaryBorderColorFocusIndicator, primaryBorderWidth, primaryCircularBorderColorFocusIndicator , circularBorderColorActive, circularBorderColorFocusIndicator, borderRadiusFocused

  • RadioGroupItem:
    focusInnerBorderColor, focusOuterBorderColor

These variables are deprecated and should not be used.

  1. For the variables related to borders, Teams now have a unified way of displaying it that is consistent with the whole Teams theme => border styles should be unified, no custom border appearances.

  2. For the other variables, they were either used in dead style objects or they do not match redline requirements => NO IMPACT

Description

This PR covers:

  1. implementation of utility function that will be used for styling components on focus state (from keyboard): getBorderFocusStyles from getBorderFocusStyles.ts

  2. integration of getBorderFocusStyles for border focus styles for following components:

  • Button
  • Attachment
  • RadioGroupItem

Screenshots for Button:

Regular Theme

Screenshot 2019-05-06 at 11 40 42

Dark Theme

Screenshot 2019-05-06 at 11 41 01

High Contrast Theme

Screenshot 2019-05-06 at 11 41 23

We will adopt this mechanism for all other components in separate PR.

  1. simplifying component variables and writing styles for border by:
  • moving generic border variables from component variables to siteVariables.ts
  • leveraging getBorderFocusStyles and generic border styles from siteVariables.ts
  1. fix for Button outline styles overlapping issue in Grid:

BEFORE

Screenshot 2019-04-25 at 15 07 36

AFTER

Screenshot 2019-04-25 at 15 09 35

@bmdalex bmdalex added ⚙️ enhancement New feature or request 🚀 ready for review redlines Update of the redlines for the mentioned component architecture labels Apr 25, 2019
@bmdalex bmdalex self-assigned this Apr 25, 2019
@codecov
Copy link

codecov bot commented Apr 25, 2019

Codecov Report

Merging #1269 into master will decrease coverage by 0.18%.
The diff coverage is 28.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1269      +/-   ##
==========================================
- Coverage   72.55%   72.36%   -0.19%     
==========================================
  Files         754      756       +2     
  Lines        5647     5678      +31     
  Branches     1677     1662      -15     
==========================================
+ Hits         4097     4109      +12     
- Misses       1544     1563      +19     
  Partials        6        6
Impacted Files Coverage Δ
...teams/components/Attachment/attachmentVariables.ts 0% <ø> (ø) ⬆️
...s/components/RadioGroup/radioGroupItemVariables.ts 0% <ø> (ø) ⬆️
...s-high-contrast/components/Alert/alertVariables.ts 0% <ø> (ø) ⬆️
...rc/themes/teams/components/Alert/alertVariables.ts 0% <ø> (ø) ⬆️
...high-contrast/components/Button/buttonVariables.ts 0% <ø> (ø) ⬆️
...es/teams-dark/components/Button/buttonVariables.ts 0% <ø> (ø) ⬆️
...contrast/components/Attachment/attachmentStyles.ts 25% <ø> (+5%) ⬆️
.../themes/teams/components/Button/buttonVariables.ts 0% <0%> (ø) ⬆️
...eams/components/RadioGroup/radioGroupItemStyles.ts 18.18% <0%> (+1.51%) ⬆️
...src/themes/teams/components/Menu/menuItemStyles.ts 7.14% <0%> (ø) ⬆️
... and 12 more

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 08d2f82...f78512f. Read the comment docs.

@bmdalex bmdalex force-pushed the feat/border-styles-mechanism branch from 4160d18 to e1e81cd Compare April 25, 2019 14:58
@bmdalex bmdalex force-pushed the feat/border-styles-mechanism branch from e1e81cd to 208b974 Compare April 29, 2019 16:42
@bcalvery
Copy link
Contributor

The focus border as originally implemented appears around the item, but in this PR it looks like it is within the item. Has the design changed to be more easily implemented?

Before:
image
After:
image

This is pretty subtle in normal themes, but in contrast the "focus border" was much thicker and would look distinctly different if drawn "inside" the button.

@bmdalex bmdalex force-pushed the feat/border-styles-mechanism branch from f9e2feb to b83cc4c Compare May 6, 2019 09:07
@bmdalex
Copy link
Collaborator Author

bmdalex commented May 6, 2019

@bcalvery these changes are all part of the new design indeed

@bmdalex bmdalex force-pushed the feat/border-styles-mechanism branch from 5ba6650 to ee1120d Compare May 6, 2019 16:49
@bmdalex bmdalex force-pushed the feat/border-styles-mechanism branch from 71c726d to 851d06a Compare May 9, 2019 12:09
CHANGELOG.md Outdated Show resolved Hide resolved
@bmdalex bmdalex force-pushed the feat/border-styles-mechanism branch from 5d97b4e to bc1ef5b Compare May 10, 2019 09:11
@bmdalex
Copy link
Collaborator Author

bmdalex commented May 10, 2019

I had to squash my commits into one in order to be able to merge more easily the big amount of changes that got to master recently via #1069

Brigette Lundeen and others added 3 commits May 10, 2019 13:28
* changelog

* - removed side effects from getBorderFocusStyles by not applying any styles to the element directly (only when focused from keyboard);
* - reverted changes to ':active' selector and refactored for readability

* aligned alert focus styles with new approach

* aligned border colors to latest design

* aligned border colors to latest design v2

* fix outline

* changelog update

* addressed PR comments:
* - optimized attachment dom structure (removed 2 redundant DOM elements);
* - boxShadow consistency fixes;
* - fixed wrong casting;
* - improved comments.

* added consistency in icon styles outline vs filled

* chose better name for getIconFillOrOutlineStyles

* addressed PR comments
@bmdalex bmdalex force-pushed the feat/border-styles-mechanism branch from 407376b to f78512f Compare May 10, 2019 11:29
@bmdalex bmdalex merged commit eb47a32 into master May 10, 2019
@delete-merged-branch delete-merged-branch bot deleted the feat/border-styles-mechanism branch May 10, 2019 11:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
architecture ⚙️ enhancement New feature or request 🚀 ready for review redlines Update of the redlines for the mentioned component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants