From c4a53dfbdfbce044801d8e434aa43ba7293349c8 Mon Sep 17 00:00:00 2001 From: Jiuqing Song Date: Mon, 16 Oct 2023 09:33:41 -0700 Subject: [PATCH] Standalone editor: Decouple DarkColorHandler (#2144) --- .../lib/formatHandlers/utils/color.ts | 45 +++----------- .../test/formatHandlers/utils/colorTest.ts | 62 ++++++------------- .../lib/context/ColorManager.ts | 36 +++++++++++ .../lib/context/EditorContext.ts | 6 +- .../lib/index.ts | 1 + 5 files changed, 68 insertions(+), 82 deletions(-) create mode 100644 packages-content-model/roosterjs-content-model-types/lib/context/ColorManager.ts diff --git a/packages-content-model/roosterjs-content-model-dom/lib/formatHandlers/utils/color.ts b/packages-content-model/roosterjs-content-model-dom/lib/formatHandlers/utils/color.ts index 88a2e0f9364..5a24444305f 100644 --- a/packages-content-model/roosterjs-content-model-dom/lib/formatHandlers/utils/color.ts +++ b/packages-content-model/roosterjs-content-model-dom/lib/formatHandlers/utils/color.ts @@ -1,4 +1,4 @@ -import type { DarkColorHandler } from 'roosterjs-editor-types'; +import type { ColorManager } from 'roosterjs-content-model-types'; /** * List of deprecated colors @@ -35,26 +35,20 @@ export const DeprecatedColors: string[] = [ export function getColor( element: HTMLElement, isBackground: boolean, - darkColorHandler: DarkColorHandler | undefined | null, + darkColorHandler: ColorManager | undefined, isDarkMode: boolean ): string | undefined { - let color: string | undefined; - - if (!color) { - color = - (darkColorHandler && - tryGetFontColor(element, isDarkMode, darkColorHandler, isBackground)) || - (isBackground ? element.style.backgroundColor : element.style.color) || - element.getAttribute(isBackground ? 'bgcolor' : 'color') || - undefined; - } + let color = + (isBackground ? element.style.backgroundColor : element.style.color) || + element.getAttribute(isBackground ? 'bgcolor' : 'color') || + undefined; if (color && DeprecatedColors.indexOf(color) > -1) { color = undefined; } if (darkColorHandler) { - color = darkColorHandler.parseColorValue(color).lightModeColor; + color = darkColorHandler.parseColorValue(color, isDarkMode).lightModeColor; } return color; @@ -67,7 +61,7 @@ export function setColor( element: HTMLElement, lightModeColor: string, isBackground: boolean, - darkColorHandler: DarkColorHandler | undefined | null, + darkColorHandler: ColorManager | undefined, isDarkMode: boolean ) { const effectiveColor = darkColorHandler @@ -80,26 +74,3 @@ export function setColor( element.style.color = effectiveColor; } } - -/** - * There is a known issue that when input with IME in Chrome, it is possible Chrome insert a new FONT tag with colors. - * If editor is in dark mode, this color will cause the FONT tag doesn't have light mode color info so that after convert - * to light mode the color will be wrong. - * To workaround it, we check if this is a known color (for light mode with VariableBasedDarkColor enabled, all used colors - * are stored in darkColorHandler), then use the related light mode color instead. - */ -function tryGetFontColor( - element: HTMLElement, - isDarkMode: boolean, - darkColorHandler: DarkColorHandler, - isBackground: boolean -) { - let darkColor: string | null; - - return element.tagName == 'FONT' && - !element.style.getPropertyValue(isBackground ? 'background-color' : 'color') && - isDarkMode && - (darkColor = element.getAttribute(isBackground ? 'bgcolor' : 'color')) - ? darkColorHandler.findLightColorFromDarkColor(darkColor) - : null; -} diff --git a/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/utils/colorTest.ts b/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/utils/colorTest.ts index d33a276b846..63597535080 100644 --- a/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/utils/colorTest.ts +++ b/packages-content-model/roosterjs-content-model-dom/test/formatHandlers/utils/colorTest.ts @@ -1,4 +1,4 @@ -import { DarkColorHandler } from 'roosterjs-editor-types'; +import { ColorManager } from 'roosterjs-content-model-types'; import { getColor, setColor } from '../../../lib/formatHandlers/utils/color'; import { itChromeOnly } from 'roosterjs-editor-dom/test/DomTestHelper'; @@ -7,33 +7,33 @@ describe('getColor with darkColorHandler', () => { const parseColorValue = jasmine.createSpy().and.returnValue({ lightModeColor: 'green', }); - const darkColorHandler = ({ parseColorValue } as any) as DarkColorHandler; + const darkColorHandler = ({ parseColorValue } as any) as ColorManager; const div = document.createElement('div'); const color = getColor(div, false, darkColorHandler, false); expect(color).toBe('green'); expect(parseColorValue).toHaveBeenCalledTimes(1); - expect(parseColorValue).toHaveBeenCalledWith(undefined); + expect(parseColorValue).toHaveBeenCalledWith(undefined, false); }); it('getColor from no color, dark mode', () => { const parseColorValue = jasmine.createSpy().and.returnValue({ lightModeColor: 'green', }); - const darkColorHandler = ({ parseColorValue } as any) as DarkColorHandler; + const darkColorHandler = ({ parseColorValue } as any) as ColorManager; const div = document.createElement('div'); const color = getColor(div, false, darkColorHandler, true); expect(color).toBe('green'); expect(parseColorValue).toHaveBeenCalledTimes(1); - expect(parseColorValue).toHaveBeenCalledWith(undefined); + expect(parseColorValue).toHaveBeenCalledWith(undefined, true); }); - it('getColor from style color, light mode', () => { + it('getColor from style color, dark mode', () => { const parseColorValue = jasmine.createSpy().and.returnValue({ lightModeColor: 'green', }); - const darkColorHandler = ({ parseColorValue } as any) as DarkColorHandler; + const darkColorHandler = ({ parseColorValue } as any) as ColorManager; const div = document.createElement('div'); div.style.color = 'red'; @@ -42,14 +42,14 @@ describe('getColor with darkColorHandler', () => { expect(color).toBe('green'); expect(parseColorValue).toHaveBeenCalledTimes(1); - expect(parseColorValue).toHaveBeenCalledWith('red'); + expect(parseColorValue).toHaveBeenCalledWith('red', true); }); - it('getColor from attr color, light mode', () => { + it('getColor from attr color, dark mode', () => { const parseColorValue = jasmine.createSpy().and.returnValue({ lightModeColor: 'green', }); - const darkColorHandler = ({ parseColorValue } as any) as DarkColorHandler; + const darkColorHandler = ({ parseColorValue } as any) as ColorManager; const div = document.createElement('div'); div.setAttribute('color', 'blue'); @@ -57,14 +57,14 @@ describe('getColor with darkColorHandler', () => { expect(color).toBe('green'); expect(parseColorValue).toHaveBeenCalledTimes(1); - expect(parseColorValue).toHaveBeenCalledWith('blue'); + expect(parseColorValue).toHaveBeenCalledWith('blue', true); }); - it('getColor from attr color with var, light mode', () => { + it('getColor from attr color with var, dark mode', () => { const parseColorValue = jasmine.createSpy().and.returnValue({ lightModeColor: 'green', }); - const darkColorHandler = ({ parseColorValue } as any) as DarkColorHandler; + const darkColorHandler = ({ parseColorValue } as any) as ColorManager; const div = document.createElement('div'); div.style.color = 'var(--varName, blue)'; @@ -72,14 +72,14 @@ describe('getColor with darkColorHandler', () => { expect(color).toBe('green'); expect(parseColorValue).toHaveBeenCalledTimes(1); - expect(parseColorValue).toHaveBeenCalledWith('var(--varName, blue)'); + expect(parseColorValue).toHaveBeenCalledWith('var(--varName, blue)', true); }); - it('getColor from style color with data-ogsc, light mode', () => { + it('getColor from style color with data-ogsc, dark mode', () => { const parseColorValue = jasmine.createSpy().and.returnValue({ lightModeColor: 'green', }); - const darkColorHandler = ({ parseColorValue } as any) as DarkColorHandler; + const darkColorHandler = ({ parseColorValue } as any) as ColorManager; const div = document.createElement('div'); div.dataset.ogsc = 'yellow'; @@ -88,36 +88,14 @@ describe('getColor with darkColorHandler', () => { expect(color).toBe('green'); expect(parseColorValue).toHaveBeenCalledTimes(1); - expect(parseColorValue).toHaveBeenCalledWith('red'); - }); - - it('get color from FONT tag in dark mode', () => { - const parseColorValue = jasmine.createSpy().and.returnValue({ - lightModeColor: 'green', - }); - const findLightColorFromDarkColor = jasmine.createSpy().and.returnValue('red'); - const darkColorHandler = ({ - parseColorValue, - findLightColorFromDarkColor, - } as any) as DarkColorHandler; - const font = document.createElement('font'); - - font.color = '#112233'; - - const color = getColor(font, false, darkColorHandler, true); - - expect(color).toBe('green'); - expect(parseColorValue).toHaveBeenCalledTimes(1); - expect(parseColorValue).toHaveBeenCalledWith('red'); - expect(findLightColorFromDarkColor).toHaveBeenCalledTimes(1); - expect(findLightColorFromDarkColor).toHaveBeenCalledWith('#112233'); + expect(parseColorValue).toHaveBeenCalledWith('red', true); }); }); describe('setColor with darkColorHandler', () => { it('setColor from no color, light mode', () => { const registerColor = jasmine.createSpy().and.returnValue('green'); - const darkColorHandler = ({ registerColor } as any) as DarkColorHandler; + const darkColorHandler = ({ registerColor } as any) as ColorManager; const div = document.createElement('div'); setColor(div, '', false, darkColorHandler, false); @@ -128,7 +106,7 @@ describe('setColor with darkColorHandler', () => { it('setColor from no color, dark mode', () => { const registerColor = jasmine.createSpy().and.returnValue('green'); - const darkColorHandler = ({ registerColor } as any) as DarkColorHandler; + const darkColorHandler = ({ registerColor } as any) as ColorManager; const div = document.createElement('div'); setColor(div, '', false, darkColorHandler, true); @@ -146,7 +124,7 @@ describe('setColor with darkColorHandler', () => { itChromeOnly('setColor from a color with existing color, dark mode', () => { const registerColor = jasmine.createSpy().and.returnValue('green'); - const darkColorHandler = ({ registerColor } as any) as DarkColorHandler; + const darkColorHandler = ({ registerColor } as any) as ColorManager; const div = document.createElement('div'); div.style.color = 'blue'; diff --git a/packages-content-model/roosterjs-content-model-types/lib/context/ColorManager.ts b/packages-content-model/roosterjs-content-model-types/lib/context/ColorManager.ts new file mode 100644 index 00000000000..4f4b37a04c3 --- /dev/null +++ b/packages-content-model/roosterjs-content-model-types/lib/context/ColorManager.ts @@ -0,0 +1,36 @@ +/** + * Represents a combination of color key, light color and dark color, parsed from existing color value + */ +export interface Colors { + /** + * Light mode color value + */ + lightModeColor: string; + + /** + * Dark mode color value, if found, otherwise undefined + */ + darkModeColor?: string; +} + +/** + * A handler object for dark color, used for variable-based dark color solution + */ +export interface ColorManager { + /** + * Given a light mode color value and an optional dark mode color value, register this color + * so that editor can handle it, then return the CSS color value for current color mode. + * @param lightModeColor Light mode color value + * @param isDarkMode Whether current color mode is dark mode + */ + registerColor(lightModeColor: string, isDarkMode: boolean): string; + + /** + * Parse an existing color value, if it is in variable-based color format, extract color key, + * light color and query related dark color if any + * @param color The color string to parse + * @param isInDarkMode Whether current content is in dark mode. When set to true, if the color value is not in dark var format, + * we will treat is as a dark mode color and try to find a matched dark mode color. + */ + parseColorValue(color: string | null | undefined, isInDarkMode?: boolean): Colors; +} diff --git a/packages-content-model/roosterjs-content-model-types/lib/context/EditorContext.ts b/packages-content-model/roosterjs-content-model-types/lib/context/EditorContext.ts index eb84ec5e4aa..a5857457313 100644 --- a/packages-content-model/roosterjs-content-model-types/lib/context/EditorContext.ts +++ b/packages-content-model/roosterjs-content-model-types/lib/context/EditorContext.ts @@ -1,6 +1,6 @@ +import type { ColorManager } from './ColorManager'; import type { ContentModelDomIndexer } from './ContentModelDomIndexer'; import type { ContentModelSegmentFormat } from '../format/ContentModelSegmentFormat'; -import type { DarkColorHandler } from 'roosterjs-editor-types'; /** * An editor context interface used by ContentModel PAI @@ -17,9 +17,9 @@ export interface EditorContext { defaultFormat?: ContentModelSegmentFormat; /** - * Dark model color handler + * Color manager, to help manager color in dark mode */ - darkColorHandler?: DarkColorHandler | null; + darkColorHandler?: ColorManager; /** * Whether to handle delimiters in Content Model diff --git a/packages-content-model/roosterjs-content-model-types/lib/index.ts b/packages-content-model/roosterjs-content-model-types/lib/index.ts index ae18e6c93b4..fda7ad8af1c 100644 --- a/packages-content-model/roosterjs-content-model-types/lib/index.ts +++ b/packages-content-model/roosterjs-content-model-types/lib/index.ts @@ -150,3 +150,4 @@ export { export { DomToModelOption } from './context/DomToModelOption'; export { ModelToDomOption } from './context/ModelToDomOption'; export { ContentModelDomIndexer } from './context/ContentModelDomIndexer'; +export { ColorManager, Colors } from './context/ColorManager';