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(material-experimental/mdc-checkbox): fix broken dark theme #20940

Merged
merged 1 commit into from Nov 2, 2020

Conversation

mmalerba
Copy link
Contributor

This is a re-submit of #20926 with a fix for the focus state

@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 30, 2020
@mmalerba mmalerba added P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful target: patch This PR is targeted for the next patch release labels Oct 30, 2020
Copy link
Contributor

@wagnermaciel wagnermaciel left a comment

Choose a reason for hiding this comment

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

lgtm

@wagnermaciel wagnermaciel added the action: merge The PR is ready for merge by the caretaker label Oct 30, 2020
@crisbeto
Copy link
Member

What is broken in the current dark theme styles? It seemed to work for me in the dev app.

@wagnermaciel
Copy link
Contributor

What is broken in the current dark theme styles? It seemed to work for me in the dev app.

We had a screenshot failure from the previous change where the focus state of a checkbox was not the correct color (I think it was darker than it was supposed to be). I'm not super familiar with mdc-checkbox so I don't fully understand why, but the changes in this PR have fixed the issue. I'm working on getting it submitted as quickly as possible

@mmalerba
Copy link
Contributor Author

I forgot to include the styles to re-color the ripple for primary, accent, warn states - so the ripple was always gray. Fixed it in this PR

@crisbeto
Copy link
Member

crisbeto commented Oct 31, 2020

@wagnermaciel I meant to ask what issue was the original PR fixing, before we had to revert it?

@wagnermaciel wagnermaciel merged commit 3e9a26c into angular:master Nov 2, 2020
wagnermaciel pushed a commit that referenced this pull request Nov 2, 2020
wagnermaciel pushed a commit that referenced this pull request Nov 2, 2020
@wagnermaciel
Copy link
Contributor

@crisbeto I'm not sure. @mmalerba might have the answer you're looking for

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants