-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Deprecate use of $smoothLine as well as $plotStyle='lineMarker' in Chart/DataSeries for ScatterCharts #2935
Comments
JpGraph is not deprecated. It is just not maintained in Composer, and the version there is not Php8 compatible (maybe not even some versions of 7) so we can't allow its use in our test suite. However, you can manually substitute the current version of that product for the version that Composer brings in, and it should work just fine. I have no idea what it's doing with lineMarker, but am reluctant to make it stop working (under the uncertain assumption that it works now). As for smoothLine, that was one of your first requests that I worked on. It seemed to me at the time that Excel required it at both the DS and the DSV levels. I can look into it again. |
This annoying behavior tripped me up while I was testing Axis cases. Thank you for the JpGraph clarification - I had hoped to use it, and I now feel free to try it - if I can find a compatible version. [N.B. I understand removing properties is a breaking change, but having two identically-named properties in two similarly-named classes (DataSeriesValues & DataSeries) with identical getter/setter methods is seriously confusing. At least to me.] I repeat my previous request to remove DataSeries property $smoothLine and its getter/setter methods. In a ScatterChart, the accidental assignment of 'lineMarker' to $plotStyle will suppress a $DSV whose $smoothLine property is true, and the user will have a difficult time discovering the reason for the resultant straight-line. To prevent this unfortunate circumstance, I propose that Writer/Xlsx change the qualification for the call to startElement('c:smooth') in the "// Values" section of function writePlotArea() from: WAS: if ($groupType === DataSeries::TYPE_SCATTERCHART && $plotGroup->getPlotStyle() === 'smoothMarker') IS: if ($groupType === DataSeries::TYPE_SCATTERCHART) This will allow a $DSV->smoothLine property value of TRUE to be expressed correctly in the xml regardless of its containing $DS->plotStyle value, (which is the correct behavior in my opinion.) |
Under what circumstances would you "accidentally" set the style to lineMarker? It sounds like you're trying to get PhpSpreadsheet to ignore something the programmer has explicitly requested - it certainly isn't used as any kind of a default. Perhaps, after looking at the finished product, smooth lines aren't desired after all, and it's easier to change the DataSeries property to lineMarker in one place than it is to remove the smooth declarations from all the DSVs? It is possible that you can make a stronger case for using the c:smooth tag for "not lineMarker"; I would still need to think about that. Issue #2783 has more details about how to use JpGraph. |
"Accidental" was a poor choice of words. I should have perhaps used "unintentionally" or "unknowingly". Excel describes 'smooth line' charts as well as 'line charts'. It is not a stretch to associate those choices with 'lineMarker' and 'smoothMarker'. But what if you want a mix? I submit that a programmer who assembles a ScatterChart may simply ignore assigning any value to the DS->plotStyle property since it is possible to desire an XY chart with both smooth lines and straight lines, and maybe no lines. So the programmer omits it, and its value is null. If the programmer has assigned TRUE to any DSV->smoothLine property, she will be mystified why it doesn't work. (We'll know, but she won't have much of a clue.) Changing Writer masks her (unfortunate) choice of not assigning 'smoothMarker' to DS->plotStyle. Thank you for the JpGraph link. |
Fix PHPOffice#2257. Fix PHPOffice#2929. Fix PHPOffice#2935 (probably in a way that will not satisfy the requester). 2257 and 2929 requested changes that ultimately affect the same section of code, so it's appropriate to deal with them together. 2257 requests the ability to make the chart background transparent (so that the Excel gridlines are visible beneath the chart), and the ability to hide an Axis. 2929 requests the ability to set a gradient background on the chart.
Sorry, I don't think you've made a case for any code changes. You have made a case for documentation changes, and 2950, which will close this ticket when merged, will document (in both places) that scatterLines needs to be set at both the DS and DSV level. |
Ok. Your call certainly. Documenting will help avoid the possibility shown below using sample/Chart/33_Chart_create_scatter2.php. Case 1: As written - behavior as expected
Case 2: Modified to comment the SMOOTHMARKER designation. I suggest this might be reasonable thinking since, after all, I know one line will be smooth, one straight, and one has no line at all. To ensure the correct display, I have specified each DSV's line charactieristics. Since I don't want to have the series combination ($series) override each DSV assignment, I comment the last line,)
|
Fix #2257. Fix #2929. Fix #2935 (probably in a way that will not satisfy the requester). 2257 and 2929 requested changes that ultimately affect the same section of code, so it's appropriate to deal with them together. 2257 requests the ability to make the chart background transparent (so that the Excel gridlines are visible beneath the chart), and the ability to hide an Axis. 2929 requests the ability to set a gradient background on the chart.
### Added - Implementation of the new `TEXTBEFORE()`, `TEXTAFTER()` and `TEXTSPLIT()` Excel Functions - Implementation of the `ARRAYTOTEXT()` and `VALUETOTEXT()` Excel Functions - Support for [mitoteam/jpgraph](https://packagist.org/packages/mitoteam/jpgraph) implementation of JpGraph library to render charts added. - Charts: Add Gradients, Transparency, Hidden Axes, Rounded Corners, Trendlines, Date Axes. ### Changed - Allow variant behaviour when merging cells [Issue #3065](#3065) - Merge methods now allow an additional `$behaviour` argument. Permitted values are: - Worksheet::MERGE_CELL_CONTENT_EMPTY - Empty the content of the hidden cells (the default behaviour) - Worksheet::MERGE_CELL_CONTENT_HIDE - Keep the content of the hidden cells - Worksheet::MERGE_CELL_CONTENT_MERGE - Move the content of the hidden cells into the first cell ### Deprecated - Axis getLineProperty deprecated in favor of getLineColorProperty. - Moved majorGridlines and minorGridlines from Chart to Axis. Setting either in Chart constructor or through Chart methods, or getting either using Chart methods is deprecated. - Chart::EXCEL_COLOR_TYPE_* copied from Properties to ChartColor; use in Properties is deprecated. - ChartColor::EXCEL_COLOR_TYPE_ARGB deprecated in favor of EXCEL_COLOR_TYPE_RGB ("A" component was never allowed). - Misspelled Properties::LINE_STYLE_DASH_SQUERE_DOT deprecated in favor of LINE_STYLE_DASH_SQUARE_DOT. - Clone not permitted for Spreadsheet. Spreadsheet->copy() can be used instead. ### Removed - Nothing ### Fixed - Fix update to defined names when inserting/deleting rows/columns [Issue #3076](#3076) [PR #3077](#3077) - Fix DataValidation sqRef when inserting/deleting rows/columns [Issue #3056](#3056) [PR #3074](#3074) - Named ranges not usable as anchors in OFFSET function [Issue #3013](#3013) - Fully flatten an array [Issue #2955](#2955) [PR #2956](#2956) - cellExists() and getCell() methods should support UTF-8 named cells [Issue #2987](#2987) [PR #2988](#2988) - Spreadsheet copy fixed, clone disabled. [PR #2951](#2951) - Fix PDF problems with text rotation and paper size. [Issue #1747](#1747) [Issue #1713](#1713) [PR #2960](#2960) - Limited support for chart titles as formulas [Issue #2965](#2965) [Issue #749](#749) [PR #2971](#2971) - Add Gradients, Transparency, and Hidden Axes to Chart [Issue #2257](#2257) [Issue #2229](#2929) [Issue #2935](#2935) [PR #2950](#2950) - Chart Support for Rounded Corners and Trendlines [Issue #2968](#2968) [Issue #2815](#2815) [PR #2976](#2976) - Add setName Method for Chart [Issue #2991](#2991) [PR #3001](#3001) - Eliminate partial dependency on php-intl in StringHelper [Issue #2982](#2982) [PR #2994](#2994) - Minor changes for Pdf [Issue #2999](#2999) [PR #3002](#3002) [PR #3006](#3006) - Html/Pdf Do net set background color for cells using (default) nofill [PR #3016](#3016) - Add support for Date Axis to Chart [Issue #2967](#2967) [PR #3018](#3018) - Reconcile Differences Between Css and Excel for Cell Alignment [PR #3048](#3048) - R1C1 Format Internationalization and Better Support for Relative Offsets [Issue #1704](#1704) [PR #3052](#3052) - Minor Fix for Percentage Formatting [Issue #1929](#1929) [PR #3053](#3053)
This is:
N.B. DataSeries->plotStyle property value STYLE_LINEMARKER (='lineMarker') is currently examined only in Chart/Renderer/JpGraph.php, whose use is deprecated. Otherwise, DataSeries property value 'lineMarker' is unused.
Similarly, the DataSeries->smoothLine property is never tested, but its presence potentially confuses users who mistakenly use this (identically named) property and its (identically named) getter/setter methods in both DataSeries as well as DataSeriesValues.
What is the expected behavior?
When the user assigns the 'smoothLine' property to a DataSeriesValues array [via $DSV->set_smoothLine(true) ] then the Chart line should be displayed as a smooth Bezier line regardless of any property in its DataSeries, (in my opinion).
What is the current behavior?
If DataSeriesValues->smoothLine == TRUE its line will be straight if its DataSeries->plotStyle == 'lineMarker' (regardless of the value of DataSeries->smoothLine,) This is because Chart/Writer/Xlsx qualifies its writing of the <c:smooth> element with a logical AND of '$plotType ===DataSeries::TYPE_SCATTERCHART' and '$plotStyle' === 'smoothMarker'. Therefore, a DataSeriesValues array with a specified property value of TRUE for 'smoothLine' will not be expressed in the xml as <c:smooth val="1"> unless the DataSeries' plotStyle == 'smoothMarker'. Instead, <c:smooth> will be absent from the xml.
What are the steps to reproduce?
In example file samples/Chart/33_Chart_create_scatter2.php, change as follows:
WAS: DataSeries::STYLE_SMOOTHMARKER // plotStyle
IS: DataSeries::STYLE_LINEMARKER // plotStyle
This changes the original smooth (Bezier) line nature of the first DataSeriesValues series to a straight line point connection (which is the default behavior).
IS: // DataSeries::STYLE_LINEMARKER // plotStyle
Same result.
Neither result is affected by any value of property DataSeries->smoothLine, including null.
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:
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
Does an issue affect all spreadsheet file formats? If not, which formats are affected?
Xlsx is affected
Which versions of PhpSpreadsheet and PHP are affected?
1.24 7.3
The text was updated successfully, but these errors were encountered: