Support shadow dom in webgl renderer #5334
Conversation
CGNonofr
force-pushed
the
webgl-renderer-shadow-dom
branch
from
306427a to
08de2e9
Compare
test/playwright/TestUtils.ts
Outdated
| await ctx.page.evaluate(` | ||
| window.unicode = new UnicodeGraphemesAddon(); | ||
| window.term.loadAddon(window.unicode); | ||
| window.term.unicode.activeVersion = '15-graphemes'; | ||
| `); | ||
| } | ||
| await ctx.page.waitForSelector('.xterm-rows'); | ||
| await ctx.page.locator('.xterm-rows').waitFor(); |
There was a problem hiding this comment.
Can you switch this change back unless it does something useful?
There was a problem hiding this comment.
Amended as well
note that the waitForSelector method comment suggests to use that syntax instead
There was a problem hiding this comment.
Good to know, don't want to be inconsistent though and instead migrate all at once
| // Remove shadow root if it exists | ||
| const newElement = element.cloneNode(false); | ||
| element.replaceWith(newElement); | ||
| element = newElement |
There was a problem hiding this comment.
Shouldn't this stuff only happen if !!useShadowDom?
There was a problem hiding this comment.
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 dispose on any existing term instance a few line above)
There was a problem hiding this comment.
(if an element has a shadow dom attached, it doesn't seem we can still continue injecting other elements in it)
CGNonofr
force-pushed
the
webgl-renderer-shadow-dom
branch
from
08de2e9 to
3ad83dd
Compare
|
ping @Tyriar ? |
|
Is there anything I can do to make it move forward? |
|
Is there still hope? It's a bit frustrating to invest time in a fix to see it sleep for months |
|
Ping @Tyriar |
|
@CGNonofr I feel you, this is what you're competing with though as I'm the person that typically merges xterm.js PRs 😅 
|
|
I would be happy to help you bring this number down to 683 (: |
There was a problem hiding this comment.
Just the one outstanding comment in #5334 (comment), then good to merge 👌
|
no worries! thanks and good luck for the other 683 tasks! |
Similar to what has been done in #3239, but for the webgl renderer
fix #5338