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

Page fit and scaling not changing PDF output #3266

Closed
2 of 8 tasks
Brandin opened this issue Dec 30, 2022 · 9 comments · Fixed by #3279
Closed
2 of 8 tasks

Page fit and scaling not changing PDF output #3266

Brandin opened this issue Dec 30, 2022 · 9 comments · Fixed by #3279

Comments

@Brandin
Copy link

Brandin commented Dec 30, 2022

This is:

- [x] a bug report
- [ ] 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?

When I use the following two methods, both the width and height should output in a single page:

$timeSheetSpreadsheet->getActiveSheet()->getPageSetup()->setFitToWidth(1);
$timeSheetSpreadsheet->getActiveSheet()->getPageSetup()->setFitToHeight(0);

I would expect when not using the above, that scaling method below would reduce the right side of a page being cut off, and from rows pushing into sheet 2 as the sheet would be scaled.

$timeSheetSpreadsheet->getActiveSheet()->getPageSetup()->setScale(10);

What is the current behavior?

setFitToWidth and setFitToHeight are not not changing the output

setScale is not changing the output.

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';

        // Load the Excel sheets into PhpSpreadsheet
        $timesheetFilePath = $abs_us_root.$us_url_root.'time-sheets/'.Input::get('Timesheet');
        $timeSheetSpreadsheet = \PhpOffice\PhpSpreadsheet\IOFactory::load($timesheetFilePath);

        // Set fit to height and width - USE FIT OR SCALE, NOT BOTH
        $timeSheetSpreadsheet->getActiveSheet()->getPageSetup()->setFitToWidth(1);
        $timeSheetSpreadsheet->getActiveSheet()->getPageSetup()->setFitToHeight(0);

        // Set scale of the page - USE FIT OR SCALE, NOT BOTH
        $timeSheetSpreadsheet->getActiveSheet()->getPageSetup()->setScale(10);

        // Create a new PDF writer
        $writer = new \PhpOffice\PhpSpreadsheet\Writer\Pdf\DomPDF($timeSheetSpreadsheet);
        $fileName = 'Combined_Excel_Sheets.pdf';

        $currentUnixTimestamp = time();
        $fileName = 'Combined_'.date('Y-m-d').'_'.$currentUnixTimestamp.'.pdf';
        $writer->save($fileName);

Given the nature of the file, I cannot upload it, but I can provide it in private if that helps. I'm unsure what is causing this, as I've verified that changes to this code significantly is not changing the output - at all. I've outputted it and checked the new produced files and no changes to the scale/fit are occurring. I have commented out the opposing methods (I either use Fit or Scale, not both, since fit overrides scale.)

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, PDF

Which versions of PhpSpreadsheet and PHP are affected?

PhpSpreadsheet 1.26.0
PHP 8.1.6

@Brandin Brandin changed the title PDF scaling not changing output Page fit and scaling not changing PDF output Dec 30, 2022
@oleibman
Copy link
Collaborator

I have set up a private repository oleibman/Brandin that only you and I can access. Perhaps you can upload the file there.

@Brandin
Copy link
Author

Brandin commented Dec 31, 2022

I have set up a private repository oleibman/Brandin that only you and I can access. Perhaps you can upload the file there.

Excellent. I've gone ahead and pushed them. Let me know anything else you need to help.

@oleibman
Copy link
Collaborator

oleibman commented Jan 2, 2023

I have run some quick tests without using either scale or zoom.

For your ExpenseClaim file, I do not see a problem when using Mpdf. I do see some of the right-hand part is cut-off with Dompdf. I am not sure that we can do anything about that; I will continue to investigate.

For your TimeSheet file, I see that both Mpdf and Dompdf flow onto a second vertical page (and Dompdf loses some data on the right). This is true even if I use setFitToHeight(1). I will investigate what, if anything, can be done about that. I know how to set the Excel file to honor setFitToWidth/Height; I am not sure offhand how the equivalent would be performed for Pdf.

@oleibman
Copy link
Collaborator

oleibman commented Jan 2, 2023

Here is a workaround that eliminates the Height problem for Mpdf. It does not seem to be working for Dompdf; I need to research if anything extra is needed there. I also need to think about when it might be appropriate for PhpSpreadsheet to do this automatically so that you don't have to use this workaround (possibly do it if and only if sheet's fitToHeight is 1):

    function addAvoidPageBreak(string $html): string
    {
        return preg_replace('~</style>~', 'table.sheet0 {page-break-inside:avoid} </style>', $html);
    }

        $writer = new \PhpOffice\PhpSpreadsheet\Writer\Pdf\Mpdf($spreadsheet);

        $writer->setEditHtmlCallback('addAvoidPageBreak');
        $writer->save($outfile);

@oleibman
Copy link
Collaborator

oleibman commented Jan 2, 2023

For the height problem, it looks like Dompdf will honor break-inside rather than page-break-inside. Mpdf does not honor break-inside.

@oleibman
Copy link
Collaborator

oleibman commented Jan 2, 2023

@Brandin I have a tentative PR. It uses a very slightly modified version of your Timesheet file, removing the author's name and your name as author and maintainer. I don't see anything else which would seem to be a Privacy or Intellectual Property exposure, but perhaps I'm overlooking something. Please let me know if that would be okay.

@Brandin
Copy link
Author

Brandin commented Jan 3, 2023

@oleibman Thank you for the update.

As long as you remove the remainder of the details in the rows (mostly the A and B column data), no concerns.

@oleibman
Copy link
Collaborator

oleibman commented Jan 3, 2023

Okay, I will mask the details in columns A and B (there are only two rows). Expect the PR tomorrow or the next day.

FWIW, I believe that Dompdf is supposed to handle the CSS attribute transform, which could be used to fix the width problem, but it does not appear to work correctly. I have opened an issue there.

@oleibman
Copy link
Collaborator

oleibman commented Jan 3, 2023

Dompdf believes it is handling transform correctly. They suggested an additional parameter, which seems unsatisfactory in other ways.

oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Jan 4, 2023
Fix PHPOffice#3266. Each sheet in a spreadsheet can specify that it be set to fit width or height to a fixed number of pages. Mpdf and most browsers already handle the common case of fit to 1 page wide; I am unable to find a solution for Dompdf or Tcpdf. Code is added for the common case of fit to 1 page high when possible; this will usually work in Mpdf, Dompdf, and most browsers. I am not able to come up with a way to handle fit to more than 1 page wide or high.
oleibman added a commit that referenced this issue Jan 11, 2023
* Attempt To Honor Fit to 1-Page Height for Html/Pdf

Fix #3266. Each sheet in a spreadsheet can specify that it be set to fit width or height to a fixed number of pages. Mpdf and most browsers already handle the common case of fit to 1 page wide; I am unable to find a solution for Dompdf or Tcpdf. Code is added for the common case of fit to 1 page high when possible; this will usually work in Mpdf, Dompdf, and most browsers. I am not able to come up with a way to handle fit to more than 1 page wide or high.

* Synchronizer

Remove one unused assignment in test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants