Review Request: Ability to select which screen savers are used for "Random"

Oswald Buddenhagen ossi at kde.org
Sat Sep 22 10:28:35 BST 2012


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


this UI is completely opaque to the user and thus not acceptable.
you have two options:
- add a proper header to the tree and put the checkboxes into a separate column "allow in 'random'". the list's double function is still confusing, though.
- put it into a separate config dialog after all. with some minor refactoring there shouldn't be much code duplication. i'd prefer this approach.

i'm pretty sure this patch will massively conflict with http://git.reviewboard.kde.org/r/106124/

please use kdelibs (or even better qt, as far as i'm concerned) coding style for new code. at the very least stay consistent with yourself.




kcontrol/screensaver/scrnsave.cpp
<http://git.reviewboard.kde.org/r/106524/#comment15270>

    endsWith



kcontrol/screensaver/scrnsave.cpp
<http://git.reviewboard.kde.org/r/106524/#comment15271>

    unrelated style change. don't do that.


- Oswald Buddenhagen


On Sept. 21, 2012, 12:57 p.m., Jonathan Marten wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106524/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2012, 12:57 p.m.)
> 
> 
> Review request for KDE Runtime.
> 
> 
> Description
> -------
> 
> The referenced bug suggests that it should be possible to individually select screen savers that are to be considered when the "Random" option is chosen.  This patch implements that.
> 
> In order to avoid having to duplicate the complete saver tree view (and the code to generate it) within the random saver's setup mode, check boxes are added to the kcontrol saver list (with the exception of the random saver itself).  The state of these is saved to the random saver's config file.  This selection is in addition to the two options within the saver's setup dialogue, so if for example "Use OpenGL screen savers" is not checked than any OpenGL savers will be ignored even if their boxes are checked in the tree view.
> 
> (Submitting to kde-runtime, there is no kde-workspace group)
> 
> 
> This addresses bug 57572.
>     http://bugs.kde.org/show_bug.cgi?id=57572
> 
> 
> Diffs
> -----
> 
>   kcontrol/screensaver/scrnsave.h 7c8deba 
>   kcontrol/screensaver/scrnsave.cpp c0507d4 
>   kscreensaver/krandom_screensaver/random.cpp 4047184 
> 
> Diff: http://git.reviewboard.kde.org/r/106524/diff/
> 
> 
> Testing
> -------
> 
> Built saver and kcontrol module with these changes.  Checked operation of kcontrol module, saving of settings in the config file and operation of the random saver.
> 
> 
> Screenshots
> -----------
> 
> kcmshell4 screensaver
>   http://git.reviewboard.kde.org/r/106524/s/733/
> 
> 
> Thanks,
> 
> Jonathan Marten
> 
>

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


More information about the kde-core-devel mailing list