[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:       todd rme <toddrme2178 () gmail ! com>
Date:       2011-01-26 16:43:32
Message-ID: AANLkTinA6DAPRMgkZQohVnYgUCcZ71NCPJ_Txv+ePxR2 () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


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" fe=
ature in Picture Frame. If the User right clicks on Picture Frame, there wi=
ll be an option "Set Wallpaper Image". This Option will set the current Ima=
ge 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 o=
f Wallpaper other than Image and Slideshow. If there is better solution tha=
n 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 differen=
t
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 t=
o
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 woul=
d
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 an=
d
then set the image to that.

This also has the benefit that other widgets would also be able to hook int=
o
this and change the wallpaper without much work and without worrying about
what plugin is being used.

-Todd

[Attachment #5 (text/html)]

<div class="gmail_quote">On Wed, Jan 26, 2011 at 11:20 AM, Sinny Kumari <span \
dir="ltr">&lt;<a href="mailto:ksinny@gmail.com">ksinny@gmail.com</a>&gt;</span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex;">





 <div>
  <div style="font-family:Verdana, Arial, Helvetica, Sans-Serif">
   <table style="border:1px #c9c399 solid" bgcolor="#f9f3c9" cellpadding="8" \
width="100%">  <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 black solid" bgcolor="#fefadf" \
cellpadding="8" cellspacing="0" width="100%">  <tbody><tr>
  <td>

<div>Review request for Plasma.</div>
<div>By Sinny Kumari.</div>





<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Description </h1>
<table style="border:1px solid #b8b5a0" bgcolor="#ffffff" cellpadding="10" \
cellspacing="0" width="100%">  <tbody><tr>
  <td>
   <pre style="margin:0;padding:0;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-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 &quot;Set \
Wallpaper Image&quot; feature in Picture Frame. If the User right clicks on Picture \
Frame, there will be an option &quot;Set Wallpaper Image&quot;. This Option will set \
the current Image Of Picture Frame as Wallpaper Image.</pre>
  </td>
 </tr>
</tbody></table>


<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Testing </h1>
<table style="border:1px solid #b8b5a0" bgcolor="#ffffff" cellpadding="10" \
cellspacing="0" width="100%">  <tbody><tr>
  <td>
   <pre style="margin:0;padding:0;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-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:#575012;font-size:10pt;margin-top:1.5em">Diffs </h1>
<ul style="margin-left:3em;padding-left:0">

 <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>_______________________________________________<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>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&#39;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><br>-Todd<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