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

fix(Divider): Matching divider Teams theme to redlines#249

Merged
codepretty merged 6 commits intomasterfrom
fix/divider-redlines
Sep 20, 2018
Merged

fix(Divider): Matching divider Teams theme to redlines#249
codepretty merged 6 commits intomasterfrom
fix/divider-redlines

Conversation

@codepretty
Copy link
Collaborator

@codepretty codepretty commented Sep 18, 2018

Matched divider styles for the Teams theme to the redlines. This includes updating colors, variable names, spacing.

In Teams theme, there are only 2 types of dividers

1. Default divider w/text
image

2. Primary, important divider w/text
image

@codecov
Copy link

codecov bot commented Sep 18, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #249   +/-   ##
=======================================
  Coverage   91.84%   91.84%           
=======================================
  Files          61       61           
  Lines        1042     1042           
  Branches      134      154   +20     
=======================================
  Hits          957      957           
  Misses         82       82           
  Partials        3        3

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 77e457f...297de62. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Sep 18, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #249   +/-   ##
=======================================
  Coverage   91.84%   91.84%           
=======================================
  Files          61       61           
  Lines        1042     1042           
  Branches      134      154   +20     
=======================================
  Hits          957      957           
  Misses         82       82           
  Partials        3        3

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 77e457f...297de62. Read the comment docs.

@codecov
Copy link

codecov bot commented Sep 18, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #249   +/-   ##
=======================================
  Coverage   92.57%   92.57%           
=======================================
  Files          62       62           
  Lines        1131     1131           
  Branches      147      147           
=======================================
  Hits         1047     1047           
  Misses         81       81           
  Partials        3        3

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 1c4d67e...77ec515. Read the comment docs.

@mnajdova
Copy link
Contributor

Little bit worried about not applying any height on the Divider, as currently we used it also as an invisible divider between elements. Please take a look into this visual regression: https://screener.io/v2/states/rkGqtccHX.stardust-ui-react/fix%2Fdivider-redlines/1024x768/Chrome/portalexampleshorthandtsx-0
Please reason for this and update the styles accordingly, if you think they should be changed.

@notandrew
Copy link
Contributor

Maybe we add an optional buffer size between elements, but I'm not sure that we want a height set. That may make it harder to modify in the case where we want smaller spacers between objects.

@codepretty
Copy link
Collaborator Author

I don't think we want to be setting a height either, but for dividers in the Teams theme it's probably okay to have top/bottom spacing. At least for now until we see other use cases for it and then we can make spacing optional if that is needed.

So now we can at least have 2 dividers in succession that don't sit on top of each other...
image

@codepretty codepretty merged commit 6218033 into master Sep 20, 2018
@codepretty codepretty deleted the fix/divider-redlines branch September 20, 2018 18:43
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.

4 participants