Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unreadable text colors #3801

Merged
merged 1 commit into from
Oct 29, 2019
Merged

Fix unreadable text colors #3801

merged 1 commit into from
Oct 29, 2019

Conversation

petemill
Copy link
Member

Fix brave/brave-browser#6573 (regression which was introduced by #3569)

Typography GetColor overrides become context specific, not global.

Instead of overriding color values, override context-specific styles. In our case, we only override button text color at the moment.

The global color value overrides were affecting text color in other circumstances, since chromium now defines the color values multiple times.

Submitter Checklist:

Test Plan:

On issue

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@petemill petemill self-assigned this Oct 28, 2019
@petemill petemill mentioned this pull request Oct 28, 2019
32 tasks
@bsclifton bsclifton added this to the 0.73.x - Nightly milestone Oct 29, 2019
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

Changes work great! 👍 Really nice change to how it's done, using GetColor instead of altering the color definitions 😄 Nice work!

Instead of overriding color values, override context-specific styles. In our case, we only override button text color at the moment.

The global styles was affected text color in other circumstances, since chromium now defines the color values multiple times
@bsclifton
Copy link
Member

CI shows as aborted; basically, agent was disconnected while in either the test_unit step or create_dist step on Windows. All builds (and all other CI) passed just fine:
https://staging.ci.brave.com/job/brave-browser-build-pr/job/fix-text-color/3/flowGraphTable/

@bsclifton bsclifton merged commit d4589d1 into master Oct 29, 2019
@bsclifton bsclifton deleted the fix-text-color branch October 29, 2019 22:53
bsclifton added a commit that referenced this pull request Oct 31, 2019
bsclifton added a commit that referenced this pull request Oct 31, 2019
bsclifton added a commit that referenced this pull request Oct 31, 2019
Uplift "Fix Unreadable Text Colors" #3801 to 0.72.x
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

white text is being used in certain area's making it hard to read when using light theme
2 participants