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

fix(radio buttons): Update radio button styles #830

Merged
merged 22 commits into from
Mar 25, 2019

Conversation

notandrew
Copy link
Member

@notandrew notandrew commented Feb 1, 2019

Updating the radio buttons to match color redlines.

BEFORE
image

AFTER
Teams theme
image

Teams dark theme
image

Teams high contrast theme
image

@codecov
Copy link

codecov bot commented Feb 2, 2019

Codecov Report

Merging #830 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #830   +/-   ##
=======================================
  Coverage   93.54%   93.54%           
=======================================
  Files          21       21           
  Lines         728      728           
  Branches       69       69           
=======================================
  Hits          681      681           
  Misses         47       47

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 da0fc96...e75b9ba. Read the comment docs.

@codecov
Copy link

codecov bot commented Feb 2, 2019

Codecov Report

Merging #830 into master will decrease coverage by <.01%.
The diff coverage is 53.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #830      +/-   ##
==========================================
- Coverage   82.09%   82.08%   -0.01%     
==========================================
  Files         711      713       +2     
  Lines        8528     8536       +8     
  Branches     1224     1226       +2     
==========================================
+ Hits         7001     7007       +6     
- Misses       1511     1513       +2     
  Partials       16       16
Impacted Files Coverage Δ
...s/components/RadioGroup/radioGroupItemVariables.ts 100% <ø> (ø) ⬆️
...t/components/RadioGroup/radioGroupItemVariables.ts 100% <100%> (ø)
.../react/src/themes/teams-dark/componentVariables.ts 100% <100%> (ø) ⬆️
...c/themes/teams-high-contrast/componentVariables.ts 100% <100%> (ø) ⬆️
...k/components/RadioGroup/radioGroupItemVariables.ts 100% <100%> (ø)
...ct/src/themes/teams-high-contrast/siteVariables.ts 100% <100%> (ø) ⬆️
...eams/components/RadioGroup/radioGroupItemStyles.ts 28.57% <25%> (+8.57%) ⬆️

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 22f0841...3407906. Read the comment docs.

@codepretty codepretty changed the title updating radio button styles fix(radio buttons): Update radio button styles Feb 6, 2019
manajdov added 2 commits February 26, 2019 14:50
# Conflicts:
#	packages/react/src/themes/teams-dark/siteVariables.ts
#	packages/react/src/themes/teams/components/RadioGroup/radioGroupItemVariables.ts
#	packages/react/src/themes/teams/siteVariables.ts
@codepretty
Copy link
Collaborator

Waiting on the radio button html structure PR to complete to fix & finish this redlining PR.

@layershifter
Copy link
Member

@codepretty @notandrew #1070 was merged 🎉

const radioStyles: ComponentSlotStylesInput<
RadioGroupItemProps & RadioGroupItemState,
RadioGroupItemVariables
> = {
root: ({ props: p, variables: v }): ICSSInJSStyle => ({
alignItems: 'baseline',
// can remove this after global style for border-box goes in
Copy link
Collaborator

Choose a reason for hiding this comment

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

todo: remove after #1057 gets checked in

...(p.isFromKeyboard && {
// this creates both inset and outset box shadow that some readers (NVDA) show when radio is not checked but it is focused
boxShadow: `0 0 0 ${pxToRem(1)} ${v.color},` + `0 0 0 ${pxToRem(2)} ${v.color} inset`,
// can remove this after global style for border-box goes in
Copy link
Collaborator

Choose a reason for hiding this comment

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

todo: remove after #1057 gets checked in

colorDisabled: string

colorBorder: string
colorBorderChecked: string
// can these be global colors so we don't have to define for every component?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any suggestions? Almost every component will need focusInnerBorderColor and focusOuterBorderColor, is there a better way to define so it's not added to all the variables?

Copy link
Contributor

Choose a reason for hiding this comment

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

After the color scheme this will be aligned with the color scheme used per theme. Let's restrain for making some changes at this moment.

Copy link
Contributor

@mnajdova mnajdova Mar 22, 2019

Choose a reason for hiding this comment

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

We can define the borderFocus and shadowFocus colors int he scheme.

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.

Check comments before merging.

@codecov
Copy link

codecov bot commented Mar 25, 2019

Codecov Report

Merging #830 into master will decrease coverage by <.01%.
The diff coverage is 53.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #830      +/-   ##
==========================================
- Coverage   82.09%   82.08%   -0.01%     
==========================================
  Files         711      713       +2     
  Lines        8528     8536       +8     
  Branches     1224     1161      -63     
==========================================
+ Hits         7001     7007       +6     
- Misses       1511     1513       +2     
  Partials       16       16
Impacted Files Coverage Δ
...s/components/RadioGroup/radioGroupItemVariables.ts 100% <ø> (ø) ⬆️
...t/components/RadioGroup/radioGroupItemVariables.ts 100% <100%> (ø)
.../react/src/themes/teams-dark/componentVariables.ts 100% <100%> (ø) ⬆️
...c/themes/teams-high-contrast/componentVariables.ts 100% <100%> (ø) ⬆️
...k/components/RadioGroup/radioGroupItemVariables.ts 100% <100%> (ø)
...ct/src/themes/teams-high-contrast/siteVariables.ts 100% <100%> (ø) ⬆️
...eams/components/RadioGroup/radioGroupItemStyles.ts 28.57% <25%> (+8.57%) ⬆️

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 22f0841...3d43f60. Read the comment docs.

@codepretty codepretty merged commit e62ce21 into master Mar 25, 2019
@delete-merged-branch delete-merged-branch bot deleted the andmarti/radio-colors branch March 25, 2019 18:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🚀 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.

None yet

5 participants