excelize icon indicating copy to clipboard operation
excelize copied to clipboard

WIP add support for new icon sets

Open imirkin opened this issue 1 year ago • 2 comments

PR Details

This adds support for the "new" icon sets (3 triangles, 3 stars, 5 boxes). These were introduced in a later revision of the spec, and thus need different namespaces/etc.

Description

For the new icon sets, we add an x14-namespaced conditional formatting entry + rules. Note that unlike the data bar, these are entirely in the "new" land, so no need to add it in the old location + ext.

Still WIP:

  • [ ] Strategy for priorities
  • [ ] GUID generation? The Excel-generated samples have GUIDs, but seem to work without it
  • [x] Decoding

Looking for some feedback/suggestions.

Related Issue

#2038

How Has This Been Tested

I added the conditional format with all 3 "new" icon sets (as well as a data bar with solid=true just to make sure I'm not breaking anything existing), and it displayed fine in Excel.

Types of changes

  • [ ] Docs change / refactoring / dependency upgrade
  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • [x] My code follows the code style of this project.
  • [x] My change requires a change to the documentation.
  • [x] I have updated the documentation accordingly.
  • [x] I have read the CONTRIBUTING document.
  • [x] I have added tests to cover my changes.
  • [x] All new and existing tests passed.

imirkin avatar Dec 11 '24 21:12 imirkin

Codecov Report

Attention: Patch coverage is 94.54545% with 3 lines in your changes missing coverage. Please review.

Project coverage is 99.19%. Comparing base (8e04909) to head (38c080f). Report is 24 commits behind head on master.

Files with missing lines Patch % Lines
styles.go 94.54% 2 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2042      +/-   ##
==========================================
- Coverage   99.20%   99.19%   -0.01%     
==========================================
  Files          32       32              
  Lines       29975    30017      +42     
==========================================
+ Hits        29737    29776      +39     
- Misses        158      160       +2     
- Partials       80       81       +1     
Flag Coverage Δ
unittests 99.19% <94.54%> (-0.01%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Dec 11 '24 22:12 codecov[bot]

The problem here, BTW, is that priority is determined based on analyzing the list of ws.ConditionalFormatting. But this won't count the "x14" cfrules. So we have to re-parse it. There's a bit of proliferation of that parsing code, so it should get centralized somehow. But re-parsing it from scratch every time we add a conditional format rule will lead to O(n^2) behavior. (I guess that's already there, but adding some numbers isn't so heavy as parsing a bunch of xml each time.)

Suggestions welcome.

imirkin avatar Dec 21 '24 18:12 imirkin