-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
a11y high contrast mode and focus state bug fixes for web components #28717
Changes from all commits
7f9af9e
70c55e0
f3cb7f4
a72d10f
2941fa7
9b350fc
2cf59ce
79b6bff
5558b93
3c045c3
2370967
9c4eb6b
056b82e
196ed30
2a9ae23
677a59c
17819f9
dfe1311
dd472a4
d1b6af5
658f7f8
4af2957
3d50372
d6ffb3a
7bc3824
27a2c1e
d9cf80e
48220af
26f8be1
44082c1
b9d268d
2fb2cfa
22323ce
1533572
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "prerelease", | ||
"comment": "(accessibility): high contrast fixes", | ||
"packageName": "@fluentui/web-components", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import { css } from '@microsoft/fast-element'; | ||
import { display } from '@microsoft/fast-foundation'; | ||
import { display, forcedColorsStylesheetBehavior } from '@microsoft/fast-foundation'; | ||
import { | ||
borderRadiusCircular, | ||
borderRadiusLarge, | ||
|
@@ -308,4 +308,10 @@ export const styles = css` | |
color: ${colorNeutralForegroundDisabled}; | ||
cursor: not-allowed; | ||
} | ||
`; | ||
`.withBehaviors( | ||
forcedColorsStylesheetBehavior(css` | ||
:host([appearance='transparent']:hover) .control { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @brianchristopherbrady what is the reason we need both of these? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
border-color: Highlight; | ||
} | ||
`), | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import { css } from '@microsoft/fast-element'; | ||
import { display } from '@microsoft/fast-foundation'; | ||
import { display, forcedColorsStylesheetBehavior } from '@microsoft/fast-foundation'; | ||
import { | ||
borderRadiusCircular, | ||
borderRadiusSmall, | ||
|
@@ -127,4 +127,16 @@ export const styles = css` | |
:host([disabled]) .checked-indicator { | ||
background: ${colorNeutralForegroundDisabled}; | ||
} | ||
`; | ||
`.withBehaviors( | ||
forcedColorsStylesheetBehavior(css` | ||
:host .control { | ||
border-color: InactiveBorder; | ||
} | ||
:host([aria-checked='true']) .checked-indicator, | ||
:host([aria-checked='true']:active) .checked-indicator, | ||
:host([aria-checked='true']:hover) .checked-indicator { | ||
background-color: Highlight; | ||
border-color: ActiveBorder; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another instance where it's odd for active border to be on the non checked. |
||
`), | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import { css } from '@microsoft/fast-element'; | ||
import { display } from '@microsoft/fast-foundation'; | ||
import { display, forcedColorsStylesheetBehavior } from '@microsoft/fast-foundation'; | ||
import { | ||
borderRadiusCircular, | ||
colorCompoundBrandBackground, | ||
|
@@ -18,18 +18,22 @@ import { | |
colorNeutralStrokeAccessibleHover, | ||
colorNeutralStrokeAccessiblePressed, | ||
colorNeutralStrokeDisabled, | ||
colorStrokeFocus2, | ||
colorTransparentBackground, | ||
colorTransparentStroke, | ||
curveEasyEase, | ||
durationNormal, | ||
fontFamilyBase, | ||
fontSizeBase300, | ||
fontWeightRegular, | ||
lineHeightBase300, | ||
shadow4, | ||
spacingHorizontalS, | ||
spacingHorizontalXS, | ||
spacingHorizontalXXS, | ||
spacingVerticalS, | ||
spacingVerticalXS, | ||
strokeWidthThick, | ||
} from '../theme/design-tokens.js'; | ||
|
||
export const styles = css` | ||
|
@@ -142,4 +146,32 @@ export const styles = css` | |
:host([aria-checked='true'][disabled]) .checked-indicator { | ||
background: ${colorNeutralForegroundDisabled}; | ||
} | ||
`; | ||
|
||
:host(:focus-visible) { | ||
border-color: ${colorTransparentStroke}; | ||
outline: ${strokeWidthThick} solid ${colorTransparentStroke}; | ||
box-shadow: ${shadow4}, 0 0 0 2px ${colorStrokeFocus2}; | ||
} | ||
`.withBehaviors( | ||
forcedColorsStylesheetBehavior(css` | ||
.switch { | ||
border-color: InactiveBorder; | ||
} | ||
:host([aria-checked='true']) .switch, | ||
:host([aria-checked='true']:active) .switch, | ||
:host([aria-checked='true']:hover) .switch { | ||
background: Highlight; | ||
border-color: Highlight; | ||
} | ||
.checked-indicator, | ||
:host(:hover) .checked-indicator, | ||
:host(:active) .checked-indicator { | ||
background-color: ActiveCaption; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Likewise can these also be a single selector? Let's look for duplication like this elsewhere and fix as well please :) |
||
:host([aria-checked='true']) .checked-indicator, | ||
:host([aria-checked='true']:hover) .checked-indicator, | ||
:host([aria-checked='true']:active) .checked-indicator { | ||
background-color: ButtonFace; | ||
} | ||
`), | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import { css } from '@microsoft/fast-element'; | ||
import { forcedColorsStylesheetBehavior } from '@microsoft/fast-foundation'; | ||
import { styles as ButtonStyles } from '../button/button.styles.js'; | ||
import { | ||
colorBrandBackgroundHover, | ||
|
@@ -105,4 +106,16 @@ export const styles = css` | |
:host([aria-pressed='true'][appearance='transparent']:active) .control { | ||
color: ${colorNeutralForeground2BrandPressed}; | ||
} | ||
`; | ||
`.withBehaviors( | ||
forcedColorsStylesheetBehavior(css` | ||
:host([aria-pressed='true']) .control, | ||
:host([aria-pressed='true'][appearance='primary']) .control, | ||
:host([aria-pressed='true'][appearance='subtle']) .control, | ||
:host([aria-pressed='true'][appearance='outline']) .control, | ||
:host([aria-pressed='true'][appearance='transparent']) .control, | ||
:host([aria-pressed='true'][appearance='transparent']) .control { | ||
background: SelectedItem; | ||
color: SelectedItemText; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Combine with the first block which shares the same styles |
||
`), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🕵 fluentui-web-components-v3 Open the Visual Regressions report to inspect the affected screenshots
Accordion 9 screenshots