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

Mark Gaiser markg85 at gmail.com
Wed Jan 9 09:17:10 UTC 2013


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


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.

- Mark Gaiser


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/9166e14b/attachment.html>


More information about the Plasma-devel mailing list