Skip to content

fix(xlsx): write colour scale and data bar colours as ARGB - #606

Open
ronistrom wants to merge 1 commit into
productdevbook:mainfrom
ronistrom:fix/cf-color-argb
Open

ronistrom wants to merge 1 commit into
productdevbook:mainfrom
ronistrom:fix/cf-color-argb

Conversation

@ronistrom

@ronistrom ronistrom commented Oct 20, 2026 •

Copy link
Copy Markdown

Closes #595

What was wrong

Every other colour field turns a 6-digit RGB into ARGB with an FF alpha prefix: fonts, fills, borders, rich text, tab colour and sparklines. Colour scale and data bar colours in conditionalRules were written exactly as given:

conditionalRules: [
  { type: "colorScale", priority: 1, range: "A1:A3", colorScale: { cfvo: [{ type: "min" }, { type: "max" }], colors: ["638EC6", "F8696B"] } },
  { type: "dataBar", priority: 2, range: "A1:A3", dataBar: { cfvo: [{ type: "min" }, { type: "max" }], color: "638EC6" } },
]
<!-- before -->
<colorScale>…<color rgb="638EC6"/><color rgb="F8696B"/></colorScale>
<dataBar>…<color rgb="638EC6"/></dataBar>
<!-- after -->
<colorScale>…<color rgb="FF638EC6"/><color rgb="FFF8696B"/></colorScale>
<dataBar>…<color rgb="FF638EC6"/></dataBar>

CT_Color/@rgb is ST_UnsignedIntHex, an xsd:hexBinary of length 4 (eight hex digits), so the 6-digit form is not schema-valid.

Why the tests didn't catch it

coverage-xlsx-writer.test.ts had a data bar test that wrote "638EC6" and asserted <color rgb="638EC6"/>, so the test pinned the bug. The round-trip tests in conditional-formatting.test.ts all use 8-digit colours.

What landed

  • A small toArgb(rgb) helper in worksheet-writer.ts. serializeColorAttrs and the colour scale, data bar and sparkline sites all use it. The sparkline site had the same check written inline, and its output is unchanged.
  • serializeConditionalFormatting is shared with writeXlsxStream / writeXlsxStreamSheets, so the streaming writers are fixed too. There is a test for that.
  • Doc comments on colorScale.colors and dataBar.color say that a 6-digit value is accepted and reads back in 8-digit form. The reader keeps conditional-formatting colours exactly as written, so "638EC6" now reads back as "FF638EC6". It is the same colour, but a strict equality check on a round trip will see the difference. Before this change, the value read back unchanged because the file held the invalid 6-digit form.

Tests: two of the three new tests fail against main, plus the corrected data bar assertion. The third new test checks that 8-digit colours pass through untouched, which already worked. bun run test passes on Node 24.22.0, with 240 files and 10,701 tests.

Checked in Excel

In Excel for Mac, with the colour scale on A1:A10 and the data bars on B1:B10, written by main and by this branch. Both files open without a repair prompt. Both show A running red to green and blue bars in B. Excel accepts the six-digit form too, so there is no visible difference in Excel. The change makes the value schema-valid, and the after file shows Excel takes it.

Not verified

  • LibreOffice. Not opened. According to the issue it already accepted both forms.
  • Windows Excel, and stricter readers. Not checked.

Summary by CodeRabbit

  • Enhancements

    • Improved Excel color handling by automatically converting six-digit RGB colors to opaque ARGB values for color scales, data bars, and sparkline series.
    • Existing eight-digit ARGB color values remain unchanged.
    • Six-digit RGB colors are now consistently preserved through supported worksheet-writing workflows.
  • Documentation

    • Clarified accepted color formats and automatic alpha-prefix behavior for conditional-formatting colors.

Every other colour field turns a 6-digit RGB into ARGB with an FF alpha
prefix. Colour scale and data bar colours in conditional rules were
written as given, so `"638EC6"` went out as `<color rgb="638EC6"/>`.
CT_Color/@rgb is ST_UnsignedIntHex, four bytes, so that value is not
schema-valid. Both sites, and the sparkline colour that did the same
thing inline, now go through one `toArgb` helper.

A 6-digit colour now reads back as its 8-digit ARGB form, because the
reader keeps conditional-formatting colours as written.

Closes productdevbook#595
@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2cdcb60e-d73e-4fb7-8b6b-1c9669802172
📥 Commits

Reviewing files that changed from the base of the PR and between c991ad0 and 47daef2.

📒 Files selected for processing: 3
  • src/_types.ts
  • src/xlsx/worksheet-writer.ts
  • test/coverage-xlsx-writer.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

📝 Walkthrough

The worksheet writer now prefixes six-digit RGB values with FF when it serializes color-scale, data-bar, and sparkline colors. Documentation and tests describe and verify the color format.

Changes: Worksheet color normalization

Layer / File(s) Summary
Normalize serialized worksheet colors
src/_types.ts, src/xlsx/worksheet-writer.ts, test/coverage-xlsx-writer.test.ts
Documentation describes accepted RGB and ARGB formats. The writer prefixes six-digit RGB values with FF for color scales, data bars, and sparkline series. Tests cover regular and streaming writers, and verify that eight-digit values remain unchanged.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: productdevbook

Merge Risk: ⚪ Minimal · up to 47dae

No actionable merge-blocking risk is identified; the change is ready for normal merge checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: writing color-scale and data-bar colors as ARGB. It matches the implementation and stated objective.
Linked Issues check Passed Issue #595 requires six-digit conditional-formatting colours to receive an FF prefix and eight-digit colours to remain unchanged, with test coverage. toArgb now applies this behavior to color-scal…
Out of Scope Changes check Passed The changes remain connected to issue #595. The shared toArgb helper replaces equivalent logic for sparklines without changing sparkline output. The type comments and streaming-writer test document …
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files.
Full details: Linked Issues check

Explanation

Issue #595 requires six-digit conditional-formatting colours to receive an FF prefix and eight-digit colours to remain unchanged, with test coverage. toArgb now applies this behavior to color-scale and data-bar serialization in src/xlsx/worksheet-writer.ts. The tests cover six-digit values, eight-digit pass-through, and the streaming writer. The updated public comments document the behavior.

Full details: Out of Scope Changes check

Explanation

The changes remain connected to issue #595. The shared toArgb helper replaces equivalent logic for sparklines without changing sparkline output. The type comments and streaming-writer test document or verify the same color-normalization behavior. No unrelated product behavior is introduced.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Colour scale and data bar colours are not normalised to ARGB like other colours

1 participant