-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Support shadow dom in webgl renderer #5334
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
base: master
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
|
@@ -412,7 +412,7 @@ class TerminalCoreProxy { | |
} | ||
} | ||
|
||
export async function openTerminal(ctx: ITestContext, options: ITerminalOptions | ITerminalInitOnlyOptions = {}, testOptions: { loadUnicodeGraphemesAddon: boolean } = { loadUnicodeGraphemesAddon: true }): Promise<void> { | ||
export async function openTerminal(ctx: ITestContext, options: ITerminalOptions | ITerminalInitOnlyOptions = {}, { useShadowDom = false, loadUnicodeGraphemesAddon = true }: { loadUnicodeGraphemesAddon?: boolean, useShadowDom?: boolean } = { }): Promise<void> { | ||
await ctx.page.evaluate(` | ||
if ('term' in window) { | ||
try { | ||
|
@@ -423,13 +423,48 @@ export async function openTerminal(ctx: ITestContext, options: ITerminalOptions | |
// HACK: Tests may have side effects that could cause the terminal not to be removed. This | ||
// assertion catches this case early. | ||
strictEqual(await ctx.page.evaluate(`document.querySelector('#terminal-container').children.length`), 0, 'there must be no terminals on the page'); | ||
await ctx.page.evaluate(` | ||
|
||
let script = ` | ||
window.term = new window.Terminal(${JSON.stringify({ allowProposedApi: true, ...options })}); | ||
window.term.open(document.querySelector('#terminal-container')); | ||
`); | ||
let element = document.querySelector('#terminal-container'); | ||
|
||
// Remove shadow root if it exists | ||
const newElement = element.cloneNode(false); | ||
element.replaceWith(newElement); | ||
element = newElement | ||
Comment on lines
+431
to
+434
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. Shouldn't this stuff only happen if 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. No, because if the previous test was using shadow dom, the next test need to clean it no matter what (just like the call to 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. (if an element has a shadow dom attached, it doesn't seem we can still continue injecting other elements in it) |
||
`; | ||
|
||
|
||
if (useShadowDom) { | ||
script += ` | ||
const shadowRoot = element.attachShadow({ mode: "open" }); | ||
|
||
// Copy parent styles to shadow DOM | ||
const styles = Array.from(document.querySelectorAll('link[rel="stylesheet"]')); | ||
styles.forEach((styleEl) => { | ||
const clone = document.createElement('link'); | ||
clone.rel = 'stylesheet'; | ||
clone.href = styleEl.href; | ||
shadowRoot.appendChild(clone); | ||
}); | ||
|
||
// Create new element inside the shadow DOM | ||
element = document.createElement('div'); | ||
element.style.width = '100%'; | ||
element.style.height = '100%'; | ||
shadowRoot.appendChild(element); | ||
`; | ||
} | ||
|
||
script += ` | ||
window.term.open(element); | ||
`; | ||
|
||
await ctx.page.evaluate(script); | ||
|
||
// HACK: This is a soft layer breaker that's temporarily included until unicode graphemes have | ||
// more complete integration tests. See https://github.com/xtermjs/xterm.js/pull/4519#discussion_r1285234453 | ||
if (testOptions.loadUnicodeGraphemesAddon) { | ||
if (loadUnicodeGraphemesAddon) { | ||
await ctx.page.evaluate(` | ||
window.unicode = new UnicodeGraphemesAddon(); | ||
window.term.loadAddon(window.unicode); | ||
|
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.
Can you keep testOptions as it was?
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.
Then I'm not sure how to be able to:
loadUnicodeGraphemesAddon
default valueuseShadowDom
useShadowDom
parameterWithout adding some boilerplate in the function itself 🤔 what is the issue with that syntax?