fix: revert to canvas renderer for xterm #8138
Conversation
The default changed from canvas to DOM in the last update. The canvas renderer resolves line spacing issues and has better performance, as well as bringing us back to the previous status quo.
3d71f40 to
029c6d3
Compare
| @@ -38,7 +38,7 @@ Object.defineProperty(window, "TextEncoder", { | |||
| const renderTerminal = async ( | |||
| route = `/${MockUser.username}/${MockWorkspace.name}/terminal`, | |||
| ) => { | |||
| const utils = renderWithAuth(<TerminalPage />, { | |||
| const utils = renderWithAuth(<TerminalPage renderer="dom" />, { | |||
There was a problem hiding this comment.
Is this because Jest can't handle canvas elements? If so, how was this test working before? Asking because I'm curious if this canvas switch is the culprit behind #7948.
There was a problem hiding this comment.
Yup the Jest test looks for HTML elements which will only be present in the DOM renderer. Technically setting renderer here is not necessary since the default is already DOM but I added it be explicit.
Before the major bump to xterm, back when the default was canvas and before it was broken out into an addon, we would explicitly set rendererType to dom so it would have been using the DOM renderer then too.
Unfortunately I think that means #7948 is unrelated. If it was using canvas I think we would get a different error since .xterm-rows should not exist at all (we should have seen the error no xterm-rows instead).
The default changed from canvas to DOM in the last update. The canvas renderer resolves line spacing issues and has better performance, as well as bringing us back to the previous status quo.
In the new version the canvas is enabled via an addon.
Fixes #7953.