fix(xlsx): honor icon-set cfvo thresholds and their types#407
Merged
Conversation
Icon-set conditional formatting assigned most cells the wrong band. Two causes (issue #406): 1. umya-spreadsheet drops iconSet <cfvo> children written as start/end tag pairs (the Event::Start reader gap already fixed for dataBar), so the threshold collection came back empty and office2pdf fell back to equal-thirds bands. 2. office2pdf computed every threshold as min + range*(val/100), treating the cfvo value as a percentage even for type="num". Parse the iconSet cfvo (type, val) pairs from the raw worksheet XML — which office2pdf already reads for the icon-set type — and compute each threshold per its cfvo type: num is literal, percent is a fraction of the range, percentile interpolates the values, min/max are the extremes. umya's typeless cfvos remain a legacy fallback. Verified on the KPI fixture: bands now match Excel (107% green-up, 93% yellow-side, 84% red-down). Remaining glyph weight is tracked in #377. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Yonghye Kwon <developer.0hye@gmail.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Yonghye Kwon <developer.0hye@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Icon-set conditional formatting assigned most cells the wrong band — values landed one band too low. On
10_kpi_tracker_en.xlsxthe 3Arrows rule usestype="num"thresholds[0, 0.9, 1.0], but 107% rendered with the middle (yellow) icon and 93% with the low (red) icon instead of green-up / yellow-side (issue #406).Two root causes:
<cfvo>elements are written as start/end tag pairs (openpyxl style); umya'sIconSetreader only matchesEvent::Empty, soget_cfvo_collection()came back empty and office2pdf fell back to equal-thirds thresholds (the same reader gap fixed forDataBarin fix: read dataBar cfvo and color children written as start/end tags MathNya/umya-spreadsheet#347).min + range·(val/100), treating the value as a percentage even fortype="num".The iconSet cfvo
(type, val)pairs are now parsed from the raw worksheet XML (where office2pdf already reads the icon-set type) and each threshold is resolved per its type:numliteral,percenta fraction of the range,percentileinterpolated over the values,min/maxthe extremes. umya's typeless cfvos remain a legacy fallback, and equal-thirds the last resort.Verified on the KPI fixture: every cell's band now matches Excel (107% green-up, 93% yellow-side, 113% green-up, 84% red-down, 103%/130% green-up).
Related issue
Fixes #406
Testing
cargo test --workspace— greenworksheet_hints_collect_icon_set_cfvo_thresholds,worksheet_hints_do_not_confuse_databar_cfvo_with_icon_settest_cond_fmt_icon_set_num_thresholds_from_start_end_cfvos(rewrites cfvos into start/end tags, asserts the per-band colors)10_kpi_tracker_en.xlsx: bands match ExcelVisual impact
Visual audit
10_kpi_tracker_en.xlsx(synthetic real-world audit set, external)fixassets/bugfixes/issue-406/gt.jpgassets/bugfixes/issue-406/before.jpgassets/bugfixes/issue-406/after.jpgVisual comparison
Required inspection
Deviation audit
Checklist
Signed-off-byline🤖 Generated with Claude Code