Treat "initial" as black for border color#3313
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts table border color parsing so that when pasted content uses the CSS keyword initial for a border color, the system treats it as a concrete default color (black) instead of effectively “no color”, preserving intended borders during paste/transform.
Changes:
- Default
border*-color: initialto#000000duringborderColorFormatHandler.parse(). - Refactor
apply()slightly to reuse the computed border color property key. - Add a unit test covering the new
initialparsing behavior (light mode).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/roosterjs-content-model-dom/lib/formatHandlers/common/borderColorFormatHandler.ts | Adds defaulting behavior for initial border color and minor apply refactor. |
| packages/roosterjs-content-model-dom/test/formatHandlers/common/borderColorFormatHandlerTest.ts | Adds a parse test asserting initial becomes black. |
Comments suppressed due to low confidence (1)
packages/roosterjs-content-model-dom/lib/formatHandlers/common/borderColorFormatHandler.ts:53
- When
context.isDarkModeis true and adarkColorHandleris present,getLightModeColor('#000000', true, handler)can return an empty string (it treats unknown non-var colors as potentially dark colors and drops them). For the'initial'case this would defeat the intent (defaulting to black) and may result in a border value without a color. Consider bypassinggetLightModeColorfor the'initial'keyword (or call it withisDarkMode=false/withoutdarkColorHandler) so the default black is preserved regardless of current mode.
const color = retrieveElementColor(element, key);
const borderColor = color == 'initial' ? DEFAULT_COLOR : color;
if (borderColor) {
const lightModeColor = getLightModeColor(
borderColor,
!!context.isDarkMode,
context.darkColorHandler
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| expect(colorUtils.retrieveElementColor).toHaveBeenCalledWith(div, 'borderTop'); | ||
| expect(colorUtils.getLightModeColor).toHaveBeenCalledWith('#000000', false, undefined); | ||
| expect(format.borderTop).toBe('1px solid #000000'); | ||
| }); |
There was a problem hiding this comment.
The new "initial" parsing behavior is only covered in light mode. Adding a dark-mode test (with context.isDarkMode = true and a darkColorHandler present) would help catch regressions where the default black gets dropped/altered by getLightModeColor logic in dark mode.
| }); | |
| }); | |
| it('Parse border with initial color in dark mode - should use default color', () => { | |
| div.style.borderWidth = '1px'; | |
| div.style.borderStyle = 'solid'; | |
| div.style.borderTopColor = 'initial'; | |
| const mockDarkColorHandler = { | |
| knownColors: {}, | |
| getDarkColor: jasmine.createSpy('getDarkColor').and.returnValue('#000000'), | |
| updateKnownColor: jasmine.createSpy('updateKnownColor'), | |
| generateColorKey: jasmine.createSpy('generateColorKey').and.returnValue('--colorKeyInitial'), | |
| reset: jasmine.createSpy('reset'), | |
| }; | |
| context.isDarkMode = true; | |
| context.darkColorHandler = mockDarkColorHandler; | |
| spyOn(colorUtils, 'retrieveElementColor').and.returnValue('initial'); | |
| spyOn(colorUtils, 'getLightModeColor').and.returnValue('#000000'); | |
| borderColorFormatHandler.parse(format, div, context, {}); | |
| expect(colorUtils.retrieveElementColor).toHaveBeenCalledWith(div, 'borderTop'); | |
| expect(colorUtils.getLightModeColor).toHaveBeenCalledWith( | |
| '#000000', | |
| true, | |
| mockDarkColorHandler | |
| ); | |
| expect(format.borderTop).toBe('1px solid #000000'); | |
| }); |
When pasting a table that has border color set to "initial", use black as default color. Otherwise, the table border color handler will treat the table as it has no border color.