A bit ugly attempt to implement print preview with an additional configuration widget.
Details
Details
- Reviewers
tcanabrava - Group Reviewers
KDE Edu - Commits
- R334:1414513d4479: Print preview for KmPlot
- Plot something.
- Choose "File -> Print Preview"
- Press the last button on the toolbar to configure the print options.
- Press the "Ok" button.
- The preview should be updated with the parameters chosen on step 3.
- Press the "Print" button. The plot with the chosen parameters should be printed (identical to the preview).
I need an advice on the quality of the code.
Many thanks in advance for your reviews.
Diff Detail
Diff Detail
- Repository
- R334 KmPlot
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
| kmplot/maindlg.cpp | ||
|---|---|---|
| 703–704 | The same pattern can be found above. Just for other possible reviewers, the discussions on this topic can be found by the following addresses https://blogs.kde.org/node/3919 https://phabricator.kde.org/D7285 https://phabricator.kde.org/D17697 It seems that this is a questionable decision but it might be of some sense. Thanks in advance for your opinions. | |
Comment Actions
I feel there's some code that could be shared between this function and the one above
| kmplot/maindlg.cpp | ||
|---|---|---|
| 235 ↗ | (On Diff #49527) | maybe use a new style connect here? |
| kmplot/maindlg.cpp | ||
|---|---|---|
| 235 ↗ | (On Diff #49527) | There is no addAction(KStandardAction::StandardAction, QString, QObject, Func) prototype in KActionCollection, so it's OK. |
Comment Actions
Then it looks ok to me (well there's no need to move some of the includes to the header, with fordward declarations it would be enough, but who cares i guess :D)
Let's see if @tcanabrava has something else to say :)
| kmplot/maindlg.cpp | ||
|---|---|---|
| 235 ↗ | (On Diff #49527) | oh, someone should fix that :) |