Review Request: port kdelib's tooltip shadow code to icontasks, and remove halo

[Aaron J. Seigo]

> On Jan. 9, 2013, 9:17 a.m., Mark Gaiser wrote:
> > I cannot imagine that code duplication is the way to go here. Surely there is a smarter way that doesn't involve duplicating code. There where other duplicated code paths before in KDE like the tooltip stuff and the rules to blur behind a window. The tooltip mess is still active, the blur behind is finally solved but that's mainly because nearly all apps added the needed lines to enable that feature.
> > 
> > If you duplicate this class then it's bound to be a problematic part in the future. I strongly recommend finding a different solution that does not involve duplicating this code.
> 
> Cedric Bellegarde wrote:
>     In fact, tooltip.cpp in icontasks is already forked code from kdelibs...
>     
>     But i think my one line patch from https://git.reviewboard.kde.org/r/108241/ should be ok, no?
> 
> Mark Gaiser wrote:
>     I'm not going to judge, but it "seems" better then duplicating a class.
> 
> Xuetian Weng wrote:
>     Cedric Bellegarde, no, since disable overriding the shadow will only give tooltip default oxygen shadow, instead of the correct plasma svg based one. Check other theme like Slim Glow, I don't think your patch will still work.
>     
>     Mark Gaiser, for long term solution, every one should use Plasma::Dialog even for Tooltip, but Plasma::Dialog still missing one api to override the imagePath to use, so it's not a good idea to open this dup class up in kdelibs. And actually, this part of code is being copied to kde-workspace once. Instead of modifying the kdelibs API, this patch is the only way to go for 4.10.

agreed; short term solution for 4.10 is to duplicate this one class. this is inevitable since icontasks already copied (forked, really) some code from libplasma. (and this is a good reason why not to do that :)

long term, icontasks needs to use either Plasma::ToolTip or Plasma::Dialog. preferably ToolTip. in both cases, it will probably mean some modifications to ToolTip or Dialog which should be done in libplasma2 (or, well, in the QML components that replace those bits)


- Aaron J.


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


On Jan. 8, 2013, 2:06 p.m., Xuetian Weng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108265/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2013, 2:06 p.m.)
> 
> 
> Review request for Plasma, Cedric Bellegarde and Aaron J. Seigo.
> 
> 
> Description
> -------
> 
> copy dialogshadows to icontasks, and use DialogShadows to handle icon tasks shadow.
> 
> This is a different solution of https://git.reviewboard.kde.org/r/108241/ , I guess this is what it should be.
> 
> (remove halo part is just to keep consistency with default tooltip)
> 
> 
> Diffs
> -----
> 
>   applets/icontasks/CMakeLists.txt 1ff6042 
>   applets/icontasks/tooltips/dialogshadows.cpp PRE-CREATION 
>   applets/icontasks/tooltips/dialogshadows_p.h PRE-CREATION 
>   applets/icontasks/tooltips/tooltip.cpp 43f09ed 
>   applets/icontasks/tooltips/tooltipmanager.cpp dd36a9e 
>   applets/icontasks/tooltips/windowpreview.cpp 94c9eb2 
> 
> Diff: http://git.reviewboard.kde.org/r/108265/diff/
> 
> 
> Testing
> -------
> 
> tested, no problem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130109/dc0a2656/attachment-0001.html>