D9792: even slimmer scrollbars

[Vlad Zagorodniy]

zzag added a comment.


  I'm sorry, but these scrollbars have too thin sliders.
  
  F5801991: scrollbar.png 
  
  Slider occupies only 15% of space in horizontal direction, that's a "big" 
waste of space.

REPOSITORY
  R31 Breeze

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: zzag, rikmills, davidedmundson, ngraham, colomar, abetts, plasma-devel, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
mart, hein

D9792: even slimmer scrollbars

[Rik Mills]

rikmills added a comment.


  In https://phabricator.kde.org/D9792#193697, @hpereiradacosta wrote:
  
  > In https://phabricator.kde.org/D9792#193473, @rikmills wrote:
  >
  > > In Neon and Kubuntu CI with Qt 5.9.3
  > >
  > > 
https://build.neon.kde.org/job/xenial_unstable_plasma_breeze_bin_amd64/186/console
  > >
  > > 17:55:15 /workspace/build/kstyle/breezestyle.cpp: In member function 
‘virtual bool Breeze::Style::drawScrollBarSliderControl(const QStyleOption*, 
QPainter*, const QWidget*) const’:
  > >  17:55:15 /workspace/build/kstyle/breezestyle.cpp:5013:136: error: ‘const 
class QStyleOption’ has no member named ‘styleObject’
  > >  17:55:15  const bool widgetMouseOver( widget ? 
_animations->scrollBarEngine().isHovered( widget, QStyle::SC_ScrollBarGroove )  
: option->styleObject->property("hover").toBool());
  > >  17:55:15 
^
  > >  17:55:15 kstyle/CMakeFiles/breeze.dir/build.make:910: recipe for target 
'kstyle/CMakeFiles/breeze.dir/breezestyle.cpp.o' failed
  >
  >
  > Thanks for reporting.
  >  Looking at the log in more detail, this is in fact the qt4/kde4 built of 
breeze that fails.
  >  And indeed styleObject is not defined for qt4.
  
  
  Ah, I forgot that our builds were still making the Qt4 style. Also explains 
why the main KDE CI build did not fail!
  
  Thanks

REPOSITORY
  R31 Breeze

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: rikmills, davidedmundson, ngraham, colomar, abetts, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart, hein

D9792: even slimmer scrollbars

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  Fixed with 
https://commits.kde.org/breeze/fc830e4710a9994a13911ac6cbc60ffda8f27715

REPOSITORY
  R31 Breeze

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: rikmills, davidedmundson, ngraham, colomar, abetts, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart, hein

D9792: even slimmer scrollbars

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  In https://phabricator.kde.org/D9792#193473, @rikmills wrote:
  
  > In Neon and Kubuntu CI with Qt 5.9.3
  >
  > 
https://build.neon.kde.org/job/xenial_unstable_plasma_breeze_bin_amd64/186/console
  >
  > 17:55:15 /workspace/build/kstyle/breezestyle.cpp: In member function 
‘virtual bool Breeze::Style::drawScrollBarSliderControl(const QStyleOption*, 
QPainter*, const QWidget*) const’:
  >  17:55:15 /workspace/build/kstyle/breezestyle.cpp:5013:136: error: ‘const 
class QStyleOption’ has no member named ‘styleObject’
  >  17:55:15  const bool widgetMouseOver( widget ? 
_animations->scrollBarEngine().isHovered( widget, QStyle::SC_ScrollBarGroove )  
: option->styleObject->property("hover").toBool());
  >  17:55:15   
  ^
  >  17:55:15 kstyle/CMakeFiles/breeze.dir/build.make:910: recipe for target 
'kstyle/CMakeFiles/breeze.dir/breezestyle.cpp.o' failed
  
  
  Thanks for reporting.
  Looking at the log in more detail, this is in fact the qt4/kde4 built of 
breeze that fails.
  And indeed styleObject is not defined for qt4. 
  Will add the necessary #ifdef asap.
  
  Hugo

REPOSITORY
  R31 Breeze

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: rikmills, davidedmundson, ngraham, colomar, abetts, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart, hein

D9792: even slimmer scrollbars

[Rik Mills]

rikmills added a comment.


  In Neon and Kubuntu CI with Qt 5.9.3
  
  
https://build.neon.kde.org/job/xenial_unstable_plasma_breeze_bin_amd64/186/console
  
  17:55:15 /workspace/build/kstyle/breezestyle.cpp: In member function ‘virtual 
bool Breeze::Style::drawScrollBarSliderControl(const QStyleOption*, QPainter*, 
const QWidget*) const’:
  17:55:15 /workspace/build/kstyle/breezestyle.cpp:5013:136: error: ‘const 
class QStyleOption’ has no member named ‘styleObject’
  17:55:15  const bool widgetMouseOver( widget ? 
_animations->scrollBarEngine().isHovered( widget, QStyle::SC_ScrollBarGroove )  
: option->styleObject->property("hover").toBool());
  17:55:15  
   ^
  17:55:15 kstyle/CMakeFiles/breeze.dir/build.make:910: recipe for target 
'kstyle/CMakeFiles/breeze.dir/breezestyle.cpp.o' failed

REPOSITORY
  R31 Breeze

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: rikmills, davidedmundson, ngraham, colomar, abetts, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart added a comment.


  landed just in master post 5.12

REPOSITORY
  R31 Breeze

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: davidedmundson, ngraham, colomar, abetts, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

This revision was automatically updated to reflect the committed changes.
Closed by commit R31:d4b07d9e1daf: even slimmer scrollbars (authored by mart).

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9792?vs=25385&id=25590

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

AFFECTED FILES
  kstyle/breezestyle.cpp

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: davidedmundson, ngraham, colomar, abetts, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart, hein

D9792: even slimmer scrollbars

[David Edmundson]

davidedmundson added a comment.


  Please merge after 5.12 branches.

REPOSITORY
  R31 Breeze

BRANCH
  arcpatch-D9792

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: davidedmundson, ngraham, colomar, abetts, plasma-devel, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart, hein

D9792: even slimmer scrollbars

[Hugo Pereira Da Costa]

hpereiradacosta accepted this revision.
hpereiradacosta added a comment.


  Ship it ! Thanks !

REPOSITORY
  R31 Breeze

BRANCH
  arcpatch-D9792

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: ngraham, colomar, abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> mart wrote in breezestyle.cpp:5016
> it works ok for the qwidget case, unfortunately in the case of qml, 
> _animations will always be empty, so i still have to check 
> option->styleObject anyways

this is the shortest form i could come up which works with both qwidgets and qml

REPOSITORY
  R31 Breeze

BRANCH
  arcpatch-D9792

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: ngraham, colomar, abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart updated this revision to Diff 25385.
mart added a comment.


  - shorter form

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9792?vs=25240&id=25385

BRANCH
  arcpatch-D9792

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

AFFECTED FILES
  kstyle/breezestyle.cpp

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: ngraham, colomar, abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> hpereiradacosta wrote in breezestyle.cpp:5016
> Ok. Just checked: this whole code should still work if replaced by:
> 
>   const bool widgetMouseOver( _animations->scrollBarEngine().isHovered( 
> widget, QStyle::SC_ScrollBarGroove ) );
> 
> (used elsewhere also)
> it relies on the fact that hover state is properly updated, which seems to be 
> the case, at least with QWidget.
> Can you double check with QtQuick ? If yes, that would be a nice replacement.

it works ok for the qwidget case, unfortunately in the case of qml, _animations 
will always be empty, so i still have to check option->styleObject anyways

REPOSITORY
  R31 Breeze

BRANCH
  arcpatch-D9792

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: ngraham, colomar, abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Hugo Pereira Da Costa]

hpereiradacosta added inline comments.

INLINE COMMENTS

> breezestyle.cpp:5016
> +// in case this QStyle is used by QQuickControls QStyle wrapper
> +else if( option->styleObject ) widgetMouseOver = 
> option->styleObject->property("hover").toBool();
> +

Ok. Just checked: this whole code should still work if replaced by:

  const bool widgetMouseOver( _animations->scrollBarEngine().isHovered( widget, 
QStyle::SC_ScrollBarGroove ) );

(used elsewhere also)
it relies on the fact that hover state is properly updated, which seems to be 
the case, at least with QWidget.
Can you double check with QtQuick ? If yes, that would be a nice replacement.

REPOSITORY
  R31 Breeze

BRANCH
  arcpatch-D9792

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: ngraham, colomar, abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Thomas Pfeiffer]

colomar added a comment.


  Dunno why I only noticed it now, but of course then this is not the right 
place to fix it. Sorry for the noise.

REPOSITORY
  R31 Breeze

BRANCH
  arcpatch-D9792

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: ngraham, colomar, abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  In https://phabricator.kde.org/D9792#190540, @ngraham wrote:
  
  > In https://phabricator.kde.org/D9792#190538, @colomar wrote:
  >
  > > Looks great!
  > >  My only concern would be that in the mouseover state the contrast 
between the grip and groove is quite low. This could make the two difficult to 
distinguish, especially for color blind users. Could perhaps the contrast in 
terms of brightness be increased a bit?
  >
  >
  > +1; maybe we could make the groove lighter?
  
  
  But that should be a different patch. Mouse-over and scrollbar background 
have not been changed by this patch, and have been like that since day one. 
  No objection against changing them but
  
  - in a different patch
  - consistently with the rest of breeze. (the same shade of grey is used 
elsewhere, such as sliders, and progressbars, possibly also inactive tabs in 
tabs background.
  
  As for the mouse-over color, for the handle itself, I think it comes from the 
palette unmodified.
  
  Best,
  
  Hugo

REPOSITORY
  R31 Breeze

BRANCH
  arcpatch-D9792

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: ngraham, colomar, abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Nathaniel Graham]

ngraham added a comment.


  In https://phabricator.kde.org/D9792#190538, @colomar wrote:
  
  > Looks great!
  >  My only concern would be that in the mouseover state the contrast between 
the grip and groove is quite low. This could make the two difficult to 
distinguish, especially for color blind users. Could perhaps the contrast in 
terms of brightness be increased a bit?
  
  
  +1; maybe we could make the groove lighter?

REPOSITORY
  R31 Breeze

BRANCH
  arcpatch-D9792

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: ngraham, colomar, abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Thomas Pfeiffer]

colomar added a comment.


  Looks great!
  My only concern would be that in the mouseover state the contrast between the 
grip and groove is quite low. This could make the two difficult to distinguish, 
especially for color blind users. Could perhaps the contrast in terms of 
brightness be increased a bit?

REPOSITORY
  R31 Breeze

BRANCH
  arcpatch-D9792

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: colomar, abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart marked an inline comment as done.

REPOSITORY
  R31 Breeze

BRANCH
  arcpatch-D9792

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart updated this revision to Diff 25240.
mart added a comment.


  fix the math

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9792?vs=25152&id=25240

BRANCH
  arcpatch-D9792

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

AFFECTED FILES
  kstyle/breezestyle.cpp

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Hugo Pereira Da Costa]

hpereiradacosta added inline comments.

INLINE COMMENTS

> breezestyle.cpp:5046
> +if (StyleConfigData::animationsEnabled()) {
> +color.setAlphaF(color.alphaF() * (0.7 + grooveAnimationOpacity));
> +}

This generates some warnings here, because 0.7 + grooveAnimationOpacity can 
become larger than unity.

REPOSITORY
  R31 Breeze

BRANCH
  phab/slimScrollbars

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart added a comment.


  so, a slightly different approach now, maybe it's a tad cleaner: don't try to 
draw the handle twice and cross fade it, but draw it smaller and then animate 
the handle size (and opacity) on mouse over

REPOSITORY
  R31 Breeze

BRANCH
  phab/slimScrollbars

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart updated this revision to Diff 25152.
mart added a comment.


  - const opacity

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9792?vs=25151&id=25152

BRANCH
  phab/slimScrollbars

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

AFFECTED FILES
  kstyle/breezestyle.cpp

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart updated this revision to Diff 25151.
mart added a comment.


  - use the same slider, animate its size

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9792?vs=25089&id=25151

BRANCH
  phab/slimScrollbars

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

AFFECTED FILES
  kstyle/breezestyle.cpp

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Hugo Pereira Da Costa]

hpereiradacosta added inline comments.

INLINE COMMENTS

> mart wrote in breezestyle.cpp:5041
> i like the second option, calling by hand drawScrollBarSliderControl, tough 
> it would still need to call the superclass drawScrollBarComplexControl? which 
> then would have to know about not drawing the handle?

Yeah this needs some thinking.

Maybe not call the base class drawComplexControl and implement all the 
subcontrol calls ourselves ? Disable the call for slider subcontrol  
(CE_ScrollBarSlider), as we already do for CE_ScrollBarAddPage and SubPage ?

The second has the disadvantage that applications that call this control 
element directly (for say a custom scrollbar), will get broken.

REPOSITORY
  R31 Breeze

BRANCH
  phab/slimScrollbars

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> mart wrote in breezestyle.cpp:5041
> you mean making drawScrollBarSliderControl a complete noop?

i like the second option, calling by hand drawScrollBarSliderControl, tough it 
would still need to call the superclass drawScrollBarComplexControl? which then 
would have to know about not drawing the handle?

REPOSITORY
  R31 Breeze

BRANCH
  phab/slimScrollbars

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> hpereiradacosta wrote in breezestyle.cpp:5041
> This whole code feels hackish.
> In principle we know (elsewhere in the code), if any part of the scrollbar is 
> hovered, since it is used to make the groove appear. 
> I checked that this is done in a separate method though, namely 
> Breeze::Style::drawScrollBarComplexControl, which calls the base class method 
> in the end, which in turn ends up calling this method. 
> I wonder if we should not simply move the code from 
> drawScrollbarSliderControl to the ComplexControl method, thus making the 
> hacking above unnecessary. (makes sense ?)

you mean making drawScrollBarSliderControl a complete noop?

REPOSITORY
  R31 Breeze

BRANCH
  phab/slimScrollbars

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Andres Betts]

abetts accepted this revision.
abetts added a comment.
This revision is now accepted and ready to land.


  I like it. I think it falls in line with a less obtrusive bar and one that 
demands attention.

REPOSITORY
  R31 Breeze

BRANCH
  phab/slimScrollbars

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta, abetts
Cc: abetts, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  Also, checking the patch online, it also indirecly changes color handling. In 
the previous implementation, focus color was used to render the 
'non-mouse-over' scrollbar for the widget with focus. This is not the case 
anymore with this patch, because "disabled" is used unconditionally, when not 
mouse over. I am not sure this is a good thing. Maybe focus color should be 
preserved on the thin scrollbar.

REPOSITORY
  R31 Breeze

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Hugo Pereira Da Costa]

hpereiradacosta added inline comments.

INLINE COMMENTS

> hpereiradacosta wrote in breezestyle.cpp:5041
> This whole code feels hackish.
> In principle we know (elsewhere in the code), if any part of the scrollbar is 
> hovered, since it is used to make the groove appear. 
> I checked that this is done in a separate method though, namely 
> Breeze::Style::drawScrollBarComplexControl, which calls the base class method 
> in the end, which in turn ends up calling this method. 
> I wonder if we should not simply move the code from 
> drawScrollbarSliderControl to the ComplexControl method, thus making the 
> hacking above unnecessary. (makes sense ?)

One possibility, maybe, would be to call drawScrollbarSliderControl explicitely 
in drawScrollBarComplexControl rather than letting the parent class method do 
that, and just pass it a modified (smaller) rectangle when not mouseovered.

REPOSITORY
  R31 Breeze

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  Hi,
  
  That looks nice. I don't think it needs a separate settings, since once again 
the hit area is the same as before. 
  Then: would be nice to have VDG oppinion on that, and see if we can make the 
code less hackish (on the mouse-over detection side).
  
  Hugo

INLINE COMMENTS

> breezestyle.cpp:5041
> +if( grooveOpacity == AnimationData::OpacityInvalid ) grooveOpacity = 
> (widgetMouseOver ? 1 : 0);
> +
>  const auto mode( _animations->scrollBarEngine().animationMode( 
> widget, SC_ScrollBarSlider ) );

This whole code feels hackish.
In principle we know (elsewhere in the code), if any part of the scrollbar is 
hovered, since it is used to make the groove appear. 
I checked that this is done in a separate method though, namely 
Breeze::Style::drawScrollBarComplexControl, which calls the base class method 
in the end, which in turn ends up calling this method. 
I wonder if we should not simply move the code from drawScrollbarSliderControl 
to the ComplexControl method, thus making the hacking above unnecessary. (makes 
sense ?)

REPOSITORY
  R31 Breeze

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart added a comment.


  left sidebar: QML, main area: QWidgets
  F5633964: Spectacle.nS6609.png 
  
  on mouse over
  F5633969: Spectacle.lx6609.png 

REPOSITORY
  R31 Breeze

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart edited the summary of this revision.

REPOSITORY
  R31 Breeze

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart added a reviewer: hpereiradacosta.

REPOSITORY
  R31 Breeze

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

To: mart, #plasma, #kirigami, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, davidedmundson, mart, hein

D9792: even slimmer scrollbars

[Marco Martin]

mart created this revision.
mart added reviewers: Plasma, Kirigami, VDG.
Restricted Application added projects: Plasma, Kirigami.
Restricted Application added a subscriber: plasma-devel.
mart requested review of this revision.

REVISION SUMMARY
  the scrollbar handle will be very slim normally, and grow a bit on mouse
  over, this makes it work better with QtQuickControls2 listviews where
  the contents overlap the scrollbar (as other platform are going in
  this direction since they all have scrollbars visible only on scroll
  or mouse over)
  this makes scrollbars appear more light but still always visible to
  convey information

TEST PLAN
  works both as qwidget and with the qstyle-based qqc2-desktop-style

REPOSITORY
  R31 Breeze

BRANCH
  phab/slimScrollbars

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

AFFECTED FILES
  kstyle/breezestyle.cpp

To: mart, #plasma, #kirigami, #vdg
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, davidedmundson, mart, hein