Skip to content

More Chart Fixes#2841

Merged
oleibman merged 4 commits intoPHPOffice:masterfrom
oleibman:scatter2
May 21, 2022
Merged

More Chart Fixes#2841
oleibman merged 4 commits intoPHPOffice:masterfrom
oleibman:scatter2

Conversation

@oleibman
Copy link
Collaborator

@oleibman oleibman commented May 19, 2022

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 Define color of the marker on a chart #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 Set color for xaxis / yaxis title #2700. Supersedes PR fix #2700 : set xaxis/yaxis and chart title color #2701. Demonstrated in sample 32readwriteStockChart5.

Fix #2817, preventing corruption of some scatter charts.

Fix corruption issue in Bubble Charts, and add support for 3D Bubble Charts. See issue #2763. Surface charts remain corrupted.

This is:

- [x] 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
  • Documentation is updated as necessary

Why this change is needed?

Provide an explanation of why this change is needed, with links to any Issues (if appropriate).
If this is a bugfix or a new feature, and there are no existing Issues, then please also create an issue that will make it easier to track progress with this PR.

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 added 3 commits May 20, 2022 12:11
Fix PHPOffice#2817, where @bridgeplayr gives an excellent description of the problem and how it should be solved.
Sample produced corrupt output - see issue PHPOffice#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 oleibman merged commit ef031e7 into PHPOffice:master May 21, 2022
@oleibman oleibman deleted the scatter2 branch May 25, 2022 08:13
oleibman added a commit to oleibman/PhpSpreadsheet that referenced this pull request May 26, 2022
Continuing the work from PHPOffice#2828, PHPOffice#2841, PHPOffice#2846, and PHPOffice#2852. This is probably my last change in this area for a while.

Bubble charts can have bubbles of different sizes. Phpspreadsheet had not supported this. Openpyxl comes with sample code to generate such a chart. I was especially drawn to that solution because its namespace usage would have been unexpected before 2852. And it turned out to come with other surprises - use of absolute paths in the .rels files (PhpSpreadsheet expected only relative), use of a one-cell anchor to place the chart (PhpSpreadsheet expected two-cell anchor or absolute positioning), plaintext in the legend (Phpspreadsheet expected RichText), no cached values for chart data. Excel handles the file okay, and this PR makes sure PhpSpreadsheet does as well. This file is now Samples/Templates/32readwriteBubbleChart2, and is used for both generating a sample output file and in formal tests. A new sample in the 33* series demonstrates how to code these.
@oleibman oleibman mentioned this pull request May 26, 2022
7 tasks
oleibman added a commit that referenced this pull request May 29, 2022
Continuing the work from #2828, #2841, #2846, and #2852. This is probably my last change in this area for a while.

Bubble charts can have bubbles of different sizes. Phpspreadsheet had not supported this. Openpyxl comes with sample code to generate such a chart. I was especially drawn to that solution because its namespace usage would have been unexpected before 2852. And it turned out to come with other surprises - use of absolute paths in the .rels files (PhpSpreadsheet expected only relative), use of a one-cell anchor to place the chart (PhpSpreadsheet expected two-cell anchor or absolute positioning), plaintext in the legend (Phpspreadsheet expected RichText), no cached values for chart data. Excel handles the file okay, and this PR makes sure PhpSpreadsheet does as well. This file is now Samples/Templates/32readwriteBubbleChart2, and is used for both generating a sample output file and in formal tests. A new sample in the 33* series demonstrates how to code these.
MarkBaker added a commit that referenced this pull request Jul 9, 2022
Note that this will be the last 1.x branch release before the 2.x release. We will maintain both branches in parallel for a time; but users are requested to update to version 2.0 once that is fully available.

### Added

