Allow theming with arbitrary CSS color strings #1346
Conversation
|
Really nice PR, this is addressing one of the bigger areas of concern, thanks! If I'm reading the code correctly, passing alpha values like There's also one test failling: |
| @@ -2233,38 +2233,6 @@ function wasMondifierKeyOnlyEvent(ev: KeyboardEvent): boolean { | |||
| * ANSI color code. | |||
| */ | |||
|
|
|||
| // Colors 0-15 + 16-255 | |||
| // Much thanks to TooTallNate for writing this. | |||
| const vcolors: number[][] = (function(): number[][] { | |||
There was a problem hiding this comment.
I've wanted this gone for a long time 😛
|
This looks great, I haven't tested the code but if everything works fine and the tests pass then I'm 👍 |
|
I've fixed the test, and amended the commit!
This parses alpha channels.
Selections work as expected. They're partially transparent. |
src/renderer/ColorManager.test.ts
Outdated
| @@ -17,261 +17,272 @@ describe('ColorManager', () => { | |||
| dom = new jsdom.JSDOM(''); | |||
| window = dom.window; | |||
| document = window.document; | |||
| (<any>window).HTMLCanvasElement.prototype.getContext = () => ({ | |||
There was a problem hiding this comment.
It might make sense to pull in node-canvas (which JSDom integrates with) as a dev dependency if we want to test more rendering-related codepaths, but this works for now.
There was a problem hiding this comment.
I want to improve the testing situation, I was thinking about puppeteer but I'm not sure if that's the right tool for the job thinking about it some more. #1247
This mostly worked before, but clearColor made the assumption that the colors were always in #RRBBGG format. See the discussion here: xtermjs#1327 (comment) This changes the internal representation of a color so that we hold onto into an object containing the original css string along with an RGBA value encoded as a 32-bit uint. clearColor can then use that RGBA representation. Additionally, this deduplicates some code between ColorManager and Terminal. Terminal was generating the 256 ansi colors the same way ColorManager was, so this just makes Terminal use the same constant.
Fixes xterm/xterm.js#1357.
|
I merged bgw#2 (Block transparent colors without allowTransparency) into this PR, and I'm not aware of any outstanding issues, so I think this is ready to be merged. I'll do the node-canvas stuff as a separate PR once this stuff lands. |
|
Thanks @bgw 👍 |
This mostly worked before, but
clearColormade the assumption that the colors were always in#RRGGBBformat. See the discussion here: #1327 (comment)This changes the internal representation of a color so that we hold onto into an object containing the original css string along with an RGBA value encoded as a 32-bit uint.
clearColorcan then use that RGBA representation.Additionally, this deduplicates some code between
ColorManagerandTerminal.Terminalwas generating the 256 ansi colors the same wayColorManagerwas, so this just makesTerminaluse the same constant.256 colors still works:
and so does the 24-bit color approximation:
The static char atlas looks sane, so
clearColoris working:We can change the theme using arbitrary css strings:
And
clearColorstill works properly, but now using a pink background: