Review Request: In Konqueror's session restore dialog, allow user to chose which sessions to restore

[Dawit Alemayehu]

> On Oct. 8, 2012, 10:30 a.m., David Faure wrote:
> > konqueror/src/konqsessionmanager.cpp, line 123
> > <http://git.reviewboard.kde.org/r/106503/diff/6/?file=88345#file88345line123>
> >
> >     Did you change this if() wrongly?
> >     
> >     Now the code says "if I could find the dialog-warning icon, then ignore it and ask QStyle instead".
> >     
> >     This makes no sense ;)

I did not change anything. That is exactly how it is in KMessageBox::createKMessageBox. Also I do not see how you can read the if statement the way you did. To me the if statement reads, "if you find the dialog-warning icon, then use the metrics from the current QStyle when setting it". 


> On Oct. 8, 2012, 10:30 a.m., David Faure wrote:
> > konqueror/src/konqsessionmanager.cpp, line 145
> > <http://git.reviewboard.kde.org/r/106503/diff/6/?file=88345#file88345line145>
> >
> >     All this generic code could be simplified for the use case at hand. Maybe after inserting a '\n' in the string (between the two sentences), and then all that squeezing and wrapping can be removed. Let's keep this simple and maintainable.

No need to add '\n' and hard code the line break. Simply setting the label wrapable and removing all the wrapping and squeezing related code is sufficient.


- Dawit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106503/#review20065
-----------------------------------------------------------


On Oct. 4, 2012, 3:50 a.m., Dawit Alemayehu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106503/
> -----------------------------------------------------------
> 
> (Updated Oct. 4, 2012, 3:50 a.m.)
> 
> 
> Review request for KDE Base Apps and David Faure.
> 
> 
> Description
> -------
> 
> The attached patch fixes one of those pet peeve bugs that infurate me from time to time by allowing me to unselect the sessions I do not want to be restored when Konqueror's restore session dialog pops up.
> 
> 
> This addresses bug 260282.
>     http://bugs.kde.org/show_bug.cgi?id=260282
> 
> 
> Diffs
> -----
> 
>   konqueror/src/konqsessionmanager.h ee629e4 
>   konqueror/src/konqsessionmanager.cpp 68a003f 
> 
> Diff: http://git.reviewboard.kde.org/r/106503/diff/
> 
> 
> Testing
> -------
> 
> * Unselected sessions should not be restored.
> * If all available sessions are selected (the default), the behavior should remain the same as it is today.
> * If all available sessions are unselected, disable the "Restore Session" button.
> 
> 
> Screenshots
> -----------
> 
> old restore dialog
>   http://git.reviewboard.kde.org/r/106503/s/729/
> new restore dialog
>   http://git.reviewboard.kde.org/r/106503/s/731/
> new restore dialog v2
>   http://git.reviewboard.kde.org/r/106503/s/739/
> new restore dialog v3
>   http://git.reviewboard.kde.org/r/106503/s/750/
> 
> 
> Thanks,
> 
> Dawit Alemayehu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20121008/1af28d81/attachment.htm>