- Added `removeComment()` method for Worksheet [PR #2875](https://github.com/PHPOffice/PhpSpreadsheet/pull/2875/files)
- Add point size option for scatter charts [Issue #2298](#2298) [PR #2801](#2801)
- Basic support for Xlsx reading/writing Chart Sheets [PR #2830](#2830)

  Note that a ChartSheet is still only written as a normal Worksheet containing a single chart, not as an actual ChartSheet.

- Added Worksheet visibility in Ods Reader [PR #2851](#2851) and Gnumeric Reader [PR #2853](#2853)
- Added Worksheet visibility in Ods Writer [PR #2850](#2850)
- Allow Csv Reader to treat string as contents of file [Issue #1285](#1285) [PR #2792](#2792)
- Allow Csv Reader to store null string rather than leave cell empty [Issue #2840](#2840) [PR #2842](#2842)
- Provide new Worksheet methods to identify if a row or column is "empty", making allowance for different definitions of "empty":
  - Treat rows/columns containing no cell records as empty (default)
  - Treat cells containing a null value as empty
  - Treat cells containing an empty string as empty

### Changed

- Modify `rangeBoundaries()`, `rangeDimension()` and `getRangeBoundaries()` Coordinate methods to work with row/column ranges as well as with cell ranges and cells [PR #2926](#2926)
- Better enforcement of value modification to match specified datatype when using `setValueExplicit()`
- Relax validation of merge cells to allow merge for a single cell reference [Issue #2776](#2776)
- Memory and speed improvements, particularly for the Cell Collection, and the Writers.

  See [the Discussion section on github](#2821) for details of performance across versions
- Improved performance for removing rows/columns from a worksheet

### Deprecated

- Nothing

### Removed

- Nothing

### Fixed

- Xls Reader resolving absolute named ranges to relative ranges [Issue #2826](#2826) [PR #2827](#2827)
- Null value handling in the Excel Math/Trig PRODUCT() function [Issue #2833](#2833) [PR #2834](#2834)
- Invalid Print Area defined in Xlsx corrupts internal storage of print area [Issue #2848](#2848) [PR #2849](#2849)
- Time interval formatting [Issue #2768](#2768) [PR #2772](#2772)
- Copy from Xls(x) to Html/Pdf loses drawings [PR #2788](#2788)
- Html Reader converting cell containing 0 to null string [Issue #2810](#2810) [PR #2813](#2813)
- Many fixes for Charts, especially, but not limited to, Scatter, Bubble, and Surface charts. [Issue #2762](#2762) [Issue #2299](#2299) [Issue #2700](#2700) [Issue #2817](#2817) [Issue #2763](#2763) [Issue #2219](#2219) [Issue #2863](#2863) [PR #2828](#2828) [PR #2841](#2841) [PR #2846](#2846) [PR #2852](#2852) [PR #2856](#2856) [PR #2865](#2865) [PR #2872](#2872) [PR #2879](#2879) [PR #2898](#2898) [PR #2906](#2906) [PR #2922](#2922) [PR #2923](#2923)
- Adjust both coordinates for two-cell anchors when rows/columns are added/deleted. [Issue #2908](#2908) [PR #2909](#2909)
- Keep calculated string results below 32K. [PR #2921](#2921)
- Filter out illegal Unicode char values FFFE/FFFF. [Issue #2897](#2897) [PR #2910](#2910)
- Better handling of REF errors and propagation of all errors in Calculation engine. [PR #2902](#2902)
- Calculating Engine regexp for Column/Row references when there are multiple quoted worksheet references in the formula [Issue #2874](#2874) [PR #2899](#2899)
@u01jmg3
Copy link
Contributor

u01jmg3 commented Jan 12, 2023

Sorry this is a late comment.

Just wondering if you could shed some light on one of your changes to Chart.php? I couldn't see any mention of it in your notes.

-$this->writeCategoryAxis($objWriter, $xAxisLabel, $id1, $id2, $catIsMultiLevelSeries, $xAxis, ($chartType === DataSeries::TYPE_SCATTERCHART) ? 'c:valAx' : 'c:catAx');
+$this->writeCategoryAxis($objWriter, $xAxisLabel, $id1, $id2, $catIsMultiLevelSeries, $xAxis);

It now means the (horizontal) X-axis is of the type "category" rather than "value" and I can see no way of being able to modify this within PHPSpreadsheet. If you were to create this chart from scratch in Excel it would apply the type "value".

@oleibman
Copy link
Collaborator Author

You are taxing my memory - I don't know that I can give you a satisfactory explanation. I started with known samples created in Excel, and the Xml for some of these contained multiple value axes and no category axes (or perhaps vice versa) when using scatter charts. If you have a workbook that, because of this code, winds up broken when you use PhpSpreadsheet to copy it to another worksheet, I can take a look. Please open a new issue if so.

@u01jmg3
Copy link
Contributor

u01jmg3 commented Jan 13, 2023

Thanks @oleibman - see issue #3294

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

Labels

Development

Successfully merging this pull request may close these issues.

Prevent Writer/Xlsx/Chart.php corrupting Excel scattercharts (& maybe other number type charts) Set color for xaxis / yaxis title

2 participants