Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Unable to carry forward MS excel Theme colors in phpspreadsheet #3550

Closed
2 of 8 tasks
HenisPatel opened this issue May 4, 2023 · 10 comments · Fixed by #3580
Closed
2 of 8 tasks

Unable to carry forward MS excel Theme colors in phpspreadsheet #3550

HenisPatel opened this issue May 4, 2023 · 10 comments · Fixed by #3580

Comments

@HenisPatel
Copy link

HenisPatel commented May 4, 2023 •
edited
Loading

This is:

- [ ] a bug report
- [ ] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

What is the current behavior?

What are the steps to reproduce?

Please provide a Minimal, Complete, and Verifiable example of code that exhibits the issue without relying on an external Excel file or a web server:

<?php

require __DIR__ . '/vendor/autoload.php';

// Create new Spreadsheet object
$spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet();

// add code that show the issue here...

If this is an issue with reading a specific spreadsheet file, then it may be appropriate to provide a sample file that demonstrates the problem; but please keep it as small as possible, and sanitize any confidential information before uploading.

What features do you think are causing the issue

  • Reader
  • Writer
  • Styles
  • Data Validations
  • Formula Calculations
  • Charts
  • AutoFilter
  • Form Elements

Does an issue affect all spreadsheet file formats? If not, which formats are affected?

Which versions of PhpSpreadsheet and PHP are affected?
@oleibman
Copy link
Collaborator

oleibman commented May 4, 2023 •
edited
Loading

Your description is very vague. Can you upload a spreadsheet which demonstrates your problem, and tell us what you think is wrong with it? FWIW, PR #3476 added better support for theme colors. For compatibility reasons, it defaults to Excel 2007-2010 colors, but it is very easy to switch to Excel 2013+ colors.

use PhpOffice\PhpSpreadsheet\Theme as SpreadsheetTheme;

$spreadsheet->getTheme()->setThemeColorName(SpreadsheetTheme::COLOR_SCHEME_2013_PLUS_NAME);

@HenisPatel
Copy link
Author

Cell B2 background color and font color is theme color.
i can't get this color code or theme color name file import time.
i am using laravel.
Roster_01.May-07.May.2023.xls

@oleibman
Copy link
Collaborator

oleibman commented May 4, 2023

The file you uploaded is xlsx, not xls (as implied by the file name). This is not a big deal. You still weren't specific about the problem you were seeing, but I think I can make a guess now. If PhpSpreadsheet loads your spreadsheet and saves it as a new one, the background color for cell B2 does not quite match. Please confirm if my guess is correct.

There may be a problem in our "tint" calculation. @MarkBaker, are you familiar with Style/Color/changeBrightness? A web page describing tint, https://c-rex.net/samples/ooxml/e1/Part4/OOXML_P4_DOCX_fgColor_topic_ID0EX1B6.html, uses an algorithm which doesn't resemble our code, and it is difficult for me to see how we might even adapt it (if our existing code is wrong). Here's what I know about this situation:

  • In the uploaded spreadsheet, cell B2 has a fillColor of 538dd5. In an ideal world, the result of changeBrightness would be this value, or at least close to it.
  • In the xml, that cell uses a style which specifies <fgColor theme="3" tint="0.39997558519241921"/>.
  • I believe theme 3 is 'dk2', which has an rgb value in the xml of 1F497D.
  • In the spreadsheet which results from load and save, cell B2 has a fillColor of 78d1b0.
  • I wrote a program to list all 256*256*256 values for the result of changeBrightness.
  • That program confirms that the result of changeBrightness given an input of 1F497d is 78d1b0.
  • However, not a single changeBrightness result begins with 5, let alone matches our target value of 538dd5.

@HenisPatel
Copy link
Author

HenisPatel commented May 5, 2023 via email

@oleibman
Copy link
Collaborator

oleibman commented May 5, 2023

The theme color is converted to rgb (possibly incorrectly when tint is applied). It is not available to the application.

@oleibman
Copy link
Collaborator

oleibman commented May 5, 2023

The Python code at https://gist.github.com/Mike-Honey/b36e651e9a7f1d2e1d60ce1c63b9b633 for converting theme/tint to rgb gets us quite close to our target (final value is 558ed5 rather than 538dd5). It can probably be adapted to Php; I'm not sure that the effort is worthwhile.

