Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/roosterjs-content-model-core/lib/editor/Editor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,8 @@ export class Editor implements IEditor {
core.darkColorHandler,
{
tableBorders: this.isExperimentalFeatureEnabled('TransformTableBorderColors'),
}
},
core.format.defaultFormat.textColor
);

core.lifecycle.isDarkMode = !!isDarkMode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,8 @@ describe('Editor', () => {
mockedColorHandler,
{
tableBorders: false,
}
},
undefined
);
expect(mockedCore.lifecycle.isDarkMode).toEqual(true);
expect(triggerEventSpy).toHaveBeenCalledTimes(1);
Expand All @@ -1041,7 +1042,8 @@ describe('Editor', () => {
mockedColorHandler,
{
tableBorders: false,
}
},
undefined
);
expect(triggerEventSpy).toHaveBeenCalledTimes(2);
expect(triggerEventSpy).toHaveBeenCalledWith(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,34 @@ export function transformColor(
includeSelf: boolean,
direction: 'lightToDark' | 'darkToLight',
darkColorHandler?: DarkColorHandler,
transformColorOptions?: TransformColorOptions
transformColorOptions?: TransformColorOptions,
defaultTextColor?: string
) {
Comment on lines 30 to 35
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transformColor now has a new defaultTextColor parameter (used to propagate inherited text color for background contrast), but the exported function's documentation block above doesn't describe it. Please update the JSDoc to include this parameter and its effect on background color transformation.

Copilot uses AI. Check for mistakes.
const toDarkMode = direction == 'lightToDark';
const tableBorders = transformColorOptions?.tableBorders || false;
const transformer = (element: HTMLElement) => {
const transformer = (element: HTMLElement, parentTextColor?: string) => {
const textColor = getColor(element, false /*isBackground*/, !toDarkMode, darkColorHandler);
const backColor = getColor(element, true /*isBackground*/, !toDarkMode, darkColorHandler);
const comparingColor = textColor || parentTextColor;

setColor(element, textColor, false /*isBackground*/, toDarkMode, darkColorHandler);
setColor(element, backColor, true /*isBackground*/, toDarkMode, darkColorHandler);
setColor(
element,
backColor,
true /*isBackground*/,
toDarkMode,
darkColorHandler,
comparingColor
);

if (tableBorders) {
transformBorderColor(element, toDarkMode, darkColorHandler);
}

return comparingColor;
};

iterateElements(rootNode, transformer, includeSelf);
iterateElements(rootNode, transformer, includeSelf, defaultTextColor);
}

function transformBorderColor(
Expand Down Expand Up @@ -79,19 +90,22 @@ function transformBorderColor(

function iterateElements(
root: Node,
transformer: (element: HTMLElement) => void,
includeSelf?: boolean
transformer: (element: HTMLElement, parentTextColor?: string) => string | undefined,
includeSelf?: boolean,
parentTextColor?: string
) {
if (includeSelf && isHTMLElement(root)) {
transformer(root);
parentTextColor = transformer(root, parentTextColor);
}

for (let child = root.firstChild; child; child = child.nextSibling) {
let textColor = parentTextColor;

if (isHTMLElement(child)) {
transformer(child);
textColor = transformer(child, parentTextColor);
}

iterateElements(child, transformer);
iterateElements(child, transformer, false, textColor);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { getColor, setColor } from '../utils/color';
import { shouldSetValue } from '../utils/shouldSetValue';
import type { BackgroundColorFormat } from 'roosterjs-content-model-types';
import type { BackgroundColorFormat, TextColorFormat } from 'roosterjs-content-model-types';
import type { FormatHandler } from '../FormatHandler';

/**
* @internal
*/
export const backgroundColorFormatHandler: FormatHandler<BackgroundColorFormat> = {
export const backgroundColorFormatHandler: FormatHandler<
BackgroundColorFormat & TextColorFormat
> = {
Comment on lines +9 to +11
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

backgroundColorFormatHandler is now typed as FormatHandler<BackgroundColorFormat & TextColorFormat>, but the default format handler registry expects FormatHandler<BackgroundColorFormat> (see FormatHandlerTypeMap.backgroundColor). This can cause a TypeScript assignability error (or force callers/tests to widen types) even though textColor is only used as an optional hint.

Suggested fix: keep the handler typed as FormatHandler<BackgroundColorFormat> and read comparing text color via a safe cast (e.g., const comparingColor = (format as Partial<TextColorFormat>).textColor ?? context.implicitFormat.textColor) when calling setColor.

Copilot uses AI. Check for mistakes.
parse: (format, element, context, defaultStyle) => {
const backgroundColor =
getColor(
Expand Down Expand Up @@ -34,7 +36,8 @@ export const backgroundColorFormatHandler: FormatHandler<BackgroundColorFormat>
format.backgroundColor,
true /*isBackground*/,
!!context.isDarkMode,
context.darkColorHandler
context.darkColorHandler,
format.textColor
);
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,12 @@ export function getLightModeColor(
) {
if (DeprecatedColors.indexOf(color) > -1) {
return fallback;
} else if (darkColorHandler) {
} else {
const match = color.startsWith(VARIABLE_PREFIX) ? VARIABLE_REGEX.exec(color) : null;

if (match) {
color = match[2] || '';
} else if (isDarkMode) {
} else if (isDarkMode && darkColorHandler) {
// If editor is in dark mode but the color is not in dark color format, it is possible the color was inserted from external code
// without any light color info. So we first try to see if there is a known dark color can match this color, and use its related
// light color as light mode color. Otherwise we need to drop this color to avoid show "white on white" content.
Expand Down Expand Up @@ -126,20 +126,23 @@ export function retrieveElementColor(
* @param isBackground True to set background color, false to set text color
* @param isDarkMode Whether element is in dark mode now
* @param darkColorHandler @optional The dark color handler object to help manager dark mode color
* @param comparingColor @optional When generating dark color for background color, we can provide text color as comparingColor to make sure the generated dark border color has enough contrast with text color in dark mode
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSDoc for setColor's new comparingColor param mentions "generated dark border color", but this param is passed when generating dark background colors (to keep contrast with text). Please correct the wording so it matches the actual behavior.

Suggested change
* @param comparingColor @optional When generating dark color for background color, we can provide text color as comparingColor to make sure the generated dark border color has enough contrast with text color in dark mode
* @param comparingColor @optional When generating dark color for a background, we can provide the text color as comparingColor to make sure the generated dark background color has enough contrast with the text color in dark mode

Copilot uses AI. Check for mistakes.
*/
export function setColor(
element: HTMLElement,
color: string | null | undefined,
isBackground: boolean,
isDarkMode: boolean,
darkColorHandler?: DarkColorHandler
darkColorHandler?: DarkColorHandler,
comparingColor?: string
) {
const newColor = adaptColor(
element,
color,
isBackground ? 'background' : 'text',
isDarkMode,
darkColorHandler
darkColorHandler,
comparingColor
);

element.removeAttribute(isBackground ? 'bgcolor' : 'color');
Expand All @@ -154,7 +157,8 @@ export function adaptColor(
color: string | null | undefined,
colorType: 'text' | 'background' | 'border',
isDarkMode: boolean,
darkColorHandler?: DarkColorHandler
darkColorHandler?: DarkColorHandler,
comparingColor?: string
) {
const match = color && color.startsWith(VARIABLE_PREFIX) ? VARIABLE_REGEX.exec(color) : null;
const [_, existingKey, fallbackColor] = match ?? [];
Expand All @@ -164,10 +168,22 @@ export function adaptColor(
if (darkColorHandler && color) {
const key =
existingKey ||
darkColorHandler.generateColorKey(color, undefined /*baseLValue*/, colorType, element);
darkColorHandler.generateColorKey(
color,
undefined /*baseLValue*/,
colorType,
element,
comparingColor
);
const darkModeColor =
darkColorHandler.knownColors?.[key]?.darkModeColor ||
darkColorHandler.getDarkColor(color, undefined /*baseLValue*/, colorType, element);
darkColorHandler.getDarkColor(
color,
undefined /*baseLValue*/,
colorType,
element,
comparingColor
);

darkColorHandler.updateKnownColor(isDarkMode, key, {
lightModeColor: color,
Expand All @@ -185,8 +201,15 @@ export function adaptColor(
* @param lightColor The input light color
* @returns Key of the color
*/
export const defaultGenerateColorKey: ColorTransformFunction = lightColor => {
return `${COLOR_VAR_PREFIX}_${lightColor.replace(/[^\d\w]/g, '_')}`;
export const defaultGenerateColorKey: ColorTransformFunction = (
lightColor,
_1,
_2,
_3,
comparingColor
) => {
const comparingColorKey = comparingColor ? `_${comparingColor.replace(/[^\d\w]/g, '_')}` : '';
return `${COLOR_VAR_PREFIX}_${lightColor.replace(/[^\d\w]/g, '_')}${comparingColorKey}`;
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ describe('transform to dark mode', () => {

runTest(
element,
'<div style="color: var(--darkColor_red, red); background-color: var(--darkColor_green, green);"></div>',
'<div style="--darkColor_red: blue; --darkColor_green: yellow;"></div>'
'<div style="color: var(--darkColor_red, red); background-color: var(--darkColor_green_red, green);"></div>',
'<div style="--darkColor_red: blue; --darkColor_green_red: yellow;"></div>'
);
});

Expand All @@ -44,8 +44,8 @@ describe('transform to dark mode', () => {

runTest(
element,
'<div style="color: var(--darkColor_red, red); background-color: var(--darkColor_green, green);"></div>',
'<div style="--darkColor_red: blue; --darkColor_green: yellow;"></div>'
'<div style="color: var(--darkColor_red, red); background-color: var(--darkColor_green_red, green);"></div>',
'<div style="--darkColor_red: blue; --darkColor_green_red: yellow;"></div>'
);
});

Expand All @@ -58,8 +58,8 @@ describe('transform to dark mode', () => {

runTest(
element,
'<div style="color: var(--darkColor_red, red); background-color: var(--darkColor_green, green);"></div>',
'<div style="--darkColor_red: blue; --darkColor_green: yellow;"></div>'
'<div style="color: var(--darkColor_red, red); background-color: var(--darkColor_green_red, green);"></div>',
'<div style="--darkColor_red: blue; --darkColor_green_red: yellow;"></div>'
);
});

Expand Down Expand Up @@ -98,8 +98,8 @@ describe('transform to dark mode', () => {

runTest(
element,
'<div style="color: var(--darkColor_red, red); background-color: var(--darkColor_green, green); border-top: 1px solid red; border-bottom: 2px dashed green;"></div>',
'<div style="--darkColor_red: blue; --darkColor_green: yellow;"></div>'
'<div style="color: var(--darkColor_red, red); background-color: var(--darkColor_green_red, green); border-top: 1px solid red; border-bottom: 2px dashed green;"></div>',
'<div style="--darkColor_red: blue; --darkColor_green_red: yellow;"></div>'
);
});

Expand Down Expand Up @@ -142,6 +142,33 @@ describe('transform to dark mode', () => {
'<div style="--darkColor_red: blue; --darkColor_green: yellow;"></div>'
);
});

it('Has text color on parent element and background color on child element, transform with dark color handler', () => {
const element = document.createElement('div');
element.style.color = 'red';

const child1 = document.createElement('div');
child1.style.color = 'green';
element.appendChild(child1);

const span1 = document.createElement('span');
span1.style.backgroundColor = 'red';
child1.appendChild(span1);

const child2 = document.createElement('div');
child2.style.color = 'yellow';
element.appendChild(child2);

const span2 = document.createElement('span');
span2.style.backgroundColor = 'gray';
child2.appendChild(span2);

runTest(
element,
'<div style="color: var(--darkColor_red, red);"><div style="color: var(--darkColor_green, green);"><span style="background-color: var(--darkColor_red_green, red);"></span></div><div style="color: var(--darkColor_yellow, yellow);"><span style="background-color: var(--darkColor_gray_yellow, gray);"></span></div></div>',
'<div style="--darkColor_red: blue; --darkColor_green: yellow; --darkColor_red_green: blue;"></div>'
);
});
});

describe('transform to light mode', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
BackgroundColorFormat,
DomToModelContext,
ModelToDomContext,
TextColorFormat,
} from 'roosterjs-content-model-types';

describe('backgroundColorFormatHandler.parse', () => {
Expand Down Expand Up @@ -84,7 +85,7 @@ describe('backgroundColorFormatHandler.parse', () => {
describe('backgroundColorFormatHandler.apply', () => {
let div: HTMLElement;
let context: ModelToDomContext;
let format: BackgroundColorFormat;
let format: BackgroundColorFormat & TextColorFormat;

beforeEach(() => {
div = document.createElement('div');
Expand Down Expand Up @@ -122,4 +123,23 @@ describe('backgroundColorFormatHandler.apply', () => {

expectHtml(div.outerHTML, expectedResult);
});

it('Has both text and background color in dark mode', () => {
format.backgroundColor = 'red';
format.textColor = 'green';
context.isDarkMode = true;
context.darkColorHandler = {
updateKnownColor: () => {},
getDarkColor: (lightColor: string) => `var(--darkColor_${lightColor}, ${lightColor})`,
generateColorKey: defaultGenerateColorKey,
} as any;

backgroundColorFormatHandler.apply(format, div, context);

const expectedResult = [
'<div style="background-color: var(--darkColor_red_green, red);"></div>',
];

expectHtml(div.outerHTML, expectedResult);
});
});
Loading
Loading