> 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=<name of default wallpaper package> > defaultFileSuffix=<wallpaper file suffix, e.g. .jpg> > defaultWidth=<width in pixels of default wallpaper file> > defaultHeight=<height in pixels of default wallpaper file> > > 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
