fix(cdk/a11y): make cdk-high-contrast work w/ emulated view encapsulation #18152

jelbourn · 2020-01-11T00:06:38Z

Say you have this in your component:

.some-element {
  @include cdk-high-contrast() {
    border: 1px solid white;
  }
}

With this change, this will output:

.cdk-high-contrast-active .some-element,
.cdk-high-contrast-active :host .some-element {
  border: 1px solid white;
}

Here, .cdk-high-contrast-active .some-element will apply in places
where encapsulation is turned off, and .cdk-high-contrast-active :host .some-element will apply in cases where encapsulation is emulated.
Neither will work in Shadow DOM (which we don't officially support).
AFAIK, Shadow DOM would need to use :host-content(), which we could
consider adding if we get an additional request.

This adds a few more bytes, but high-contrast styles tend to be pretty
limited.

Fixes #18147

crisbeto · 2020-01-11T08:38:02Z

src/cdk/a11y/_a11y.scss

@@ -35,7 +35,8 @@
  // context. We address this by nesting the selector context under .cdk-high-contrast.
  @at-root {
    $selector-context: #{&};
-    .cdk-high-contrast-#{$target} {
+    .cdk-high-contrast-#{$target},
+     .cdk-high-contrast-#{$target} :host {


I tested it out and this breaks the high contrast styles for Edge and IE for unencapsulated components. The problem is that when encapsulation is disabled, Angular leaves the :host in place which IE/Edge don't support and causes the entire selector to be invalidated. It can be worked around by outputting a separate selector, but it means that we generate double the amount of CSS:

.cdk-high-contrast-#{$target} { @content; } .cdk-high-contrast-#{$target} :host { @content; }

We could also add another parameter to the mixin that allows the consumer to tell us whether the styles are encapsulated and we can output the correct styles based on it. By default it can output both.

I'd definitely prefer to add a parameter to toggle this.

I see- I was assuming IE would ignore a pseudo-class it didn't recognize. I pushed an update that adds a param, defaulting to emitting both styles for maximum compatibility.

biolauri

LGTM, thanks for the quick PR & fix for (my|the) issue! 🙇

crisbeto · 2020-01-15T11:57:40Z

src/cdk/a11y/_a11y.scss

+///     * `on` - works for `Emulated`, `Native`, and `ShadowDom`
+///     * `off` - works for `None`
+///     * `both` - works for all encapsulation modes by emitting the CSS twice (default).
+@mixin cdk-high-contrast($target: active, $encapsulation: 'both') {


I'm not sure the both value makes sense since encapsulation is either on or off.

What would you do instead? I want the mixin to "just work" by default, even if it is emitting extra styles.

I think the thing that's throwing me off is that it's called both which sounds like it can have more than one style encapsulations at the same time. Maybe can make it on | off | undefined and output both styles if it's emitted? Another approach could be to match the naming from Angular.

I understand your point, but I think, both is fine here, because with this option, the mixin supports both general states of view/style encapsulation. So it's rather an option of which policies are supported.
Matching the naming from Angular would be complicated, as AFAIK there's no explicit naming from Angular which addresses Emulated, Native, and ShadowDom together.

undefined is not a restricted value in Sass, so null would be the way to go. But I'm feeling that both fits better.

How about 'any'?

Sure, that works too.

Changed to any

crisbeto

LGTM, but the CI is failing. It looks like not all references to both were updated.

jelbourn · 2020-01-16T23:25:10Z

That's what I get for using the in-browser editor

@include

…tion Say you have this in your component: ```scss .some-element { @include cdk-high-contrast() { border: 1px solid white; } } ``` With this change, this will output: ```scss .cdk-high-contrast-active .some-element, .cdk-high-contrast-active :host .some-element { border: 1px solid white; } ``` Here, `.cdk-high-contrast-active .some-element` will apply in places where encapsulation is turned off, and `.cdk-high-contrast-active :host .some-element` will apply in cases where encapsulation is emulated. Neither will work in Shadow DOM (which we don't officially support). AFAIK, Shadow DOM would need to use `:host-content()`, which we could consider adding if we get an additional request. This adds a few more bytes, but high-contrast styles tend to be pretty limited.

@include

…tion (#18152) * fix(cdk/a11y): make cdk-high-contrast work w/ emulated view encapsulation Say you have this in your component: ```scss .some-element { @include cdk-high-contrast() { border: 1px solid white; } } ``` With this change, this will output: ```scss .cdk-high-contrast-active .some-element, .cdk-high-contrast-active :host .some-element { border: 1px solid white; } ``` Here, `.cdk-high-contrast-active .some-element` will apply in places where encapsulation is turned off, and `.cdk-high-contrast-active :host .some-element` will apply in cases where encapsulation is emulated. Neither will work in Shadow DOM (which we don't officially support). AFAIK, Shadow DOM would need to use `:host-content()`, which we could consider adding if we get an additional request. This adds a few more bytes, but high-contrast styles tend to be pretty limited. (cherry picked from commit ea17135)

@include

…tion (angular#18152) * fix(cdk/a11y): make cdk-high-contrast work w/ emulated view encapsulation Say you have this in your component: ```scss .some-element { @include cdk-high-contrast() { border: 1px solid white; } } ``` With this change, this will output: ```scss .cdk-high-contrast-active .some-element, .cdk-high-contrast-active :host .some-element { border: 1px solid white; } ``` Here, `.cdk-high-contrast-active .some-element` will apply in places where encapsulation is turned off, and `.cdk-high-contrast-active :host .some-element` will apply in cases where encapsulation is emulated. Neither will work in Shadow DOM (which we don't officially support). AFAIK, Shadow DOM would need to use `:host-content()`, which we could consider adding if we get an additional request. This adds a few more bytes, but high-contrast styles tend to be pretty limited.

angular-automatic-lock-bot · 2020-02-29T00:03:16Z

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.}

jelbourn added P2 The issue is important to a large percentage of users, with a workaround target: major This PR is targeted for the next major release labels Jan 11, 2020

jelbourn added this to the 9.0.0 milestone Jan 11, 2020

jelbourn requested a review from devversion as a code owner January 11, 2020 00:06

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jan 11, 2020

jelbourn requested a review from crisbeto January 11, 2020 00:09

jelbourn mentioned this pull request Jan 11, 2020

cdk-high-contrast mixin not working anymore without :host selector #18147

Closed

jelbourn force-pushed the high-contrast-fix branch from d19608a to 7257817 Compare January 11, 2020 00:47

crisbeto reviewed Jan 11, 2020

View reviewed changes

jelbourn force-pushed the high-contrast-fix branch from 7257817 to f6643c9 Compare January 15, 2020 02:30

biolauri approved these changes Jan 15, 2020

View reviewed changes

crisbeto reviewed Jan 15, 2020

View reviewed changes

crisbeto approved these changes Jan 16, 2020

View reviewed changes

crisbeto added the pr: lgtm label Jan 16, 2020

jelbourn and others added 3 commits January 28, 2020 10:06

address comments

5d54002

Change "both" to "any"

3768db6

jelbourn force-pushed the high-contrast-fix branch from 993bd7e to 3768db6 Compare January 28, 2020 18:07

jelbourn added the action: merge The PR is ready for merge by the caretaker label Jan 28, 2020

jelbourn merged commit ea17135 into angular:master Jan 29, 2020

angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(cdk/a11y): make cdk-high-contrast work w/ emulated view encapsulation #18152

fix(cdk/a11y): make cdk-high-contrast work w/ emulated view encapsulation #18152

Uh oh!

jelbourn commented Jan 11, 2020

Uh oh!

crisbeto Jan 11, 2020

Uh oh!

biolauri Jan 11, 2020

Uh oh!

jelbourn Jan 15, 2020

Uh oh!

biolauri left a comment •

edited

Loading

Uh oh!

crisbeto Jan 15, 2020

Uh oh!

jelbourn Jan 16, 2020

Uh oh!

crisbeto Jan 16, 2020

Uh oh!

biolauri Jan 16, 2020

Uh oh!

jelbourn Jan 16, 2020

Uh oh!

crisbeto Jan 16, 2020

Uh oh!

jelbourn Jan 16, 2020

Uh oh!

crisbeto left a comment

Uh oh!

jelbourn commented Jan 16, 2020

Uh oh!

angular-automatic-lock-bot bot commented Feb 29, 2020

Uh oh!

Uh oh!

fix(cdk/a11y): make cdk-high-contrast work w/ emulated view encapsulation #18152

fix(cdk/a11y): make cdk-high-contrast work w/ emulated view encapsulation #18152

Uh oh!

Conversation

jelbourn commented Jan 11, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

biolauri left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crisbeto left a comment

Choose a reason for hiding this comment

Uh oh!

jelbourn commented Jan 16, 2020

Uh oh!

angular-automatic-lock-bot bot commented Feb 29, 2020

Uh oh!

Uh oh!

biolauri left a comment •

edited

Loading