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

Add support for vertical Gridlines #2969

Closed
2 of 8 tasks
bridgeplayr opened this issue Jul 29, 2022 · 5 comments · Fixed by #3018
Closed
2 of 8 tasks

Add support for vertical Gridlines #2969

bridgeplayr opened this issue Jul 29, 2022 · 5 comments · Fixed by #3018
Labels

Comments

@bridgeplayr
Copy link

This is:

- [ ] a bug report
- [x] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

What is the current behavior?

Supported in Excel; not supported in PHPSS

What are the steps to reproduce?

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:

<?php

require __DIR__ . '/vendor/autoload.php';

// Create new Spreadsheet object
$spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet();

// add code that show the issue here...

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

  • Reader
  • Writer
  • Styles
  • Data Validations
  • Formula Calculations
  • Charts
  • AutoFilter
  • Form Elements

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

Xlsx

Which versions of PhpSpreadsheet and PHP are affected?

1.23 7.3

@bridgeplayr
Copy link
Author

This Chart generator is similar to the one in #2967

33_LineChart_DateAxis_VertGridlines.php.txt

  • This issue resolution builds on the modifications suggested in Add support for Date Axis in Line Charts #2967 (I should probably have combined these two issues.)

  • Modify Writer/Xlsx/Chart.php : change private function writePlotArea() to support $majorGridlines & $minorGridlines



        if (($chartType !== DataSeries::TYPE_PIECHART) && ($chartType !== DataSeries::TYPE_PIECHART_3D) && ($chartType !== DataSeries::TYPE_DONUTCHART)) {
            if ($chartType === DataSeries::TYPE_BUBBLECHART) {
                $this->writeValueAxis($objWriter, $xAxisLabel, $chartType, $id2, $id1, $catIsMultiLevelSeries, $xAxis, $majorGridlines, $minorGridlines);
            } else {
                // $this->writeCategoryAxis($objWriter, $xAxisLabel, $id1, $id2, $catIsMultiLevelSeries, $xAxis);
                $this->writeCategoryAxis($objWriter, $xAxisLabel, $id1, $id2, $catIsMultiLevelSeries, $xAxis, $majorGridlines, $minorGridlines);
            }


  • change argument list for private function writeCategoryAxis(), and add writing xml supporting majorGridlines and minorGridlines.
    private function writeCategoryAxis(XMLWriter $objWriter, ?Title $xAxisLabel, $id1, $id2, $isMultiLevelSeries, Axis $yAxis, GridLines $majorGridlines, GridLines $minorGridlines): void
    {
        // N.B. writeCategoryAxis may be invoked with $xAxis substituted for the last parameter($yAxis) for ScatterChart, etc
        // In that case, xAxis may contain values like the yAxis, or it may be a date axis (LINECHART).
        $AxisType = $yAxis->getAxisType() ;
        if ($AxisType !== '') {
            $objWriter->startElement('c:' . $AxisType);
        } elseif ($yAxis->getAxisIsNumericFormat()) {
            $objWriter->startElement('c:valAx');
        } else {
            $objWriter->startElement('c:catAx');
        }

        if ($majorGridlines->getObjectState()) {
          $objWriter->startElement('c:majorGridlines');
          $objWriter->startElement('c:spPr');

          $this->writeLineStyles($objWriter, $majorGridlines);

          $objWriter->startElement('a:effectLst');
          $this->writeGlow($objWriter, $majorGridlines);
          $this->writeShadow($objWriter, $majorGridlines);
          $this->writeSoftEdge($objWriter, $majorGridlines);

          $objWriter->endElement(); //end effectLst
          $objWriter->endElement(); //end spPr
          $objWriter->endElement(); //end majorGridLines
        }

        if ($minorGridlines->getObjectState()) {
            $objWriter->startElement('c:minorGridlines');
            $objWriter->startElement('c:spPr');

            $this->writeLineStyles($objWriter, $minorGridlines);

            $objWriter->startElement('a:effectLst');
            $this->writeGlow($objWriter, $minorGridlines);
            $this->writeShadow($objWriter, $minorGridlines);
            $this->writeSoftEdge($objWriter, $minorGridlines);
            $objWriter->endElement(); //end effectLst

            $objWriter->endElement(); //end spPr
            $objWriter->endElement(); //end minorGridLines
        }

33_LineChart_DateAxis_Gridline.xlsx

@oleibman
Copy link
Collaborator

Does #2923, which was merged in time for the 1.24 release, not take care of this?

@bridgeplayr
Copy link
Author

bridgeplayr commented Jul 29, 2022 via email

@bridgeplayr
Copy link
Author

bridgeplayr commented Jul 29, 2022 via email

@MarkBaker
Copy link
Member

MarkBaker commented Jul 29, 2022

[I also want to follow up issue #2823 on DateRendering, which is not done correctly in DATEVALUE. I had to invent a new function to correctly render “4/1/2001” and distinguish it from “1/4/2001”. I can do it correctly in pure php functions, but PHPSS’s DATEVALUE implementation has no option to force it to follow the user’s date specification. My implementation comment for issue #2967 includes my function xlDateValue($datestr) which returns the correct numeric date when the date string is formatted as mm/dd/yyyy. This one is pretty high on my bitch-list.]

MS Excel differentiates 1/4/2001 from 4/1/2001 based on the locale settings: entering '1/4/2001' when locale is set to us would set the internal serialized timestamp value to 36895 (4th January 2001); when the locale is not us, entering '1/4/2001' then the internal serialized timestamp value is 36982 (1st April 2001).
As PhpSpreadsheet doesn't maintain a locale for date/time settings, and uses basic PHP rules for converting a date string to an Excel serialized timestamp (if the separator between day and month is a /, then it assumes US). The internal conversion code does try to correct this if it can recognise that the resulting date would be invalid (e.g. 13/1 couldn't be us, because there is no month 13 in the calendar), but can't disambiguate 1/4 from 4/1, because both could be valid.

Until such time as we provide a locale setting (remembering that this would have to be set by the developer, even if we provided a default, because it isn't maintained in anywhere in an Excel file), then any changes to date conversion from string would be a bc break

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Aug 20, 2022
Fix PHPOffice#2967. Fix PHPOffice#2969 (which had already been fixed prior to opening the issue, but had added urgency for Date Axes). Add ability to set axis type to date axis, in addition to original possiblities of value axis and category axis.
oleibman added a commit that referenced this issue Aug 25, 2022
* Charts - Add Support for Date Axis

Fix #2967. Fix #2969 (which had already been fixed prior to opening the issue, but had added urgency for Date Axes). Add ability to set axis type to date axis, in addition to original possiblities of value axis and category axis.

* Update 33_Chart_create_line_dateaxis.php

No idea why php-cs-fixer is complaining. It didn't do so when I first uploaded. I can't duplicate problem on my own system. Not enough detail in error message for me to act. Grasping at straws, I have moved the function definition (which is the only use of braces in the entire script) from the end of the script to the beginning.

* Update 33_Chart_create_line_dateaxis.php

Some comments were mis-aligned. This may be related to the reasons behind PR #3025, which didn't take care of this because this script had not yet been merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

3 participants