----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5705/#review8425 -----------------------------------------------------------
i like the idea a lot. there are two issues that must be addressed before it can be committed, however. one is quite trivial: put curly braces around the one liner if statements, kdelibs style. e.g.: the other is almost as trivial: the new checkboxes need the labels fixed. they also need to be put into the layout properly like the rest of the widgets: a QLabel on right with the text, checkbox (with empty text!) on the right. all the widgets should line up from top to bottom along the "center" line of the label<->widget margin. with those fixes, this can go in. thanks for the patch :) /trunk/KDE/kdebase/workspace/plasma/generic/wallpapers/image/image.cpp <http://svn.reviewboard.kde.org/r/5705/#comment8739> use braces, even on single line statements. /trunk/KDE/kdebase/workspace/plasma/generic/wallpapers/image/slideshowconfig.ui <http://svn.reviewboard.kde.org/r/5705/#comment8741> should be plural and use sentence capitalization: System wallpaper /trunk/KDE/kdebase/workspace/plasma/generic/wallpapers/image/slideshowconfig.ui <http://svn.reviewboard.kde.org/r/5705/#comment8740> should be plural, and use sentence capitalization: "My downloaded wallpapers" - Aaron On 2010-10-28 15:28:16, Jeremy Whiting wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://svn.reviewboard.kde.org/r/5705/ > ----------------------------------------------------------- > > (Updated 2010-10-28 15:28:16) > > > Review request for Plasma and Davide Bettio. > > > Summary > ------- > > Prompted by bug https://bugs.kde.org/show_bug.cgi?id=253360 and my own itch > at not having the ability to easly add ~/.kde/share/wallpaper/ to the > slideshow, I added these two checkboxes from this wishlist bug. Let me know > if you think the tooltips should be reworded I just took an initial stab at > them. > > > Diffs > ----- > > /trunk/KDE/kdebase/workspace/plasma/generic/wallpapers/image/image.h > 1190556 > /trunk/KDE/kdebase/workspace/plasma/generic/wallpapers/image/image.cpp > 1189844 > > /trunk/KDE/kdebase/workspace/plasma/generic/wallpapers/image/slideshowconfig.ui > 1189844 > > Diff: http://svn.reviewboard.kde.org/r/5705/diff > > > Testing > ------- > > I works quite well actually. I still think we need to disable the apply and > ok buttons when no checkbox is checked and no paths are in the listbox, but > we work around that by putting the system folder in the list if there are no > folders in the kconfig. > > > Thanks, > > Jeremy > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
