[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request: Adding "Set Wallpaper Image" Option in Picture
From: Sinny Kumari <ksinny () gmail ! com>
Date: 2011-01-26 19:29:29
Message-ID: AANLkTim4+JtdOgZT7dAosB2dR8hhoSyDFZXzNkNWYqBO () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Right now, when an image is dragged and dropped to the Plasma Desktop there
will be an option "Image".
It sets that Image as wallpaper and changes modes to Image which is default
mode. So, In order to maintain
consistency I did the same thing.
On Thu, Jan 27, 2011 at 12:06 AM, todd rme <toddrme2178@gmail.com> wrote:
>
> On Wed, Jan 26, 2011 at 12:46 PM, Sinny Kumari <ksinny@gmail.com> wrote:
>
> >
> >
> > On Wed, Jan 26, 2011 at 10:13 PM, todd rme <toddrme2178@gmail.com> wrote:
> >
> > > On Wed, Jan 26, 2011 at 11:20 AM, Sinny Kumari <ksinny@gmail.com> wrote:
> > >
> > > > This is an automatically generated e-mail. To reply, visit:
> > > > http://svn.reviewboard.kde.org/r/6416/
> > > > Review request for Plasma.
> > > > By Sinny Kumari.
> > > > Description
> > > >
> > > > Including this patch and other two (http://svn.reviewboard.kde.org/r/6375/ , \
> > > > http://reviewboard.kde.org/r/6391/ ) , It adds "Set Wallpaper Image" feature \
> > > > in Picture Frame. If the User right clicks on Picture Frame, there will be an \
> > > > option "Set Wallpaper Image". This Option will set the current Image Of \
> > > > Picture Frame as Wallpaper Image.
> > > >
> > > > Testing
> > > >
> > > > Setting Picture Frame Image as wallpaper Image in all cases. Added QTimer in \
> > > > method Frame::setImageAsWallpaper() in Order to work properly in case of \
> > > > Wallpaper other than Image and Slideshow. If there is better solution than \
> > > > using QTimer, please suggest :)
> > > > Diffs
> > > >
> > > > - trunk/KDE/kdeplasma-addons/applets/frame/frame.h (1216449)
> > > > - trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp (1216449)
> > > >
> > > > View Diff <http://svn.reviewboard.kde.org/r/6416/diff/>
> > > >
> > > > _______________________________________________
> > > > Plasma-devel mailing list
> > > > Plasma-devel@kde.org
> > > > https://mail.kde.org/mailman/listinfo/plasma-devel
> > > >
> > > >
> > >
> > > I see one issue with this approach: it hard-codes how to deal with
> > > different plugins. What happens to the weather wallpaper? What happens to
> > > wallpaper clock or day/night wallpaper? What if someone writes their own
> > > version of the slideshow plugin? Is someone going to rewrite this every
> > > time a new plugin is developed?
> > >
> > > Might it be a better approach to have some general API for setting images
> > > to a wallpaper? The plugin would set up its own method of dealing with the
> > > pictures sent to it (which could include ignoring it). The the default
> > > plugin and virus plugin would just set the image, the slideshow plugin would
> > > add it to the list, the weather plugin would set it to the current weather
> > > condition, the day/night and clock plugins would set it to the current time.
> > >
> > > As a fall-back, if the plugin does not support the API, plasma would
> > > change to the normal wallpaper image and use the image there. So plugins
> > > that don't use images, like marble and mandelbrot, could just not implement
> > > that API, in which case selecting an image would change to the default
> > > plugin and then set the image to that.
> > >
> > > This also has the benefit that other widgets would also be able to hook
> > > into this and change the wallpaper without much work and without worrying
> > > about what plugin is being used.
> > >
> > > -Todd
> > >
> > > _______________________________________________
> > > Plasma-devel mailing list
> > > Plasma-devel@kde.org
> > > https://mail.kde.org/mailman/listinfo/plasma-devel
> > >
> > >
> >
> > This Patch works for all cases. Suppose current wallpaper is virus. If
> > User has images in Picture frame and if he likes to set the Picture frame
> > Image as wallpaper image. He can do so. "set wallpaper Image" sets the
> > current image as wallpaper image and Wallpaper Plugin will be set to Image.
> >
> > --
> > http://www.sinny.in
> >
> > _______________________________________________
> > Plasma-devel mailing list
> > Plasma-devel@kde.org
> > https://mail.kde.org/mailman/listinfo/plasma-devel
> >
> >
> Why should the plugin be set to image? I picked the virus plugin as an
> example because it uses an image as a wallpaper, but then manipulates the
> image. So if someone is using the virus wallpaper, and they select "set
> wallpaper Image", I think the expected behavior would be that the virus
> plugin starts using that image, not to change the plugin entirely.
> Similarly, if someone is using the weather wallpaper, and they select "set
> wallpaper Image", I think the expected behavior would be to use the image
> for the current weather, not to change the plugin. The same goes for the
> day/night wallpaper and the wallpaper clock. These all use images in
> various ways. I don't think people would be expecting this option to change
> the plugin, rather I think they would expect it to use the image in the
> plugin.
>
> Then you add situations I mentioned like someone making a photo montage
> wallpaper, which scatters a bunch of pictures over the desktop. Would you
> modify this function to deal with that properly, or would you erase their
> montage?
>
> I think the only way to do this properly is to let the plugins decide for
> themselves how to handle pictures sent to them. If a plugin doesn't provide
> a way to handle pictures, only then do you resort to changing the plugin.
>
> -Todd
>
> _______________________________________________
> Plasma-devel mailing list
> Plasma-devel@kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
>
>
--
http://www.sinny.in
[Attachment #5 (text/html)]
Right now, when an image is dragged and dropped to the Plasma Desktop there will be \
an option "Image".<br>It sets that Image as wallpaper and changes modes to \
Image which is default mode. So, In order to maintain<br> consistency I did the same \
thing. <br><br><div class="gmail_quote">On Thu, Jan 27, 2011 at 12:06 AM, todd rme \
<span dir="ltr"><<a \
href="mailto:toddrme2178@gmail.com">toddrme2178@gmail.com</a>></span> \
wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; \
border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"> <br><div \
class="gmail_quote"><div><div></div><div class="h5">On Wed, Jan 26, 2011 at 12:46 PM, \
Sinny Kumari <span dir="ltr"><<a href="mailto:ksinny@gmail.com" \
target="_blank">ksinny@gmail.com</a>></span> wrote:</div> </div><blockquote \
class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, \
204, 204); padding-left: 1ex;"><div><div></div><div class="h5">
<div><div></div><div><br>
<br><div class="gmail_quote">On Wed, Jan 26, 2011 at 10:13 PM, todd rme <span \
dir="ltr"><<a href="mailto:toddrme2178@gmail.com" \
target="_blank">toddrme2178@gmail.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, \
204, 204); padding-left: 1ex;">
<div class="gmail_quote"><div><div></div><div>On Wed, Jan 26, 2011 at 11:20 AM, Sinny \
Kumari <span dir="ltr"><<a href="mailto:ksinny@gmail.com" \
target="_blank">ksinny@gmail.com</a>></span> wrote:<br></div> </div><blockquote \
class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, \
204, 204); padding-left: 1ex;"><div><div></div><div>
<div>
<div style="font-family: Verdana,Arial,Helvetica,Sans-Serif;">
<table style="border: 1px solid rgb(201, 195, 153);" width="100%" \
bgcolor="#f9f3c9" cellpadding="8"> <tbody><tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/6416/" \
target="_blank">http://svn.reviewboard.kde.org/r/6416/</a> </td>
</tr>
</tbody></table>
<br>
<table style="background-repeat: repeat-x; border: 1px solid black;" width="100%" \
bgcolor="#fefadf" cellpadding="8" cellspacing="0"> <tbody><tr>
<td>
<div>Review request for Plasma.</div>
<div>By Sinny Kumari.</div>
<h1 style="color: rgb(87, 80, 18); font-size: 10pt; margin-top: 1.5em;">Description \
</h1> <table style="border: 1px solid rgb(184, 181, 160);" width="100%" \
bgcolor="#ffffff" cellpadding="10" cellspacing="0"> <tbody><tr>
<td>
<pre style="margin: 0pt; padding: 0pt; white-space: pre-wrap; word-wrap: \
break-word;">Including this patch and other two (<a \
href="http://svn.reviewboard.kde.org/r/6375/" \
target="_blank">http://svn.reviewboard.kde.org/r/6375/</a> , <a \
href="http://reviewboard.kde.org/r/6391/" \
target="_blank">http://reviewboard.kde.org/r/6391/</a> ) , It adds "Set \
Wallpaper Image" feature in Picture Frame. If the User right clicks on Picture \
Frame, there will be an option "Set Wallpaper Image". This Option will set \
the current Image Of Picture Frame as Wallpaper Image.</pre>
</td>
</tr>
</tbody></table>
<h1 style="color: rgb(87, 80, 18); font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table style="border: 1px solid rgb(184, 181, 160);" width="100%" bgcolor="#ffffff" \
cellpadding="10" cellspacing="0"> <tbody><tr>
<td>
<pre style="margin: 0pt; padding: 0pt; white-space: pre-wrap; word-wrap: \
break-word;">Setting Picture Frame Image as wallpaper Image in all cases. Added \
QTimer in method Frame::setImageAsWallpaper() in Order to work properly in case of \
Wallpaper other than Image and Slideshow. If there is better solution than using \
QTimer, please suggest :)</pre>
</td>
</tr>
</tbody></table>
<h1 style="color: rgb(87, 80, 18); font-size: 10pt; margin-top: 1.5em;">Diffs </h1>
<ul style="margin-left: 3em; padding-left: 0pt;">
<li>trunk/KDE/kdeplasma-addons/applets/frame/frame.h <span style="color: \
grey;">(1216449)</span></li>
<li>trunk/KDE/kdeplasma-addons/applets/frame/frame.cpp <span style="color: \
grey;">(1216449)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/6416/diff/" style="margin-left: 3em;" \
target="_blank">View Diff</a></p>
</td>
</tr>
</tbody></table>
</div>
</div>
<br></div></div>_______________________________________________<br>
Plasma-devel mailing list<br>
<a href="mailto:Plasma-devel@kde.org" target="_blank">Plasma-devel@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/plasma-devel" \
target="_blank">https://mail.kde.org/mailman/listinfo/plasma-devel</a><br> \
<br></blockquote></div><br><br>I see one issue with this approach: it hard-codes how \
to deal with different plugins. What happens to the weather wallpaper? What happens \
to wallpaper clock or day/night wallpaper? What if someone writes their own version \
of the slideshow plugin? Is someone going to rewrite this every time a new plugin is \
developed?<br>
<br>Might it be a better approach to have some general API for setting images to a \
wallpaper? The plugin would set up its own method of dealing with the pictures sent \
to it (which could include ignoring it). The the default plugin and virus plugin \
would just set the image, the slideshow plugin would add it to the list, the weather \
plugin would set it to the current weather condition, the day/night and clock plugins \
would set it to the current time.<br>
<br>As a fall-back, if the plugin does not support the API, plasma would change to \
the normal wallpaper image and use the image there. So plugins that don't use \
images, like marble and mandelbrot, could just not implement that API, in which case \
selecting an image would change to the default plugin and then set the image to \
that.<br>
<br>This also has the benefit that other widgets would also be able to hook into this \
and change the wallpaper without much work and without worrying about what plugin is \
being used.<br><font color="#888888"><br>-Todd<br>
</font><br>_______________________________________________<br>
Plasma-devel mailing list<br>
<a href="mailto:Plasma-devel@kde.org" target="_blank">Plasma-devel@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/plasma-devel" \
target="_blank">https://mail.kde.org/mailman/listinfo/plasma-devel</a><br> \
<br></blockquote></div><br><br>This Patch works for all cases. Suppose current \
wallpaper is virus. If User has images in Picture frame and if he likes to set the \
Picture frame<br>Image as wallpaper image. He can do so. "set wallpaper \
Image" sets the current image as wallpaper image and Wallpaper Plugin will be \
set to Image.<br clear="all"><br></div></div></div></div><font color="#888888">-- \
<br><a href="http://www.sinny.in" target="_blank">http://www.sinny.in</a><br> \
</font><div class="im"><br>_______________________________________________<br> \
Plasma-devel mailing list<br> <a href="mailto:Plasma-devel@kde.org" \
target="_blank">Plasma-devel@kde.org</a><br> <a \
href="https://mail.kde.org/mailman/listinfo/plasma-devel" \
target="_blank">https://mail.kde.org/mailman/listinfo/plasma-devel</a><br> \
<br></div></blockquote></div><br>Why should the plugin be set to image? I picked the \
virus plugin as an example because it uses an image as a wallpaper, but then \
manipulates the image. So if someone is using the virus wallpaper, and they select \
"set wallpaper Image", I think the expected behavior would be that the \
virus plugin starts using that image, not to change the plugin entirely. Similarly, \
if someone is using the weather wallpaper, and they select "set wallpaper \
Image", I think the expected behavior would be to use the image for the current \
weather, not to change the plugin. The same goes for the day/night wallpaper and the \
wallpaper clock. These all use images in various ways. I don't think people \
would be expecting this option to change the plugin, rather I think they would expect \
it to use the image in the plugin.<br>
<br>Then you add situations I mentioned like someone making a photo montage \
wallpaper, which scatters a bunch of pictures over the desktop. Would you modify \
this function to deal with that properly, or would you erase their montage?<br>
<br>I think the only way to do this properly is to let the plugins decide for \
themselves how to handle pictures sent to them. If a plugin doesn't provide a \
way to handle pictures, only then do you resort to changing the plugin.<br> <font \
color="#888888">
<br>-Todd<br>
</font><br>_______________________________________________<br>
Plasma-devel mailing list<br>
<a href="mailto:Plasma-devel@kde.org">Plasma-devel@kde.org</a><br>
<a href="https://mail.kde.org/mailman/listinfo/plasma-devel" \
target="_blank">https://mail.kde.org/mailman/listinfo/plasma-devel</a><br> \
<br></blockquote></div><br><br clear="all"><br>-- <br><a href="http://www.sinny.in" \
target="_blank">http://www.sinny.in</a><br>
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic