Review Request 121997: Implement configuration dialog in Ark

[Elvis Angelaccio]

> On Giu. 3, 2015, 5:59 p.m., Ragnar Thomsen wrote:
> > I tested it and it works great :) thanks for doing this.
> > 
> > I think we need to discuss how Ark remembers the extraction settings now that we have two places where they can be set. I see these possibilities:
> > 
> > 1. Always use the settings from the settings page. This means that if the user changes the settings from the extraction dialog, then these settings are used only for that particular extraction. We should emphasize in the settings dialog, that these are the default settings.
> > 2. When the user changes the settings in the extraction dialog, store these settings and use them in the future. This means making sure the settings page displays the updated settings.
> > 
> > What do you think? I think option one makes most sense.

Option two is what is already done by Ark. We could migrate to option one, but changing the behavior of the extraction dialog would be too risky, imho. Long-time Ark users could be confused/annoyed (e.g. "Why I have to check again option foo?!?"). Let's just say that the extraction dialog allows the user to quickly access those settings, without having to open the settings dialog first.


> On Giu. 3, 2015, 5:59 p.m., Ragnar Thomsen wrote:
> > kerfuffle/ark.kcfg, line 27
> > <https://git.reviewboard.kde.org/r/121997/diff/5/?file=378719#file378719line27>
> >
> >     Why were the labels deleted here?

They were not used, were they?
Plus, I rephrased them in the actual configuration dialog.


> On Giu. 3, 2015, 5:59 p.m., Ragnar Thomsen wrote:
> > part/dialogs/previewsettings.ui, line 23
> > <https://git.reviewboard.kde.org/r/121997/diff/5/?file=378726#file378726line23>
> >
> >     I propose to change the text to:
> >     "Disable preview for files larger than:"
> >     
> >     and then put the QSpinBox directly after this text (i.e. delete the QLabel).

Are you also implying to drop the related boolean setting?


> On Giu. 3, 2015, 5:59 p.m., Ragnar Thomsen wrote:
> > part/dialogs/previewsettings.ui, line 77
> > <https://git.reviewboard.kde.org/r/121997/diff/5/?file=378726#file378726line77>
> >
> >     I think 1000MB might be to low for maximum. Some users might want to set it higher. Should we up it to 10000?

Maybe, not sure about this...


- Elvis


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


On Giu. 3, 2015, 4:50 p.m., Elvis Angelaccio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121997/
> -----------------------------------------------------------
> 
> (Updated Giu. 3, 2015, 4:50 p.m.)
> 
> 
> Review request for KDE Utils and Raphael Kubo da Costa.
> 
> 
> Bugs: 165314
>     http://bugs.kde.org/show_bug.cgi?id=165314
> 
> 
> Repository: ark
> 
> 
> Description
> -------
> 
> This patch implements the standard Configuration Dialog (i.e. using `KConfigDialog`) in Ark, as one would expect from a KDE application. 
> This feature has been requested in more than one bug. I choosed to target bug 165314 since the others are more like "add the config dialog to implement the feature *foo*".
> 
> The widgets showed in the config dialog are provided by the Ark `Part` interface (just one widget for now). This should help to show the Ark settings in, for instance, Konqueror's config dialog. A similar approach is done in Kate and Cantor, from what I have seen.
> 
> I don't like very much the `document-save` icon used for the Extraction page, but I couldn't find any better.
> 
> 
> Diffs
> -----
> 
>   app/mainwindow.h f2366c8bd047e3484daa4eabeb4f4b790c437c4a 
>   app/mainwindow.cpp 470b72d00d0544662f2c1f10dbc0638b99f993af 
>   kerfuffle/ark.kcfg 97d2086688698e96c429def089c50ff3cdbe4c4e 
>   part/CMakeLists.txt cca6d25e1941cecde07d0cd342cc2602fbc5235e 
>   part/arkconfigpage.h PRE-CREATION 
>   part/arkconfigpage.cpp PRE-CREATION 
>   part/dialogs/extractionsettings.h PRE-CREATION 
>   part/dialogs/extractionsettings.ui PRE-CREATION 
>   part/dialogs/previewsettings.h PRE-CREATION 
>   part/dialogs/previewsettings.ui PRE-CREATION 
>   part/interface.h 40f590284502d23a2a4ffaa333bfd5b63e6ec773 
>   part/part.h 4c81da5541c1143ad4cfe9093f9c307145ac561e 
>   part/part.cpp ee7f75ff9acd8ea3c72aa5a400d713ffc6d1c7c4 
> 
> Diff: https://git.reviewboard.kde.org/r/121997/diff/
> 
> 
> Testing
> -------
> 
> Compile and run, then try to change some settings and check that they persist.
> 
> 
> File Attachments
> ----------------
> 
> extraction-settings.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/06/03/3dfe3110-4f46-4c14-9b19-79060577a4c9__extraction-settings.png
> preview-settings1.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/06/03/92e1e6a4-6b6a-4a2f-b149-809501173925__preview-settings1.png
> preview-settings2.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/06/03/200d41a5-6f06-4bea-9f92-d8899b051bf4__preview-settings2.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-utils-devel/attachments/20150603/2f79879d/attachment-0001.html>