> 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 > >
