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

Reassigning chart getName to getIndex and adding a proper getName. #2991

Open
7 tasks
rolinger opened this issue Aug 5, 2022 · 6 comments
Open
7 tasks

Reassigning chart getName to getIndex and adding a proper getName. #2991

rolinger opened this issue Aug 5, 2022 · 6 comments
Labels

Comments

@rolinger
Copy link

rolinger commented Aug 5, 2022

This is:

- [ ] a bug report
- [ ] a feature request
- [ x] change chart object identifiers

In excel, there are three references to charts:
chartIndex - index of all charts, starting with chart1 (top left most chart on sheet1), across all sheets, ending in chartX (bottom right most chart on sheetX). Adding, deleting charts will re-index all the charts in order. In phpspreadsheet, this is represented by $chart->getName()
chartName - above column A, there is a name field you can edit. In phpspreadsheet, there appears no way to get/set this value, please correct me if I am wrong.
chartTitle - the visual chart title, in the chart (IE: "Annual Sales Revenue"). In phpspreadsheet, this is represented by $chart->getTitle()->getCaptionText().

It took me a while to get to the bottom of all this as it kept tripping me up as I kept misinterpreting the close but different meanings of these values between excel and phpspreadsheet and what represented what.

Is it possible for these phpspreadsheet object methods to be renamed to align with excel?

getName() to getIndex()
create a new getName() that actually gets to the chart name
getTitle() would stay the same.

Unfortunately, its not easy to rely on the chart index, esp for an imported templates, because of adding/deleting/moving charts will reindex all of them making it challenging to know the exact chart you need to be working with. Thus adding a new proper getName() can allow us to tag/name a chart that we can then easily reference, the user never needs to change the name, just use it as a solid reference in order to edit/copy/duplicate/delete charts.

In my case, in my template I want to be name a chart Chart_Monthly. Using phpspreadsheet, change its title to January Sales, then later grab/copy Chart Monthly to create a new chart titled February Sales, etc. But because I might have other charts between January & February sales, I can't easily rely on the current getName() (aka: index) to retrieve the original January chart to copy. Also, this way, one wouldn't have to rely on tracking changing chart titles to find the original chart you wanted to copy.

I know this ask might be a long shot and probably has some versioning issues to simply rename certain object methods, but I thought I would throw it out there and see.

What features do you think are causing the issue

  • Reader
  • Writer
  • Styles
  • Data Validations
  • Formula Calculations
  • [x ] 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?

@MarkBaker
Copy link
Member

In excel, there are three references to charts: chartIndex - index of all charts, starting with chart1 (top left most chart on sheet1), across all sheets, ending in chartX (bottom right most chart on sheetX). Adding, deleting charts will re-index all the charts in order. In phpspreadsheet, this is represented by $chart->getName() chartName - above column A, there is a name field you can edit. In phpspreadsheet, there appears no way to get/set this value, please correct me if I am wrong. chartTitle - the visual chart title, in the chart (IE: "Annual Sales Revenue"). In phpspreadsheet, this is represented by $chart->getTitle()->getCaptionText().

chartIndex is simply a unique reference to the chart. It doesn't always reflect it's position on the page, but more closely the order in which charts were created (or loaded from file).
The editable name field in MS Excel isn't loaded when the chart is loaded; If a non-default name exists, then it's maintained in the xdr:cNvPr element of the xdr:nvGraphicFramePr in the drawing xml file. This element isn't currently read by the loader, only the coordinate information is actually loaded from that file. The name assigned by the reader is the actual internal filename.

Is it possible for these phpspreadsheet object methods to be renamed to align with excel?

getName() to getIndex() create a new getName() that actually gets to the chart name getTitle() would stay the same.

Not without a BC break, which means a new major version release. Version 2.0 is already close to release, so it may be possible to add this feature.

@rolinger
Copy link
Author

rolinger commented Aug 8, 2022

ugh, ok....I am trying to clone a chart:

$newChart = clone $chart
$sheet->addChart($newChart) ;

however, $newChart has the same getName() as the original $chart. So when $newChart is added the chartCollection it now has two chart4 in it. And there doesn't appear to be a setName('chart5') method that would allow me to change the cloned name before adding it back to the collection. Is there another method that I could use to clone a chart?

Essentially, clone the chart and change the dataseries...mainly just the data values. All I can find references to is creating new charts with new data series, but nothing to that would allow me to change existing data series.

@oleibman
Copy link
Collaborator

oleibman commented Aug 8, 2022

