Review Request: Enable kdialog to set an icon for passivepopup as well

[Kai Uwe Broulik]

> On May 6, 2011, 1:03 p.m., Christoph Feck wrote:
> > kdialog/kdialog.cpp, line 381
> > <http://git.reviewboard.kde.org/r/101282/diff/4/?file=16242#file16242line381>
> >
> >     Do not create a new KIconLoader, but use the global one.
> >     
> >     The other issue is the hardcoded size. While the example uses 32 pixels, it does not imply the class does not handle different sizes.
> >     
> >     But if the fallback does not work anyway, just forget about the changes in this section (as long as you don't want to find the cause for the bug). Sorry for the noise.

Okay, I will look into it :)
I thought something like that will come *g* I think that Dialog icon style will automatically provide a decent size, so just removing that parameter should do it.
As you can see by my code I am not (yet :P) an experienced coder… I think we can just leave the fallback as is (without custom icon support).

I think there are no use cases for the fallback anyway:
* If Plasma is not running (e. g. you create a "rescue console" with a simple bash script or something) those passive popups will look ugly and be useless (since there is nothing else running then) and you rather use a normal dialog instead
* If Plasma is running, you have the nice Plasma notifications
* If you are not running KDE at all, you would probably not use KDialog at all (dependencies!) and use notify-send instead for such a purpose which then is handled by the respective desktop environment.


- Kai Uwe


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101282/#review3158
-----------------------------------------------------------


On May 6, 2011, 12:16 p.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101282/
> -----------------------------------------------------------
> 
> (Updated May 6, 2011, 12:16 p.m.)
> 
> 
> Review request for KDE Base Apps.
> 
> 
> Summary
> -------
> 
> Since I use kdialog a lot, I wanted it to be able to set an icon for passive Plasma notifications as well.
> I saw that the icon was hardcoded to dialog-information and tried to enable it to parse the --icon command line argument and pass it to the dbus call that triggers the notification.
> I could not test the patch however, since I do not (yet) have a full development environment or its dependencies, and I do not even know if this is the right approach, but I hope you can have a look at it.
> 
> 
> Diffs
> -----
> 
>   kdialog/kdialog.cpp b80ad9a 
> 
> Diff: http://git.reviewboard.kde.org/r/101282/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kai Uwe
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110506/d5ad3db5/attachment.htm>