Support color transformation in table borders

[juliaroldi]

Introduce a new TransformTableBorderColor that enable border color transformation when the editor switch from light to dark mode.

[JiuqingSong]

No, this should not be here. The function adjustContainerColor is only for changing color of editor container DIV, not the editor content. And it is only called once when initialize editor.

Border color should be rendered by content model only, no direction DOM modification from other ways.

[JiuqingSong]

why only TH and TD?

[JiuqingSong]

suggest we do a string search first, for example, search for "--var" in the string and only when it exist, we call extractBorderValues and do color transformation.

[JiuqingSong]

add inline comments

[JiuqingSong]

same here, why only TD and TH

[juliaroldi]

Should we apply this change to all elements? Should image borders be affected by this change?

[JiuqingSong]

It should be PM's decision if we want to support other elements as well. My personal opinion is we can support all. After all this is just for dark mode, it will not change the final HTML. So think about a table and some other element has the same border color, then switch to dark mode, table border color changed, but others do not, which seems like a bug to me.

About this comment, my point is no matter if we want to support for other elements, this check should not be here. This is just a format handler, it should only care format, but not element. It should not know any detail about the element.

If we only want to apply this behavior to table, then build two separate format handlers, one for table and one for others. They can share some code, but should be two entry points.

[JiuqingSong]

Combine this for loop with previous one at line 86

[JiuqingSong]

And for dark mode, first calculate dark color, then reuse the same line at 89 for setting back styles.

[JiuqingSong]

It is a good idea to split the code of getting color from DOM and transform to light mode. Actually we can refactor it even better. Let's split it into 3 parts:
Prepare: create a BorderKey type (you can put it in types package) then create some string arrays and maps:

type BorderKey = 'borderTop' | 'borderRight' | 'borderBottom' | 'borderLeft';

/**
 * Keys of border items
 */
export const BorderKeys: (BorderKey & keyof BorderFormat & keyof CSSStyleDeclaration)[] = [
    'borderTop',
    'borderRight',
    'borderBottom',
    'borderLeft',
];

export const BorderColorKeyMap: {
    [key in BorderKey]: string & keyof CSSStyleDeclaration;
} = {
    borderTop: 'borderTopColor',
    borderRight: 'borderRightColor',
    borderBottom: 'borderBottomColor',
    borderLeft: 'borderLeftColor',
};

1. retrieveElementColor(element, isDarkMode, source: 'text' | 'background' | BorderKey, fallback?: string): string {...}. This will retrieve color from element according to its source string. When retrieve from text or background, keep the existing code to get from CSS then HTML attribute, for others, we can have a map from this border keys to the border color property names, for example:

function retrieveElementColor(
    element: HTMLElement,
    source: 'text' | 'background' | BorderKey,
    fallback?: string
): string | undefined {
    switch (source) {
        case 'text':
            return element.style.color || element.getAttribute('color') || fallback;

        case 'background':
            return element.style.backgroundColor || element.getAttribute('bgcolor') || fallback;

        default:
            return element.style.getPropertyValue(BorderColorKeyMap[source]) || fallback;
    }
}

2. getLightModeColor(...), this is to transform dark color to light color (same with your getColorInternal)
3. getColor(), keep the same parameter list and call to this two functions to keep compatibility:

function getColor(...) {
   const color = retrieveElementColor(element, isDarkMode, isBackground ? 'background' : 'color', fallback);

  return getLightModeColor(color, ...);
}

Then, you just need to call retrieveElementColor() and getLightModeColor() for borders in borderFormatHandler, no need to call extractBorderValues to avoid an extra RegEx call when parse.

Then for the setColor function, we can do a similar refactoring, the difference is when set color we don't need to set border color separately, but reuse the similar way you have to set borderLeft/Top/Right/Bottom in a single string.

[JiuqingSong]

Please refer to my previous comment, no need to have a separate setColorOnBorder function, but just set them together with border width and border style.

[JiuqingSong]

There are two "borderColor"

[JiuqingSong]

I believe you can use something like element.style.borderLeftColor and others to get border color directly, no need to call extractBorderValues() here.

[JiuqingSong]

As long as you are directly operating on DOM element, you can use these attributes, no need to extract from the string.

[JiuqingSong]

Same here, directly set element.style.borderColor

[JiuqingSong]

No need to combine them back, just set border[Direction]Color is enough.

[JiuqingSong]

See you already have these attributes, just use them when set/get colors