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

feat(Header): introduce 'color' variable #72

Merged

Conversation

kuzhelov
Copy link
Contributor

@kuzhelov kuzhelov commented Aug 9, 2018

Header color

Introduce color property to Header and Header.Description

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

API Proposal

Header - color (variable)

Defines color of Header.

image

<Header variables={{ color: 'red' }} content="..." />

Header.Description - color (variable)

Defines color of header's description (Header.Description).

image

<Header ... >
   <Header.Description variables={{ color: 'red' }} content='...' />
</Header>

@codecov
Copy link

codecov bot commented Aug 9, 2018

Codecov Report

Merging #72 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #72      +/-   ##
==========================================
+ Coverage   85.89%   85.97%   +0.07%     
==========================================
  Files          74       76       +2     
  Lines        1106     1112       +6     
  Branches      219      228       +9     
==========================================
+ Hits          950      956       +6     
  Misses        149      149              
  Partials        7        7
Impacted Files Coverage Δ
src/components/Header/headerVariables.ts 100% <100%> (ø)
...rc/components/Header/headerDescriptionVariables.ts 100% <100%> (ø)
...teams/components/Header/headerDescriptionStyles.ts 100% <100%> (ø) ⬆️
src/themes/teams/components/Header/headerStyles.ts 100% <100%> (ø) ⬆️
src/components/Header/HeaderDescription.tsx 100% <100%> (ø) ⬆️
src/components/Header/Header.tsx 95.65% <100%> (+0.41%) ⬆️

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 a38523e...30eba36. Read the comment docs.

@@ -34,16 +35,29 @@ class Header extends UIComponent<any, any> {

/** Align header content. */
textAlign: PropTypes.oneOf(['left', 'center', 'right', 'justified']),

/** Custom values for styling variables. */
variables: PropTypes.object,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it necessary to add the variables prop? We don't have it added in any of the other components.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, this thing turns out to be necessary for the sake of preventing variables being passed further to the child DOM tree - otherwise we can end up with the following thing:

image

For the sake of preventing this to happen the variables property has been declared as handled.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it! We should address this in the other components as well. Approving this PR.

@kuzhelov kuzhelov merged commit a0f46e3 into master Aug 9, 2018
@kuzhelov kuzhelov deleted the feat/introduce-color-customization-to-header-and-description branch August 9, 2018 14:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants