[Differential] [Commented On] D3131: Show full scrollbar only on mouse over

[hpereiradacosta (Hugo Pereira Da Costa)]

hpereiradacosta added a comment.


  Just added a  couple of comments inline.
  Also, (and somewhat out of topic), playing with the scrollbar width is rather easy: just change
  
    ScrollBar_Extend,
    ScrollBar_SliderWidth,
    ScrollBar_MinSliderHeight
  
  In breeze.h
  I experienced with using 14, 6 and 6 for these dimensions, and the result is quite pleasing I think (6 is the progressbar groove width).
  Feel free to commit this in a separate post if vdg is happy about it.
  Now making this configurable is a complete different story, and in fact it is in "todo list" because ultimately, I think we want a slim (or compact) mode for breeze, to please people who complain about it wasting too much space.
  (but this is completely out of topic :))

INLINE COMMENTS

> breezescrollbardata.h:109
>              addLineAnimation().data()->setDuration( duration );
>              subLineAnimation().data()->setDuration( duration );
>          }

Here you miss a grooveAnimation().data()->setDuration( duration );

This makes sure that the groove animation is also updated "on fly" when animation durations is changed (otherwise the change is only taken into account for newly created scrollbars)

> breezestyle.cpp:4964
>          const qreal opacity( _animations->scrollBarEngine().opacity( widget, SC_ScrollBarSlider ) );
> +        QColor color = _helper->scrollBarHandleColor( palette, mouseOver, hasFocus, opacity, mode );
>  

can be made a const again, right ?

> breezestyle.cpp:4966
>  
> -        const QColor color( _helper->scrollBarHandleColor( palette, mouseOver, hasFocus, opacity, mode ) );
> -        _helper->renderScrollBarHandle( painter, handleRect, color );
> +        if( StyleConfigData::scrollBarShowOnMouseOver() ) {
> +            // finally, global opacity when ScrollBarShowOnMouseOver

this entire block can go, right ? 
In fact my compiler complains about unused variable

> breezestyle.cpp:6374
> +
> +        if (opacity == -1) opacity = 1;
>  

you could use AnimationData::OpacityInvalid instead of -1

REPOSITORY
  rBREEZE Breeze

REVISION DETAIL
  https://phabricator.kde.org/D3131

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma, #breeze, #vdg, hpereiradacosta
Cc: plasma-devel, abetts, lesliezhai, ali-mohamed, jensreuterberg, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20161028/48491cfc/attachment.html>