GibbonEdu / core

Gibbon is a flexible, open source school management platform designed to make life better for teachers, students, parents and leaders.
https://gibbonedu.org
GNU General Public License v3.0
479 stars 304 forks source link

Fix page sizes #1715

Closed Devon-Olivier closed 1 year ago

Devon-Olivier commented 1 year ago

Description

Change 'letter' pageSize to 'LETTER' for TCPDF Renderer to work for US Letter page size.

Motivation and Context

When "US Letter" page size is chosen in Home > Reports > Template Builder > Edit Template for a template using the TCPDF renderer, generated reports still remain in the A4 page size. Things work as expected with the mPDF renderer.

How Has This Been Tested?

Enviroment: Gibbon v25.0.00 PHP 8.1.13 Mariadb 10.10.2

These are run in docker containers on a linux host.

Screenshots

SKuipers commented 1 year ago

Hi Devon-Olivier, huge thanks for submitting this PR! My apologies for the wait, I generally try to look at PRs sooner than this, but it's been a busy Chinese New Year season here, right on the heels of a busy v25 release.

Your changes are looking good overall. I wonder about existing data though, as the string is being compared case sensitively, so it looks like it wouldn't catch any existing templates with the page size already lowercase.

An alternative approach is to use the strtoupper() function on the value that is passed into the TCPDF renderer, this way regardless of the source case, it will end up the correct case. In the template, the |upper twig filter could be added, to ensure that it's always checking the same case, for example {% if pageSize|upper == 'LETTER' %}. Do you want to have a look at making these changes, then I'm happy to merge this one in. Thanks! 😃