PHPOffice / PhpSpreadsheet

A pure PHP library for reading and writing spreadsheet files
https://phpspreadsheet.readthedocs.io
MIT License
13.32k stars 3.45k forks source link

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

Open rolinger opened 2 years ago

rolinger commented 2 years ago

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

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

Which versions of PhpSpreadsheet and PHP are affected?

MarkBaker commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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?

rolinger commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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.