Skip to content

refactor(cdk/a11y): Refactor focus indicator theme mixins to match other theme mixins #22427

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 21, 2021
Merged

refactor(cdk/a11y): Refactor focus indicator theme mixins to match other theme mixins #22427

merged 1 commit into from
Apr 21, 2021

Conversation

@zelliott
Copy link
Collaborator

@zelliott zelliott commented Apr 7, 2021

No description provided.

@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Apr 7, 2021
zelliott
zelliott commented Apr 7, 2021
View reviewed changes
src/dev-app/theme.scss

// Include the dark theme colors for focus indicators.
.demo-strong-focus {
&.demo-strong-focus {
Copy link
Collaborator Author

@zelliott zelliott Apr 7, 2021

Choose a reason for hiding this comment

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

This was a bug, AFAICT.

@zelliott zelliott force-pushed the focus-indicator-color branch from 96be0c5 to 02cf798 Compare April 7, 2021 22:14
zelliott
zelliott commented Apr 7, 2021
View reviewed changes
src/material/core/focus-indicators/_focus-indicators-theme.scss
border-color: $color;
@mixin theme($theme-or-color-config) {
$theme: theming.private-legacy-get-theme($theme-or-color-config);
@include theming.private-check-duplicate-theme-styles($theme, 'mat-focus-indicators') {
Copy link
Collaborator Author

@zelliott zelliott Apr 7, 2021

Choose a reason for hiding this comment

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

Note: The id to private-check-duplicate-theme-styles was changed from mat-strong-focus-indicators to mat-focus-indicators to address a linter error I was getting:

Missing check for duplicative theme styles. material/theme-mixin-api
Please include the duplicate styles check mixin: theming.private-check-duplicate-theme-styles($theme, 'mat-focus-indicators')

jelbourn
jelbourn approved these changes Apr 13, 2021
View reviewed changes
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

lgtm

@jelbourn jelbourn added target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker labels Apr 13, 2021
@wagnermaciel
Copy link
Contributor

wagnermaciel commented Apr 19, 2021

@zelliott Sorry for the delay here. Could you resolve these merge conflicts for us?

@zelliott zelliott force-pushed the focus-indicator-color branch from cb7a528 to a83c0bf Compare April 19, 2021 19:39
@zelliott zelliott requested a review from andrewseguin as a code owner April 19, 2021 21:10
@google-cla
Copy link

google-cla bot commented Apr 19, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Apr 19, 2021
@zelliott zelliott removed the request for review from andrewseguin April 19, 2021 21:11
@zelliott zelliott force-pushed the focus-indicator-color branch from e668309 to 66412b5 Compare April 19, 2021 21:13
@google-cla google-cla bot added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Apr 19, 2021
@zelliott
refactor(cdk/a11y): Refactor focus indicator theme mixins to match ot…
331ced4 
…her theme mixins

refactor(cdk/a11y): Fix lint warnings

cdk(refactor/a11y): Fix lint warnings
@zelliott zelliott force-pushed the focus-indicator-color branch from 66412b5 to 331ced4 Compare April 19, 2021 21:14
@wagnermaciel wagnermaciel added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Apr 21, 2021
@wagnermaciel wagnermaciel merged commit 43db4f5 into angular:master Apr 21, 2021
@wagnermaciel
Copy link
Contributor

wagnermaciel commented Apr 21, 2021

@zelliott I changed the target label to minor because there are merge conflicts between these changes and 11.2.x. If you'd like these changes to also be included in 11.2.x and 12.0.x, please let me know and I will create separate PRs for that. Otherwise, if you're ok with this being target: minor we can leave this as is.

@angular-automatic-lock-bot
Copy link

angular-automatic-lock-bot bot commented May 22, 2021

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 May 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jelbourn jelbourn jelbourn approved these changes

@devversion devversion Awaiting requested review from devversion

@mmalerba mmalerba Awaiting requested review from mmalerba

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: minor This PR is targeted for the next minor release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zelliott @wagnermaciel @jelbourn