Skip to content

Support for Shadow DOM#88536

Merged
alexdima merged 18 commits intomicrosoft:masterfrom
lmvco:master
Jan 20, 2020
Merged

Support for Shadow DOM#88536
alexdima merged 18 commits intomicrosoft:masterfrom
lmvco:master

Conversation

@lmvco
Copy link
Contributor

@lmvco lmvco commented Jan 13, 2020

This PR fixes the usage of the monaco-editor-core in shadowDOM (see this issue to get more details - microsoft/monaco-editor#308).
This request is based on some changes proposed in https://github.com/paulvarache/monaco-editor-element.

This PR assumes that, when the monaco-editor-core runs in a shadow DOM, the shadow root should be assigned to window.monacoShadowRoot. It is a workaround to make it visible in all the classes/helpers that deal with the DOM (e.g. dom.ts). I would like to know how can we attach it to a Monaco instance instead of having it global.

Lastly, since some browsers do not implement all the functions needed for the ShadowDOM, I had to add some polyfills. They were added to editorDom.ts (I am not sure if it's the most suitable place)

@msftclas
Copy link

msftclas commented Jan 13, 2020

CLA assistant check
All CLA requirements met.

Copy link
Member

@alexdima alexdima left a comment

Choose a reason for hiding this comment

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

The editor should avoid creating globals or changing the prototype of globals. We need to figure out how to remove these hacks before merging.

@alexdima alexdima added this to the January 2020 milestone Jan 20, 2020
@alexdima
Copy link
Member

Thank you! ❤️

@alexdima alexdima merged commit f135502 into microsoft:master Jan 20, 2020
@tejasrivastav
Copy link
Contributor

@lmvco Thanks for the contribution.
I have one question though.
when editor is initialized it sets the aria container on the document.body.

Would that cause any accessibility issue ?

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants