Review Request: fix rendering of klineedit clear button

[Michael Pyne]

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

Ship it!


I'd just ask for some spacing in the ternary operator you have to select the enabled or disable pixmap (QIcon::Enabled:QIcon::Disabled seems a little much ;) but other than that it looks good and seems to work well here.

- Michael


On 2010-04-25 03:46:50, Hugo Pereira Da Costa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3804/
> -----------------------------------------------------------
> 
> (Updated 2010-04-25 03:46:50)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> this patch
> - fixes the rendering of animated klineedit clear button, to use QPainter compositing mode rather than QPainter::setOpacity to fake semi-transparent clearButton during animation. This should be more efficient as QPainter::setOpacity (last time I checked) was using the rasterEngine. 
> 
> - uses a QIcon rather than QPixmap to store the clearButton pixmap. This allows to have the proper icon effect when rendering enabled/disabled klineedit. 
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kdeui/widgets/klineedit_p.h 1118236 
> 
> Diff: http://reviewboard.kde.org/r/3804/diff
> 
> 
> Testing
> -------
> 
> in konqueror and dolphin, as well as custom made test widgets.
> Test done in klineedit, as well as editable kcombobox
> Using either oxygen, cleanlooks and plastik style. 
> 
> Finally, tested with/without the animation being enabled
> 
> 
> Screenshots
> -----------
> 
> incorrect rendering of disabled clear-button
>   http://reviewboard.kde.org/r/3804/s/372/
> incorrect rendering of disabled clear-button
>   http://reviewboard.kde.org/r/3804/s/373/
> 
>   http://reviewboard.kde.org/r/3804/s/374/
> 
> 
> Thanks,
> 
> Hugo
> 
>