[Kde-imaging] Review Request 123918: KPBatchProgressDialog: Port from KDialog to QDialog

[Gilles Caulier]

> On May 28, 2015, 7:07 a.m., Gilles Caulier wrote:
> > The jpeglossless tool is obsolete and will be removed with KF5 port. You can drop to patch this code here.
> > This widget is also used in FlashExport and KMLExport tools. Try to perform tests with these tools.
> > Else, the KPBatchProgressDialog port sound fine for me.
> 
> Alexander Potashev wrote:
>     Why don't we remove the whole PORT.KF5/OBSOLETE/ directory from the frameworks branch then? If there is a little chance that someone will maintain these plugins, we should maintain consistency at least at the level of obvious API changes.
>     
>     FlashExport does not use class KPBatchProgressDialog, it uses only KPBatchProgressWidget.
>     
>     KMLExport uses KPBatchProgressDialog very poorly: it does not support canceling, and it closes the progress dialog immediately after finishing the export. The only way you can see the dialog is when your images do not contain geotags: in this case you get a messagebox on top of the progress dialog, and the progress dialog is not closed until you accept the message box. I think KMLExport needs a major bugfixing or a rewrite. So there is nothing to test in KPBatchProgressDialog with KMLExport at this point.

>Why don't we remove the whole PORT.KF5/OBSOLETE/ directory from the frameworks branch then? If there is a little chance that someone will maintain these plugins, we should maintain consistency at least at the level of obvious API changes.

Because I don't find time to do it yet. You can do it if you want.

In all case, these tool are obsolete and can be safety deleted. There is no reason to port it in the future.


- Gilles


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123918/#review80920
-----------------------------------------------------------


On May 29, 2015, 4:31 p.m., Alexander Potashev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123918/
> -----------------------------------------------------------
> 
> (Updated May 29, 2015, 4:31 p.m.)
> 
> 
> Review request for Kipi-plugins.
> 
> 
> Repository: kipi-plugins
> 
> 
> Description
> -------
> 
> Behaviour:
> * Clicking "Cancel" does not reject the dialog so it is not closed. This
>     is new behaviour.
> * Clicking "Close" (after Cancel button changed to Close) accepts and
>     closes the dialog.
> 
> Implementation details:
> * The Cancel/Close button is implemented as Close button in a
>     QDialogButtonBox.
> * Added method setButtonClose() for turning the "Cancel" button into
>     "Close" button. A plugin should call this when some error occurs and
>     it cannot continue its job. Because there is nothing to cancel
>     anymore, the button should be renamed to "Close".
> * The Cancel/Close button has standard text/tooltip/icon/whatsthis
>     defined in KF5::WidgetsAddons' KStandardGuiItem.
> * The signal cancelClicked() is emitted when user clicks Cancel/Close
>     like before.
> 
> 
> Diffs
> -----
> 
>   PORT.KF5/OBSOLETE/jpeglossless/plugin/plugin_jpeglossless.cpp 55bef63f320557bf1100f7a5fc1391f9e612f193 
>   common/libkipiplugins/CMakeLists.txt 618cc143ab8e18cec3cd1396efe138755ee8ddb7 
>   common/libkipiplugins/dialogs/kpbatchprogressdialog.h 853fa2ec8c4afadc66a7c08994c0aec903e1f15a 
>   common/libkipiplugins/dialogs/kpbatchprogressdialog.cpp 4fd2bbd0df8177c9b1791c28e1450772c072c688 
>   sendimages/sendimages.cpp de91084912e027330a3b9cff22bac3b6db3fdc3e 
> 
> Diff: https://git.reviewboard.kde.org/r/123918/diff/
> 
> 
> Testing
> -------
> 
> The progress dialog in SendImages can be canceled (by clicking Cancel) and then closed (by clicking the same button again after it changed to "Close"). If I don't cancel it, KMail starts successfully and the Cancel button turns into Close which can then be clicked to close the dialog and remove the temporary files.
> 
> KMLExport uses KPBatchProgressDialog very poorly: it does not support canceling, and it closes the progress dialog immediately after finishing the export. The only way you can see the dialog is when your images do not contain geotags: in this case you get a messagebox on top of the progress dialog, and the progress dialog is not closed until you accept the message box. I think KMLExport needs a major bugfixing or a rewrite. So there is nothing to test in KPBatchProgressDialog with KMLExport at this point.
> 
> 
> Thanks,
> 
> Alexander Potashev
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-imaging/attachments/20150603/5991aae1/attachment.html>