Review Request: fix rendering of klineedit clear button

Hugo Pereira Da Costa hugo at oxygen-icons.org
Sun Apr 25 18:57:52 BST 2010


Hi Aaron,
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3804/#review5220
> -----------------------------------------------------------
>
> Ship it!
>
>    

Done.

> looks good; one other change that should be made is to use QVariantAnimation now that it exists; it's rather more efficient (shared timers, etc) than QTimeLine. that shouldn't be a blocker to this commit, just something else that could be done at some point. in fact, if you don't, i will after you commit this. :)
>
>    
I agree with the QVariantAnimation (and can code it, it's up to you )

Also, along the line of using QIcon in place of QPixmap, I was thinking 
of catching Hover events,
so that you have the same (usually highlight) effect on the icon as in 
toolbars, when hovering over the button.
This way you actually have a visual feedback that pressing the icon will 
do something.
Does that make sense ? (I would still need to try first).
> - Aaron
>
>
> 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
>>
>>
>>      
>    





More information about the kde-core-devel mailing list