Add PDF orientation and page size #5679
Conversation
| table-layout:fixed !important; | ||
| width: 20cm !important; | ||
| } | ||
| width: 100% !important; |
There was a problem hiding this comment.
Why comment this out, now jasp-table will be cut-off while printing a multiple cols table. I think overlap is better than cut off because cut off cannot be noticed easily by users.
There was a problem hiding this comment.
This css obliged the table to be 20 cm wide (even if the table is self much smaller). This allows table that are wider than 20 cm to fit in a A4 format, but most of the time it is not a valid solution since the table is squeezed, and the columns overlapped each other.
To set it explicitly to 20cm does not work anyway for landscape print (or for other page size).
If I set the the width to 100% (with the table-layout fixed), then it fits the landscape size, but then all tables are as wide as the whole width of the page.
Unfortunately, the max-width does not work with table-layout fixed, this would be in fact what we are looking for.
As I cannot see a right solution here, I prefer to remove this setting.
I find the argument that the cut-off might not be noticed by users, a bit poor compared to the problems this setting generate. I find we see quite easily that a part of a table misses when it is too big.
There was a problem hiding this comment.
Yes, set 20cm explicitly is only for A4.
There was a problem hiding this comment.
We could set the exact size for each different combination of pagesize + orientation. But I wouldnt want to hold up the PR for that :p
|
Btw I cannot build your branch with new module installer things, error code was same as the github action failed info. maybe @vandenman know what happend. |
Desktop/gui/preferencesmodel.h
Outdated
| @@ -70,9 +72,16 @@ class PreferencesModel : public PreferencesModelBase | |||
| Q_PROPERTY(bool checkUpdatesAskUser READ checkUpdatesAskUser WRITE setCheckUpdatesAskUser NOTIFY checkUpdatesAskUserChanged ) | |||
| Q_PROPERTY(bool checkUpdates READ checkUpdates WRITE setCheckUpdates NOTIFY checkUpdatesChanged ) | |||
| Q_PROPERTY(int maxScaleLevels READ maxScaleLevels WRITE setMaxScaleLevels NOTIFY maxScaleLevelsChanged ) | |||
| Q_PROPERTY(QVariantList pdfPageSizeModel READ pdfPageSizeModel CONSTANT ) | |||
| Q_PROPERTY(int pdfPageSize READ pdfPageSize WRITE setPdfPageSize NOTIFY pdfPageSizeChanged ) | |||
| Q_PROPERTY(int pdfOrientation READ pdfOrientation WRITE setPdfOrientation NOTIFY pdfOrientationChanged ) | |||
There was a problem hiding this comment.
Why the hell not just a bool portrait instead of 3 ints???
And why not bool a4 iinstead of whatever int pdfPageSize is supposed to be?
There was a problem hiding this comment.
So ignore the pagesize thing ;)
| #include "enumutilities.h" | ||
|
|
||
| DECLARE_ENUM(pdfOrientation, portrait = 0, landscape); // Cf https://doc.qt.io/qt-6/qpagelayout.html#Orientation-enum | ||
| DECLARE_ENUM(pdfPageSize, letter = 0, legal, executive, A0, A1, A2, A3, A4, A5, A6); // Cf https://doc.qt.io/qt-6/qpagesize.html#PageSizeId-enum |
There was a problem hiding this comment.
Ok pagesize has more options then I thought. But the question still stands for portrait and landscape...
| table-layout:fixed !important; | ||
| width: 20cm !important; | ||
| } | ||
| width: 100% !important; |
There was a problem hiding this comment.
We could set the exact size for each different combination of pagesize + orientation. But I wouldnt want to hold up the PR for that :p
* Add PDF orientation and page size * Use a boolean for lansscape/portrait setting
Fixes jasp-stats/jasp-issues#2918