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.

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

fix #2700 : set xaxis/yaxis and chart title color #2701

Closed
wants to merge 1 commit into from

Conversation

rycks
Copy link

@rycks rycks commented Mar 19, 2022

This PR is a partial solution (only excel export) for #2700 : now you can choose font color for xaxis/yaxis/title charts.

@MarkBaker MarkBaker added charts writer/xlsx Writer for MS OfficeOpenXML-format (xlsx) spreadsheet files labels Apr 2, 2022
@MarkBaker
Copy link
Member

MarkBaker commented Apr 2, 2022

Thank you for this contribution.

Is it possible to provide some unit tests to verify this? Charts is one area that is the least covered by tests, and it really needs some TLC there.

@MarkBaker
Copy link
Member

Also, please run a phpcs check to ensure coding complies with the PSR12 standard; phpcbf or php-cs-fixer should be able to fix all these warnings

Copy link
Member

@MarkBaker MarkBaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address the requested change; and composer fix should resolve all the warnings

* @param Font $font
* @return Title
*/
public function setFont(Font $font = null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If font defaults to null, the property should also allow null; and the writer should also check for null font when reading the colour

@oleibman
Copy link
Collaborator

Converting to draft status for now. There is no need for a font property for Title, which already has a property caption, which can be a RichText item, which can already have font information. PR #2828, which I will probably merge this week, uses this information to set chart title color for both export and import.

That PR does not take care of the axis labels (I believe it will handle read correctly but that's only part of the equation). I will look into that soon. Axis labels are more complicated than chart titles because write (export) attempts to reduce the caption from array/string/RichText to a string, and it looks like it does so incorrectly. So it will need further changes, in addition to handling colors.

@oleibman oleibman marked this pull request as draft May 15, 2022 03:11
oleibman added a commit to oleibman/PhpSpreadsheet that referenced this pull request May 19, 2022
Taking up where PHPOffice#2828 left off. Most of the following changes are demonstrated in 32readwriteChartWithImages1:
- Adds support for "scheme" colors (because rgb, theme, and index colors just weren't enough for Excel) for DataSeriesValues. See issue PHPOffice#2299.
- For chart titles (including axis labels), rather than a font name, Excel supplies a 3-fold series of font names for Latin, East Asian, and Complex Scripts. New properties `latin`, `eastAsian`, and `complexScript` are added to the Font class. I frankly have no idea how, or even if, you can set these in Excel; my test case (sample 32readwriteScatterChart7) is a result of manually editing the XML.
- Add support for subscript/superscript to chart titles. This requires a new property `baseLine` in Font (positive=superscript negative=subscript baseline value says how high/low).
- Support for underscore with different scheme color than its text, using a new string property `uSchemeClr` in Font.
- Support for extra options for strikethrough, using a new string property `strikeType` in Font.
- Support for extra options for underscore type, using the existing string property `underline` in Font.
- I do not anticipate that any of the new Font properties will be used except for chart titles.
- If no default font overrides are found for a Rich Text element in chart titles, and no explicit font overrides are found for a Run under such an element, the font element of the Run is set to null.
- PhpSpreadsheet will always write a tag `a:pPr` and, underneath that, an empty tag `a:defRPr`, for default font settings for chart titles and axis labels. Combined with the previous bullet item, this will prevent PhpSpreadsheet from inadvertently overriding the Excel defaults (18 point bold Calibri for chart title, 10 point bold Calibri for axis labels).
- Axis labels will now be written to XML in the same manner as chart titles. Among other considerations, this means that they can now have colors. Fix PHPOffice#2700. Supersedes PR PHPOffice#2701. Demonstrated in sample 32readwriteStockChart5.
@oleibman oleibman mentioned this pull request May 19, 2022
7 tasks
oleibman added a commit that referenced this pull request May 21, 2022
* More Chart Fixes

Taking up where #2828 left off. Most of the following changes are demonstrated in 32readwriteChartWithImages1:
- Adds support for "scheme" colors (because rgb, theme, and index colors just weren't enough for Excel) for DataSeriesValues. See issue #2299.
- For chart titles (including axis labels), rather than a font name, Excel supplies a 3-fold series of font names for Latin, East Asian, and Complex Scripts. New properties `latin`, `eastAsian`, and `complexScript` are added to the Font class. I frankly have no idea how, or even if, you can set these in Excel; my test case (sample 32readwriteScatterChart7) is a result of manually editing the XML.
- Add support for subscript/superscript to chart titles. This requires a new property `baseLine` in Font (positive=superscript negative=subscript baseline value says how high/low).
- Support for underscore with different scheme color than its text, using a new string property `uSchemeClr` in Font.
- Support for extra options for strikethrough, using a new string property `strikeType` in Font.
- Support for extra options for underscore type, using the existing string property `underline` in Font.
- I do not anticipate that any of the new Font properties will be used except for chart titles.
- If no default font overrides are found for a Rich Text element in chart titles, and no explicit font overrides are found for a Run under such an element, the font element of the Run is set to null.
- PhpSpreadsheet will always write a tag `a:pPr` and, underneath that, an empty tag `a:defRPr`, for default font settings for chart titles and axis labels. Combined with the previous bullet item, this will prevent PhpSpreadsheet from inadvertently overriding the Excel defaults (18 point bold Calibri for chart title, 10 point bold Calibri for axis labels).
- Axis labels will now be written to XML in the same manner as chart titles. Among other considerations, this means that they can now have colors. Fix #2700. Supersedes PR #2701. Demonstrated in sample 32readwriteStockChart5.

* Fix Some Chart Corruption

Fix #2817, where @bridgeplayr gives an excellent description of the problem and how it should be solved.

* Fix Bubble Charts

Sample produced corrupt output - see issue #2763. After a lot of research, solution was just re-ordering of parameters in a single function call.

Bubble 3D had not been supported at all. It is now.

Surface Charts remain corrupted.
@oleibman
Copy link
Collaborator

Closing - superseded by #2828 and #2841, which are now both merged.

@oleibman oleibman closed this May 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
charts writer/xlsx Writer for MS OfficeOpenXML-format (xlsx) spreadsheet files
Development

Successfully merging this pull request may close these issues.

3 participants