A11y: Disable forced color adjustment for Monaco editor #265373
Conversation
… arrows in HC themes
|
If the changes appear safe, you can manually trigger the pipeline by commenting |
Oh great question. Based on the ticket, I was assuming the audit checked in other areas of the product for similar issues but perhaps that's wrong to assume. Do you know if there's an easy to way to audit the entire product to find where else this may be showing up? |
|
So just in search alone, there are more disabled buttons: 
I am just generally wondering what the accessibility issue is here: is it the low contrast of disabled actions in high contrast themes? |
Nice I can start there.
No, this is for Windows OS high contrast themes (not VS Code HC themes). When those themes are applied the OS switches to the system colors palette and the ticket reports that our disabled find widget buttons are using a color that makes them look enabled when they are not. This comment explains/shows an example (from Edge) of what it should look like. |
|
For reference, the disabled color is defined here: vscode/src/vs/platform/theme/common/colors/baseColors.ts Lines 17 to 20 in dc68074 One thing I do not fully understand: if Windows is turned into high contrast theme, VS Code should also change to high contrast automatically (@aeschli could comment on that). If there is an issue with our colors in high contrast themes, we need to fix the color in our high contrast theme. Anything else is just patches that will be hard to maintain... |
Ahh yeah that makes a ton of sense to me. I can get behind this. If so, I wonder if we would want / need to add something along these lines that makes explicit use of the forced colors palette?? export const disabledForeground = registerColor('disabledForeground',
{ dark: '#CCCCCC80', light: '#61616180', hcDark: '#A5A5A5', hcLight: '#7F7F7F', forcedColors: 'GrayText' },
nls.localize('disabledForeground', "Overall foreground for disabled elements. This color is only used if not overridden by a component."));Basically adding support for a "forced colors theme"? |
|
Ah, interesting, I'm also learning about the forced-color mode. In that mode, the colors set in CSS are ignored and replaced by a limited set of system colors based on the 'role' of the elements. Unfortunately it looks like our disabled element (marked with 'aria-disabled=true') are not recognized as disabled and rendered with the regular (button?) color instead of the recommended 'GrayText` color, and CSS adjustment would be necessary. May I suggest a change limited to Monaco? I think this addition is good enough for the hc themes: the Monaco editor can run with a high contrast theme without being in |
|
If the changes appear safe, you can manually trigger the pipeline by commenting |
|
If the changes appear safe, you can manually trigger the pipeline by commenting |
|
@aeschli @bpasero reverted the old CSS change and made some updates. Not quite sure if this was the direction you were thinking, but was my attempt at still having declarative color tokens inside the theme file and making sure that the color changes only get applied when forced colors is active. Bonus: Since system colors are finite, I also added typing so that autocomplete / intellisense kicks in nicely when adding new tokens in the future. Let me know what you think and if you'd like me to go a different direction instead. |
|
Oh two more things:
|
|
If the changes appear safe, you can manually trigger the pipeline by commenting |
|
If the changes appear safe, you can manually trigger the pipeline by commenting |
|
I talked to @hediet about whether we should do the same as in VS Code and define |
|
@aeschli can that work be deferred so we can fix this bug by the deadline? We could also wait until or if users ask for what you're suggesting to implement that. |
|
adding |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes high contrast color mapping issues in the find widget by adding forced colors media query handling. It ensures that when Windows high contrast themes are active, VS Code's built-in high contrast themes are used instead of the OS forcing system colors on Monaco editor components.
- Adds forced-colors media query detection to disable OS color forcing
- Applies forced-color-adjust: none to Monaco editor components when high contrast is active
hawkticehurst
changed the title
| this._onOSSchemeChanged(); | ||
| // Always rebuild the generated CSS so that the `forced-color-adjust: none` | ||
| // rule is added/removed reactively when the OS forced colors state changes. | ||
| this._updateThemeOrColorMap(); |
There was a problem hiding this comment.
Is that necessary? I think that's already triggered from_onOSSchemeChanged when needed
| // If the OS has forced-colors active, disable forced color adjustment for | ||
| // Monaco editor elements so that VS Code's built-in high contrast themes | ||
| // (hc-black / hc-light) are used instead of the OS forcing system colors. | ||
| if (mainWindow.matchMedia(`(forced-colors: active)`).matches) { |
There was a problem hiding this comment.
No need to check that, 'forced-color-adjust: none;' can be added also when forced colors are off, it has no effect then.
See how it is done for VS Code:
7 participants
Resolves microsoft/monaco-editor#4942
Resolves microsoft/monaco-editor#4945
Resolves microsoft/monaco-editor#4985
This PR disables forced color adjustment for the Monaco editor. This change matches what we do in VS Code desktop and now the editor will rely on the custom built-in high contrast themes that we ship instead.