From kde-panel-devel Tue Jan 26 09:47:12 2010 From: =?utf-8?q?Nicolas_L=C3=A9cureuil?= Date: Tue, 26 Jan 2010 09:47:12 +0000 To: kde-panel-devel Subject: Re: Review Request: Allow to use the wallpapermode defined on the Message-Id: <20100126094712.3650.9726 () localhost> X-MARC-Message: https://marc.info/?l=kde-panel-devel&m=126449928617822 > On 2010-01-23 20:45:32, Aaron Seigo wrote: > > is this really needed? see: http://techbase.kde.org/Projects/Plasma/Theme#Wallpaper_Access > > Nicolas Lécureuil wrote: > in fact i see : > > [Wallpaper] > defaultTheme= > defaultFileSuffix= > defaultWidth= > defaultHeight= > > But i don't see wallpaperplugin= and wallpaperpluginmode= which can be used for slideshow or other modes. > > Aaron Seigo wrote: > ah, the actual plugin being used; i don't think this should be specific to the Containment, really; it also would belong in Plama::Theme alongside wallpaperPath. perhaps Plasma::Theme::wallpaperPluginName and wallpaperPluginMode? hm.. that starts to look clumsy. maybe what would be better is a small class called WallpaperDefaults or WallpaperConfig (though that might be slightly misleading) and have a "WallpaperDefaults Plama::Theme::wallpaper()" method. WallpaperDefaults would have: > > QString path() const; > QString plugin() const; > QString mode() const; > > alternatively, we could have a WallpaperConfig class that has the above methods, along with setters, and a retore(KConfigGroup &group) method which would read wallpaperplugin and wallpaperpluginmode entries, perhaps the image path if any as well?, from the config group. Containment could then use that (and have a Containment::setWallpaper(const WallpaperConfig &config) method?) > > this would also mean deprecating Theme::wallpaperPath() and Containment::setWallpaper(QString, QString). > > it might also be an idea to allow the Wallpaper plugin itself have access to this config object > > the end result of this would be: > > * encapsulating the wallpaper default and configuration much more neatly > * keep all the wallpaper values in the desktop theme and available from Plasma::Theme > * providing some standardization of config values for wallpaper plugins? > > peronally, i think the patch you posted is Ok as a "get it done quickly" fix that Mandriva may want to include in packages; but i'd like to see something more robust, like the above, for our upstream sources. (and it really shouldn't be THAT much code. :) we could keep using the same config file values you introduce in your patch as well, that part looks fine. > > what do you think? i trust you regarding plasma :) i will try to do this ( if nobody does it before ) - Nicolas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/2702/#review3816 ----------------------------------------------------------- On 2010-01-23 19:58:30, Nicolas Lécureuil wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/2702/ > ----------------------------------------------------------- > > (Updated 2010-01-23 19:58:30) > > > Review request for Plasma. > > > Summary > ------- > > this patch add the possibility to define on the metadata.desktop the kind of wallpaper we want to use. > If none is defined in the file, the default one will be used. > > > Diffs > ----- > > /trunk/KDE/kdelibs/plasma/containment.cpp 1078694 > > Diff: http://reviewboard.kde.org/r/2702/diff > > > Testing > ------- > > > Thanks, > > Nicolas > > _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel