Skip to content

Add Conditional Formatting with IconSet #4574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

issakujitsuk
Copy link

@issakujitsuk issakujitsuk commented Aug 7, 2025

This is:

  • a bugfix
  • a new feature
  • refactoring
  • additional unit tests

Checklist:

  • Changes are covered by unit tests
    • Changes are covered by existing unit tests
    • New unit tests have been added
  • Code style is respected
  • Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
  • CHANGELOG.md contains a short summary of the change and a link to the pull request if applicable
  • Documentation is updated as necessary

Why this change is needed?

This change adds support for Conditional Formatting with IconSets in Xlsx files.
Without this change, IconSet formatting in Xlsx files was not preserved.
Fix #4560.

This PR supports the 17 IconSets from 3Arrows to 5Quarters as described in the Microsoft documentation.
IconSets from 3Stars onward are not supported in this PR because their markup structure differs significantly.

@oleibman
Copy link
Collaborator

oleibman commented Aug 7, 2025

Thanks - this looks very promising. I probably won't be able to get to it till next week.

@oleibman
Copy link
Collaborator

oleibman commented Aug 7, 2025

I think this PR would benefit from the addition of a sample illustrating some different uses of IconSets.

For the 3 unimplemented cases (3Triangles, 3Stars, 5Boxes), what happens when Xlsx Reader encounters one of those cases?Please add a unit test for that.

@oleibman
Copy link
Collaborator

oleibman commented Aug 7, 2025

And you may need to do something with "NoIcons" as well.

@issakujitsuk
Copy link
Author

@oleibman
Thanks for the advice.
I’ve added a test in tests/PhpSpreadsheetTests/Reader/Xlsx/ConditionalIconSetTest.php for reading an Icon Set that is not yet supported (3Triangles, 3Stars, 5Boxes, NoIcons).

Also, I thought it would be good to have a test for the Writer as well, so I added tests/PhpSpreadsheetTests/Writer/Xlsx/ConditionalFormatIconSetTest.php for writing a worksheet that uses an Icon Set.

I think this PR would benefit from the addition of a sample illustrating some different uses of IconSets.

I’m not entirely sure what kind of sample you mean, and I may have misunderstood your advice. Could you please give me a bit more detail?

@oleibman
Copy link
Collaborator

oleibman commented Aug 9, 2025

Thanks for making those changes.

If you have downloaded the full distribution, and I think you must have since you have a tests directory, you will have a samples directory, and, in particular, a ConditionalFormatting directory under that. Take a look at cond08_colorscale.php, which we added when we introduced color scales, just to show how it works. If you can add a member like that for icon sets, I think that would be helpful for users (like me before you submitted this PR) who aren't familiar with them. I think yours would need to be a little longer than the colorscale sample - maybe have 3 different ranges, one with a 3-something set, one with a 4-something, and one with a 5-something.

@issakujitsuk
Copy link
Author

@oleibman
Thanks for explaining that so clearly.
I added samples/ConditionalFormatting/cond09_iconset.php, which includes examples using 3Symbols, 4Arrows, and 5Quarters.
I apologize if my explanation still falls short—please let me know if there’s anything that needs clarification or improvement.

Does this look good to you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Support IconSet in Conditional Formatting
2 participants