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

Make high-contrast mode less hacky for Monaco editor in IE and Edge #3690

Closed
bgashler1 opened this issue Mar 3, 2016 · 15 comments
Closed

Make high-contrast mode less hacky for Monaco editor in IE and Edge #3690

bgashler1 opened this issue Mar 3, 2016 · 15 comments
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues themes Color theme issues windows VS Code on Windows issues
Milestone

Comments

@bgashler1
Copy link
Contributor

In our high contrast theme we are using clever CSS that sidesteps around style overrides IE and Edge do when Windows is in High Contrast Mode.

Fortunately, there's a single rule we can use in CSS now to stop IE 10+ and Edge from trying to modify our styles.

-ms-high-contrast-adjust: none

It overrides the browsers' default behavior to modify the styles to be more high-contrasty. This means we can stop using hacks like ::after and ::before elements to insert icons as non-background images (which would otherwise not be displayed as background images).

We need to test that this works well in Monaco and decide if we are ok only supporting IE 10+ as far as IE goes.

Provided we're all good, then let's replace all the hacks with natural solutions.

https://msdn.microsoft.com/en-us/library/hh771863(v=vs.85).aspx

@bgashler1 bgashler1 added accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues themes Color theme issues and removed accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues labels Mar 3, 2016
@bgashler1 bgashler1 added this to the Backlog milestone Mar 10, 2016
@bgashler1
Copy link
Contributor Author

@isidorn FYI - this is the task I'll work on once your change is in Master. I'll put this on the June Milestone.

@bgashler1 bgashler1 added the windows VS Code on Windows issues label May 18, 2016
@bgashler1 bgashler1 modified the milestones: June 2016, Backlog May 18, 2016
@dstorey
Copy link
Member

dstorey commented Jun 14, 2016

There is also the -ms-highcontrast: active media feature for a less sledge hammer approach to fixing up HC issues in IE/Edge. super useful with CSS system colours to match the user chosen HC colour settings.

@bpasero
Copy link
Member

bpasero commented Jul 9, 2016

@bgashler1 can you please give +1/-1 that we can use this solution? I would like to get rid of the hacks we have for HC theme to solve #8516

Imho it would be good to get rid of these special styles to clean up our CSS.

@bpasero bpasero modified the milestones: July 2016, Backlog Jul 9, 2016
@bpasero
Copy link
Member

bpasero commented Jul 10, 2016

@bgashler1 I verified that this CSS tricks works in IE and Edge, so I think we can use it. However, since VS Code is only ever running in a Chrome environment I think this CSS rule is not needed (unless maybe for the standalone editor). To verify that I opened Code in HC mode on windows after my change:

image

In #9023 I have a change ready that just uses the dark-theme image when we are in hc-black theme. I noticed that the standalone editor already adopted this.

@bgashler1
Copy link
Contributor Author

@bpasero yes, you're right that this is not an issue for Chrome. It's only an issue in the stand-alone editor when running on IE and Edge.

So anything that is shared code with the stand-alone editor must use one of the techniques above or else all things displayed via background-image will be removed in high contrast mode on Windows. It could really break the Monaco editor in Edge or IE on windows if high-contrast mode is enabled, because many icons would be missing.

@bpasero
Copy link
Member

bpasero commented Jul 12, 2016

@alexandrudima is there some CSS that only gets loaded in the standalone editor? If so, we should add the -ms-high-contrast-adjust: none there.

@bgashler1 note that for the release of the standalone editor, Alex was already removing all HC hacks, so the current version that is out is likely not working so well with HC mode on in Windows. However, I am not sure if this was reported already.

@bpasero bpasero added the accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues label Jul 12, 2016
@alexdima
Copy link
Member

@bgashler1
Copy link
Contributor Author

Ok, so any CSS that is shared with the standalone editor that uses background images must have the -ms-high-contrast-adjust: none on the same rule that defines the background image or above in the DOM tree. Anything with this rule, and all of its children, will then no longer lose its background images.

@alexdima
Copy link
Member

So, .monaco-editor { -ms-high-contrast-adjust: none } would do it ?

@bgashler1
Copy link
Contributor Author

bgashler1 commented Jul 13, 2016

That would do it, essentially. However, it's a "sledgehammer approach" to stop IE and Edge to mess with our styles.

If we do this, we should use JavaScript (or some other means, maybe we already are) to detect if Windows is in high-contrast mode and automatically apply the high-contrast base classes .hc-dark. Otherwise, Monaco editor will stand out like an ugly bright iframe anywhere it's used, as IE and Edge will convert the whole page to be high-contrasty except for things nested under .monaco-editor.

@bgashler1 bgashler1 assigned aeschli and unassigned bgashler1 Jul 13, 2016
@aeschli
Copy link
Contributor

aeschli commented Jul 14, 2016

@bgashler1 Not sure why you assigned this to me

@aeschli aeschli assigned bgashler1 and unassigned aeschli Jul 14, 2016
@alexdima
Copy link
Member

@bgashler1 We are not doing anything special w.r.t. to switching to hc-dark when Windows goes into high-contrast mode. We always said that is the responsibility of the embedders of the editor.

@bgashler1
Copy link
Contributor Author

@alexandrudima feel free to move to August if the team would like (see my PR comment). I'd love to have auto-recognition of high-contrast theme. I think we can use a get CSS function to see if the -ms-high-contrast: active is present. If so, we should default to the high-contrast theme. This would benefit both web-embedded Monaco and in VS Code visually and accessibility-wise.

@isidorn
Copy link
Contributor

isidorn commented Jul 26, 2016

Moving to august since @alexandrudima is on vacation and this is monaco editor related which releases do not have to be tied to our vscode releases

@alexdima
Copy link
Member

alexdima commented Aug 16, 2016

PR merged. The automatic switching of the editor to the hc-black and the automatic detection of the high contrast mode in IE/Edge will be tracked in #10576

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues themes Color theme issues windows VS Code on Windows issues
Projects
None yet
Development

No branches or pull requests

6 participants