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

feat(Text): Redlining text component for Teams dark & contrast themes #317

Merged
merged 14 commits into from
Oct 10, 2018

Conversation

codepretty
Copy link
Collaborator

@codepretty codepretty commented Oct 4, 2018

Updating the colors for Text component for Teams dark & contrast themes.

Dark theme
image

Contrast theme
image

@codecov
Copy link

codecov bot commented Oct 4, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #317   +/-   ##
=======================================
  Coverage   89.73%   89.73%           
=======================================
  Files          64       64           
  Lines        1237     1237           
  Branches      180      180           
=======================================
  Hits         1110     1110           
  Misses        125      125           
  Partials        2        2

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 02fd238...0b8917d. Read the comment docs.

@codepretty codepretty changed the title [WIP] Text component redline matching for Teams dark & contrast themes feat(Text): Redlining text component for Teams dark & contrast themes Oct 4, 2018
successTextColor: siteVariables.green04,
timestampTextColor: siteVariables.gray04,
timestampHoverTextColor: siteVariables.gray02,
importantTextColor: siteVariables.red,
Copy link
Contributor

Choose a reason for hiding this comment

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

can these either be sorted alphabetically or the same as each other?

Copy link
Member

Choose a reason for hiding this comment

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

Let's also remove the duplicate Text word in each. Variables are already namespaced by display name:

theme.componentVariables.Text().successColor


//
// COLORS
//
export const black = '#000'

export const hyperlinkColor = '#ffff01'
export const disabledColor = '#3ff23f'
export const selectionBackgroundColor = '#1aebff'
Copy link
Contributor

Choose a reason for hiding this comment

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

these may be older colors or maybe I have older color redlines... (sending image via teams)


//
// COLORS
//
export const black = '#000'

export const hyperlinkColor = '#ffff01'
export const disabledColor = '#3ff23f'
Copy link
Member

Choose a reason for hiding this comment

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

This feels too generic to be useful in all color contexts. How can we have a single color for "disabled" without knowing if it is for a foreground, background, border, etc or which component it is for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can rename to accessibleYellow and accessibleGreen for now. Although, maybe it should just be a general green and yellow option, like yellow0x and green0x...

@levithomason levithomason mentioned this pull request Oct 5, 2018
25 tasks
@codepretty codepretty merged commit 6bb9d80 into master Oct 10, 2018
@codepretty codepretty deleted the feat/dark-contrast-redlines-text branch October 10, 2018 18:47
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.

5 participants