The omission of a setName method seems unintentional, and is very easy to rectify. Adding a deep-copy clone, on the other hand, seems difficult. Is adding setName sufficient for your purposes (at least for now), and, if not, what else would you require?

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Aug 8, 2022
Addresses a problem identified in issue PHPOffice#2991. Chart name is set in constructor, but there is no method to subsequently change it. This PR adds a method to do so.
@rolinger
Copy link
Author

rolinger commented Aug 9, 2022

@oleibman - I think the only issue with my above method is adding the cloned new object with the same getName(), so a setName('chartX) should be all that is necessary.

Template with 5 charts, each titled: chart_10, chart_20, chart_30, chart_40, chart_50

$newChart = clone $chart  ;  // chart3
$newChart->getTitle()->setCaption("chart_60") ;
$sheet->addChart($newChart) ; 
$newChart->getTitle()->setCaption("chart_70") ;
$sheet->addChart($newChart) ; 
$newChart->getTitle()->setCaption("chart_80") ;
$sheet->addChart($newChart) ; 

$indexCount = 0 ;
foreach ($sheet->getSheetByName()->getChartCollection() as $chart) {
   echo "\n$indexCount : " .$chart->getName(). ", name: " .$chart->getTitle()->getCaptionText() ;
   $indexCount++ ;
}

Outs:

0 : chart1, name: chart_10
1 : chart2, name: chart_20
2 : chart3, name: chart_80
3 : chart4, name: chart_40
4 : chart5, name: chart_50
5 : chart3, name: chart_80
6 : chart3, name: chart_80
7 : chart3, name: chart_80

Because chart3 is the same getName(), all chart3 in the object get updated with the same new setCaption(), its walking the whole object finding all chart3 and changing the titles.

Changing setName() on the cloned object before doing the addChart($newChart) should clear up the issue.

Having a setName() would allow:

$newChart = clone $chart  ;  // chart3
$newChart->setName('chart6') ;
$newChart->getTitle()->setCaption("chart_60") ;
$sheet->addChart($newChart) ; 
$newChart->setName('chart7') ;
$newChart->getTitle()->setCaption("chart_70") ;
$sheet->addChart($newChart) ; 
$newChart->setName('chart8') ;
$newChart->getTitle()->setCaption("chart_80") ;
$sheet->addChart($newChart) ; 

@rolinger
Copy link
Author

rolinger commented Aug 9, 2022

And I just tried another method, instead of looping through the chartCollection(), I just added the new chart then just updated that specific index chart title. Tracking all the charts with $chartIndex, the last one added will be $chartIndex - 1. However, the same thing still happened, it updated all chart3 with the new title.

     $mainChartSheet = $mainTempFile->getSheetByName("Chart Data") ;
     $mainChartSheet->addChart($newChart) ;
     $chartIndex++ ;
     $mainTempFile->getSheetByName("Chart Data")->getChartByIndex($chartIndex-1)->getTitle()->setCaption($newCName) ;

@rolinger
Copy link
Author

rolinger commented Aug 9, 2022

And surprisingly, even though all the cloned $newChart still have the same getName() (ie: chart3), the coordinates for each one get updated correctly (20 rows apart), but the setCaption() updates all chart3 with the same title. Based on the behavior of the setCaption() I was surprised to see the setTopLeftCell()/setBottomRightCell() was setting correctly, i was expecting them to all have the same coordinates, based on the last one applied.

 $pMatch = "/(?=\d)/" ;
 $topLeft = preg_split($pMatch,$chart->getTopLeftCell(),2) ;
 $botRight = preg_split($pMatch,$chart->getBottomRightCell(),2) ;
                
 $topLeftCol = $topLeft[0] ;
 $topLeftRow = $topLeft[1] + 20 ;
 $botRightCol = $botRight[0] ;
 $botRightRow = $botRight[1] + 20 ;


 $newChart = clone $chart ;
 $newChart->getTitle()->setCaption($newCName) ;
 $newChart->setTopLeftCell($topLeftCol.$topLeftRow) ;
 $newChart->setBottomRightCell($botRightCol.$botRightRow) ;
 $sheet->addChart($newChart) ;

$newChart = clone $chart ; = should be a whole NEW object, yet when the $newChart cell coordinates are updated correctly, the getTitle()->setCaption('newName') still has some reference the original object which updates all of them. I can't make sense of it.

oleibman added a commit that referenced this issue Aug 13, 2022
Addresses a problem identified in issue #2991. Chart name is set in constructor, but there is no method to subsequently change it. This PR adds a method to do so.
MarkBaker added a commit that referenced this issue Sep 25, 2022
### 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)
guillaume-ro-fr added a commit to guillaume-ro-fr/PhpSpreadsheet that referenced this issue Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants