Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standalone editor: Decouple DarkColorHandler #2144

Merged
merged 2 commits into from
Oct 16, 2023
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
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { DarkColorHandler } from 'roosterjs-editor-types';
import type { ColorManager } from 'roosterjs-content-model-types';

/**
* List of deprecated colors
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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;
}
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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';
Expand All @@ -42,44 +42,44 @@ 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');
const color = getColor(div, false, darkColorHandler, true);

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)';
const color = getColor(div, false, darkColorHandler, true);

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';
Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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';
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Loading