Review Request 123980: [Ark] Fix the information panel

[Raphael Kubo da Costa]

> On June 2, 2015, 7:43 p.m., Raphael Kubo da Costa wrote:
> > Separating bug fixes from new features is always wise.
> > 
> > In this case, the problem your other commit introduced can be fixed by just calling `QSplitter::setSizes()` after adding the widgets, just like it was done before. Suddenly your patch gets much smaller, self-contained and with much fewer risk of introducing new bugs :-)
> > 
> > Once we have something that is finally working properly, we can worry about other changes like switching from an IntList to a boolean and things like that.
> 
> Ragnar Thomsen wrote:
>     Thanks, I'll remember that for the next time :)

Actually, I'd like to do it this time to avoid introducing new bugs. A new RR could fix the bug, and this one just introduces these cosmetic changes to the code that are not that urgent.


> On June 2, 2015, 7:43 p.m., Raphael Kubo da Costa wrote:
> > part/part.cpp, lines 159-160
> > <https://git.reviewboard.kde.org/r/123980/diff/1/?file=378629#file378629line159>
> >
> >     Why not do this in `saveSplitterSizes()`?
> 
> Ragnar Thomsen wrote:
>     I thought about that, but it didnt feel logical to put it under a function named saveSplitterSizes. I can move it if you think it belongs there.

If the problem's the function name, we can rename it to something that makes more sense with the new code -- my point is that it is better to put all the code saving the splitter settings in one place (right now we're calling `ArkSettings::save()` in two different places, for example).


- Raphael


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


On June 2, 2015, 9:09 p.m., Ragnar Thomsen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123980/
> -----------------------------------------------------------
> 
> (Updated June 2, 2015, 9:09 p.m.)
> 
> 
> Review request for KDE Utils, Elvis Angelaccio and Raphael Kubo da Costa.
> 
> 
> Repository: ark
> 
> 
> Description
> -------
> 
> The logic governing hiding/showing of the infopanel as well as storing/retrieving the size of the QSplitter was reworked into a more intuitive and simple approach. A configuration option "showInfoPanel" of type bool was added, which stores whether or not the infopanel should be shown. The option "splitterSizesWithBothWidgets" was removed, since it's no longer needed. Default values of "splitterSizes" are now set in the kcfg file, instead of in the C++ code.
> 
> The infopanel show/hide logic has been broken in the frameworks branch since this commit:
> [b1ce0d272cff155fff54ca6d7cb90f90b9075490](http://quickgit.kde.org/?p=ark.git&a=commit&h=b1ce0d272cff155fff54ca6d7cb90f90b9075490)
> 
> 
> Diffs
> -----
> 
>   kerfuffle/ark.kcfg 97d2086 
>   part/part.h 6afd90f 
>   part/part.cpp 6fc9f55 
> 
> Diff: https://git.reviewboard.kde.org/r/123980/diff/
> 
> 
> Testing
> -------
> 
> Show/hide the information panel via the Settings menu works as expected. Ark remembers whether or not it was shown after closing/opening the program. The sizes of the splitter are also remembered after hiding/showing and after program restart. Deleting the splitterSizes line in ~/.config/arkrc and opening Ark results in the default sizes of 200/100 being used.
> 
> 
> Thanks,
> 
> Ragnar Thomsen
> 
>

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