@HenisPatel
Copy link
Author

HenisPatel commented May 20, 2023 via email

@HenisPatel
Copy link
Author

HenisPatel commented May 20, 2023 via email

@HenisPatel
Copy link
Author

HenisPatel commented May 20, 2023 via email

@MarkBaker MarkBaker mentioned this issue May 20, 2023
Closed
@MarkBaker
Copy link
Member

Sorry I've not been active here much lately; real life issues can be a real problem.

The original algorithm used for tint with themes was found through trial and error, enough to try and get close to the expected shade in most cases, but there was nothing that I could find that described the actual algorithm used, so there was nothing that I could base it on. I'm pleased to know that python now has a more accurate algorithm, and at some point we should implement it for PhpSpreadsheet. So I'll leave this Issue open until that time.

For the additional issue with data validation and lists from an array, I've opened a separate issue

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue May 21, 2023
@oleibman
Redo Calculation of Color Tinting
a8cc6d0 
Fix PHPOffice#3550. Some colors are specified in Excel by specifying a theme color to which a tint is applied. The original PHPExcel algorithm for doing this was developed by trial and error, and is good enough a lot of the time. However, for the issue at hand, the resulting color is detectably different from the calculation that Excel makes. Searching the web, I found https://gist.github.com/Mike-Honey/b36e651e9a7f1d2e1d60ce1c63b9b633 which comes much closer for the case in hand, and for all the other cases that I've looked at. That code depends on Python colorsys package; I have adapted the code from the Python gist and package into a new Php class. This doesn't agree perfectly with Excel. However, if each of the red, green, and blue components (each a value between 0 and 255 inclusive) agree within plus or minus 3 (arbitrary choice) of Excel's result, I think that is good enough.

I have added a new test member which reads from a spreadsheet with Xml altered by hand to set up several theme/tint cells. These tests use the plus-or-minus-3 criterion. They result in 100% code coverage of the new class.

Unsuprisingly, some existing tests failed with the new code. Issue2387Test reads a theme/tint font color, and is changed to use the plus-or-minus-3 criterion, comparing against the color as Excel shows it.

ColorChangeBrightness showed 9 failures with the new code. It consists of calculations not involving a spreadsheet. For that reason, I felt it was sufficient to just do an exact match test, changing the 9 old results for new results confirmed with the Python code. I also added one new test case, the one that kicked off this entire PR.
@oleibman oleibman mentioned this issue May 21, 2023
Merged
11 tasks
oleibman added a commit that referenced this issue May 25, 2023
@oleibman
Redo Calculation of Color Tinting (#3580)
9a13f52 
* Redo Calculation of Color Tinting

Fix #3550. Some colors are specified in Excel by specifying a theme color to which a tint is applied. The original PHPExcel algorithm for doing this was developed by trial and error, and is good enough a lot of the time. However, for the issue at hand, the resulting color is detectably different from the calculation that Excel makes. Searching the web, I found https://gist.github.com/Mike-Honey/b36e651e9a7f1d2e1d60ce1c63b9b633 which comes much closer for the case in hand, and for all the other cases that I've looked at. That code depends on Python colorsys package; I have adapted the code from the Python gist and package into a new Php class. This doesn't agree perfectly with Excel. However, if each of the red, green, and blue components (each a value between 0 and 255 inclusive) agree within plus or minus 3 (arbitrary choice) of Excel's result, I think that is good enough.

I have added a new test member which reads from a spreadsheet with Xml altered by hand to set up several theme/tint cells. These tests use the plus-or-minus-3 criterion. They result in 100% code coverage of the new class.

Unsuprisingly, some existing tests failed with the new code. Issue2387Test reads a theme/tint font color, and is changed to use the plus-or-minus-3 criterion, comparing against the color as Excel shows it.

ColorChangeBrightness showed 9 failures with the new code. It consists of calculations not involving a spreadsheet. For that reason, I felt it was sufficient to just do an exact match test, changing the 9 old results for new results confirmed with the Python code. I also added one new test case, the one that kicked off this entire PR.

* Scrutinizer Being Stupid

It strikes again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Redo Calculation of Color Tinting oleibman/PhpSpreadsheet
3 participants
@MarkBaker @oleibman @HenisPatel