Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves dark-mode background color generation by letting the dark color callback consider the associated text color, and by incorporating the text color into the generated dark color CSS variable key so different foreground/background pairings can produce different dark backgrounds.
Changes:
- Extend dark color transformation APIs to accept an optional
comparingColor(intended to be text color when generating dark backgrounds). - Update dark color key generation to include
comparingColor(e.g.--darkColor_<bg>_<text>), and propagate inherited text color during DOM color transformation. - Update and add tests to reflect the new key format and call signatures.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/roosterjs-editor-adapter/lib/editor/EditorAdapter.ts | Passes editor default text color into transformColor during dark color transformation. |
| packages/roosterjs-content-model-types/lib/context/DarkColorHandler.ts | Extends ColorTransformFunction signature to include optional comparingColor. |
| packages/roosterjs-content-model-dom/lib/formatHandlers/utils/color.ts | Threads comparingColor through setColor/adaptColor and updates default key generation to append it. |
| packages/roosterjs-content-model-dom/lib/formatHandlers/common/backgroundColorFormatHandler.ts | Passes text color as comparingColor when applying background color in dark mode. |
| packages/roosterjs-content-model-dom/lib/domUtils/style/transformColor.ts | Propagates parent/inherited text color to improve background dark color generation and keying. |
| packages/roosterjs-content-model-core/lib/editor/Editor.ts | Passes editor default text color into transformColor when toggling dark mode. |
| packages/roosterjs-content-model-dom/test/formatHandlers/utils/colorTest.ts | Updates expectations for new key format and getDarkColor argument list. |
| packages/roosterjs-content-model-dom/test/formatHandlers/common/backgroundColorFormatHandlerTest.ts | Adds coverage for combined text+background in dark mode affecting background key. |
| packages/roosterjs-content-model-dom/test/domUtils/style/transformColorTest.ts | Updates expected background keys and adds inherited text color scenario coverage. |
| packages/roosterjs-content-model-core/test/editor/EditorTest.ts | Updates transformColor call expectations for the new optional parameter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const backgroundColorFormatHandler: FormatHandler< | ||
| BackgroundColorFormat & TextColorFormat | ||
| > = { |
There was a problem hiding this comment.
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.
| * @param baseLValue Base value of light used for dark value calculation | ||
| * @param colorType @optional Type of color, can be text, background, or border | ||
| * @param element @optional Source HTML element of the 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 |
There was a problem hiding this comment.
The new comparingColor param doc says it helps ensure the generated "dark border color" has enough contrast, but this type is used for generating dark colors in general and the param is specifically described as for background-vs-text contrast. Please update the wording to refer to "dark background color" (or generally "generated dark color"), not border 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 | |
| * @param comparingColor @optional When generating dark color for background color, we can provide text color as comparingColor to make sure the generated dark background color has enough contrast with text color in dark mode |
| * @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 |
There was a problem hiding this comment.
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.
| * @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 |
| includeSelf: boolean, | ||
| direction: 'lightToDark' | 'darkToLight', | ||
| darkColorHandler?: DarkColorHandler, | ||
| transformColorOptions?: TransformColorOptions | ||
| transformColorOptions?: TransformColorOptions, | ||
| defaultTextColor?: string | ||
| ) { |
There was a problem hiding this comment.
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.
* Fix publish file (#3267) * fix publish file * remove file * [Table Improvements] Add undoSnapshot when tab on a table cell (#3265) Add undoSnapshot after pressing Tab key in a table that has new content, otherwise if the user type content in a table and press tab to move to another cell and then undo the content, all the typed content will be removed. * [Table Improvements] Use keyboard to delete rows and columns (#3270) When press backspace or shift + delete when an entire row or column, delete the column and row. * [Table Improvements] Add Shift Cells Table Operation (#3271) Add new shift cells up and shift cells left table operations. These operations move the table cell content to the cells at left or above. * align table cell list (#3275) When apply alignment in table cells that has list items, also apply the alignment to the list items. * fill gaps (#3272) * Bump lodash from 4.17.21 to 4.17.23 (#3266) Bumps [lodash](https://github.com/lodash/lodash) from 4.17.21 to 4.17.23. - [Release notes](https://github.com/lodash/lodash/releases) - [Commits](lodash/lodash@4.17.21...4.17.23) --- updated-dependencies: - dependency-name: lodash dependency-version: 4.17.23 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jiuqing Song <jisong@microsoft.com> * fix table format (#3277) When triggering clearFormat on table cells, do not clear the cell or the table format. * [Table Improvements] Add preview for table cell selection (#3274) When start shadow edit, check if table cells are selected, if they are selected, remove the background color to make the styles changes visible in the table. * Fix outdated JSDoc comments in setTableCellsStyle.ts (#3278) Fix JSDoc comments for removeTableCellsStyle function to match actual parameters Fix JSDoc comments for setTableCellsStyle function to match actual parameters * Fix 329516 (#3276) Co-authored-by: Bryan Valverde U <bvalverde@microsoft.com> * [Table Improvements] Insert table content (#3258) When inserting a table in a range selection, insert the selected content inside the table. * Bump webpack from 5.94.0 to 5.104.1 (#3285) Bumps [webpack](https://github.com/webpack/webpack) from 5.94.0 to 5.104.1. - [Release notes](https://github.com/webpack/webpack/releases) - [Changelog](https://github.com/webpack/webpack/blob/main/CHANGELOG.md) - [Commits](webpack/webpack@v5.94.0...v5.104.1) --- updated-dependencies: - dependency-name: webpack dependency-version: 5.104.1 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Filter temporary EOP elements in Word Online paste and add test pattern support (#3283) * Filter temporary EOP elements in Word Online paste and add test pattern support - Skip elements with both 'Selected' and 'EOP' classes during WAC paste processing to remove temporary End of Paragraph markers - Add unit tests for EOP element filtering behavior (3 test cases) - Enhance test runner with --testPathPattern and --testNamePattern flags for faster targeted test execution 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * Update packages/roosterjs-content-model-plugins/lib/paste/WacComponents/processPastedContentWacComponents.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Dark color improvement (#3279) * Dark color improvement * improve * Fix #3280 (#3282) * [Table Improvements] Ignore span cells when merge table cells (#3281) When merging table cells, count table that are span as one cell, so two or more cells cannot be merge to one single span cell. * Bump main and legacyAdapter versions in versions.json --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Julia Roldi <87443959+juliaroldi@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jiuqing Song <jisong@microsoft.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
When call color callback to calculate dark color, pass in the text color for background color so the callback function can calculate a better background color according to the text color.
And in order to allow different background dark color for different text color, also add text color to background color key.
Example:
old color key:
--darkColor__f5d427(here #f5d427 is the light mode background color)new color key:
--darkColor__f5d427__ff0000(here #ff0000 is the light mode text color)