----------------------------------------------------------- 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 > >
