D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham added a comment.


  Yeah, only certain people have commit rights. If you stick around for a while 
and keep submitting good patches, you can become one of those people too. :)

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  Can you tell me why it didn't work for me? Was it some kind of permission 
issue or was I doing something wrong?

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R98:71a9a1f203df: Remove background from scrollbars when 
hovering on them (authored by mthw, committed by ngraham).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D21639?vs=59614&id=59616#toc

REPOSITORY
  R98 Breeze for Gtk

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21639?vs=59614&id=59616

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

AFFECTED FILES
  src/gtk318/widgets/_scrollbar.scss
  src/gtk320/widgets/_scrollbar.scss

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham added a comment.


  In D21639#478268 , @mthw wrote:
  
  > I tried to land this patch but I got this error:
  >
  >   fatal: remote error: service not enabled: /breeze-gtk
  >   Usage Exception: Push failed! Fix the error and run "arc land" again.
  >  
  >
  >
  > Could you help me with this?
  
  
  I'll do it, just a sec...
  
  >> Very nice first patch. May it be the first of many! If you'd like an idea 
for a follow-up patch, here's an idea: make the handles of the non-disappearing 
scrollbars gray instead blue when unhovered, and make the track only appear on 
hover, matching the ones in Firefox. You can see what I'm talking about in GIMP 
or Inkscape.
  > 
  > I will very likely look into this during summer holidays, when I will have 
more time.
  
  Excellent!

REPOSITORY
  R98 Breeze for Gtk

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  I tried to land this patch but I got this error:
  
fatal: remote error: service not enabled: /breeze-gtk
Usage Exception: Push failed! Fix the error and run "arc land" again.
  
  Could you help me with this?
  
  > Very nice first patch. May it be the first of many! If you'd like an idea 
for a follow-up patch, here's an idea: make the handles of the non-disappearing 
scrollbars gray instead blue when unhovered, and make the track only appear on 
hover, matching the ones in Firefox. You can see what I'm talking about in GIMP 
or Inkscape.
  
  I will very likely look into this during summer holidays, when I will have 
more time.

REPOSITORY
  R98 Breeze for Gtk

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham added a comment.


  Very nice first patch. May it be the first of many! If you'd like an idea for 
a follow-up patch, here's an idea: make the handles of the non-disappearing 
scrollbars gray instead blue when unhovered, and make the track only appear on 
hover, matching the ones in Firefox. You can see what I'm talking about in GIMP 
or Inkscape.

REPOSITORY
  R98 Breeze for Gtk

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

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


  Thanks, looks perfect now!

REPOSITORY
  R98 Breeze for Gtk

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw updated this revision to Diff 59614.
mthw added a comment.


  Formating

REPOSITORY
  R98 Breeze for Gtk

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21639?vs=59613&id=59614

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

AFFECTED FILES
  src/gtk318/widgets/_scrollbar.scss
  src/gtk320/widgets/_scrollbar.scss

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw updated this revision to Diff 59613.
mthw added a comment.


  Sliders are now grey, and when hovered blue and bigger.

REPOSITORY
  R98 Breeze for Gtk

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21639?vs=59383&id=59613

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

AFFECTED FILES
  src/gtk318/widgets/_scrollbar.scss
  src/gtk320/widgets/_scrollbar.scss

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham added a comment.


  I think I see the problem. By making the default color turquoise, the 
scrollbar is now always turquoise. If we can't perfectly match the 
Dolphin/QWidgets style where it's gray when inactive and turquoise when active, 
hovered, or clicked, I think we should leave it gray. That's more consistent 
with other GTK scrollbars, as well as the scrollbars in QML-based kde apps.
  
  So let's go with the original change to remove the ugly background under the 
track, and also the change to make the disappearing scrollbar become fat when 
hovered, and then leave the colors alone for now (and then in a future patch, 
fix the always-visible GTK scrollbars that are currently blue to be gray 
instead). Does that sound okay?

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  > Firefox's scrollbar still does not have the correct appearance...
  
  Do you mean Firefox having blue scroollbar now, instead of grey it had 
before? Is that a real problem? I mean, couldn't it stay like this? The problem 
roots from default color now being blue instead of grey, so Firefox is doing 
things correctly. I my opinion, other programs using blue regardless of this 
change is the problem.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham added a comment.


  Thanks! However Firefox's scrollbar still does not have the correct 
appearance when not hovered; compare it to GIMP or Inkscape. This is a 
regression from the status quo. Everything else looks good to me now, but that 
regression needs to be fixed before we can land this.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  @ngraham Can you please review this? It's as good as I can make it.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw updated this revision to Diff 59383.
mthw added a comment.


  There are lines that 'git diff' says are different altough shows them exactly 
the same and I don't know what to do with that.

REPOSITORY
  R98 Breeze for Gtk

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21639?vs=59382&id=59383

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

AFFECTED FILES
  src/gtk318/widgets/_scrollbar.scss
  src/gtk320/widgets/_scrollbar.scss

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw updated this revision to Diff 59382.
mthw added a comment.


  Hopefully the final version

REPOSITORY
  R98 Breeze for Gtk

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21639?vs=59364&id=59382

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

AFFECTED FILES
  src/gtk318/widgets/_scrollbar.scss
  src/gtk320/widgets/_scrollbar.scss

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  In D21639#476035 , @ngraham wrote:
  
  > In D21639#476019 , @mthw wrote:
  >
  > > I changed the color to always blue so it looked more like Dolphin. But I 
can change it back tomorrow.
  >
  >
  > The different behaviors are actually fairly subtle. This is what's typical 
for all KDE apps:
  >
  > - The handle is thin and gray when not in use
  
  
  The handle dissapears altogether when not in use and I cannot change that.
  
  > - Turn the handle turquoise when the view that contains the scrollbar 
receives focus
  
  When the handle appears it is turquoise.
  
  > - Show the track and widen the handle when the track is hovered over
  
  Done. I don't know if I got the size correct though.
  
  > - Turn the handle turquoise when it's hovered over (if it wasn't already 
turquoise)
  
  I made it different turquoise when hovered, like it is in Dolphin.
  
  > Not sure if it's possible to do all of that in the Breeze-GTK theme, but if 
it is, we should match it as closely as possible.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham added a comment.


  In D21639#476019 , @mthw wrote:
  
  > I changed the color to always blue so it looked more like Dolphin. But I 
can change it back tomorrow.
  
  
  The different behaviors are actually fairly subtle. This is what's typical 
for all KDE apps:
  
  - The handle is thin and gray when not in use
  - Turn the handle turquoise when the view that contains the scrollbar 
receives focus
  - Show the track and widen the handle when the track is hovered over
  - Turn the handle turquoise when it's hovered over (if it wasn't already 
turquoise)
  
  Not sure if it's possible to do all of that in the Breeze-GTK theme, but if 
it is, we should match it as closely as possible.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  I changed the color to always blue so it looked more like Dolphin. But I can 
change it back tomorrow.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  Nope sorry, found an issue. :) Now the non-disappearing scrollbars in Firefox 
are always turquoise rather than normally being gray and turning turquoise when 
hovered.
  
  Also, you didn't introduce this bug, but the non-disappearing scrollbars in 
GIMP and Inkscape are still blue when unhovered, rather than gray. Could you 
fix that too?

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham added a comment.


  In D21639#475983 , @mthw wrote:
  
  > Is there an easy way to find all spots that need editing?
  
  
  In your branch, run `git show HEAD`
  
  I see that the original formatting of the file is totally messed up though. 
:/ Definitely needs a clean-up in its own commit. Let me test a bit more and if 
everything looks good, I'll land it.
  
  Thanks very much for your contribution! Please feel free to continue. As you 
can see the Breeze-GTK theme needs a lot of love.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw updated this revision to Diff 59364.
mthw added a comment.


  Few more changes

REPOSITORY
  R98 Breeze for Gtk

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21639?vs=59362&id=59364

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

AFFECTED FILES
  src/gtk318/widgets/_scrollbar.scss
  src/gtk320/widgets/_scrollbar.scss

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw updated this revision to Diff 59362.
mthw added a comment.


  I tried to make some changes but I doubt I caught all problems.

REPOSITORY
  R98 Breeze for Gtk

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21639?vs=59352&id=59362

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

AFFECTED FILES
  src/gtk318/widgets/_scrollbar.scss
  src/gtk320/widgets/_scrollbar.scss

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  Is there an easy way to find all spots that need editing?

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham added a comment.


  So sorry to hear about the disappearing scrollbar issue being something you 
can's fix in the theme. Pity. Not totally unexpected though.
  
  This is much better, thanks! There are still a few ordinary whitespace and 
formatting issues, e.g.:
  
  F6876658: Screenshot_20190607_120533.png 

  
  F6876660: Screenshot_20190607_120750.png 

  
  I know I must seems like a tight-ass about this, but it's easy enough to fix 
and it's important to keep the diff minimal so people can easy scan the changes 
later as needed. Fix those up and let's get this in!

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  About that scrollbar hiding, after some googling it looks like it's a known 
problem in GTK+. It would seem that it cannot be changed by a theme or 
otherwise.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw updated this revision to Diff 59352.
mthw added a comment.


  An attempt to fix the number of lines.

REPOSITORY
  R98 Breeze for Gtk

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21639?vs=59350&id=59352

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

AFFECTED FILES
  src/gtk318/widgets/_scrollbar.scss
  src/gtk320/widgets/_scrollbar.scss

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham added a comment.


  Very close! The track and hover behavior now look perfect. But the handle 
still disappears entirely when the mouse is outside of the scrollview. Dolphin 
doesn't do that.
  
  Also `src/gtk320/widgets/_scrollbar.scss` still has the wrong line endings. 
You can see how Phabricator sneakily says 265 lines have changed: F6876484: 
Screenshot_20190607_101446.png 
  
  It doesn't print the changes in the diff, but every line has changed line 
endings. Gotta undo that before this can land.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw updated this revision to Diff 59350.
mthw added a comment.


  Scrollbars now should work exactly like in Dolphin, please review. Hope there 
are no problems with the file like last time. Everything looks fine here.

REPOSITORY
  R98 Breeze for Gtk

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21639?vs=59336&id=59350

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

AFFECTED FILES
  src/gtk318/widgets/_scrollbar.scss
  src/gtk320/widgets/_scrollbar.scss

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  In D21639#475884 , @filipf wrote:
  
  > You can also turn on the option to show trailing whitespace in Kate:
  >
  > F6876354: image.png 
  
  
  I have set this and everything looks fine, I will try to go for full Dolphin 
scrollbar clone and probably will post at least a half baked solution later.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Filip Fila]

filipf added a comment.


  You can also turn on the option to show trailing whitespace in Kate:
  
  F6876354: image.png 

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: filipf, ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham added a comment.


  If you want to make the scrollbar behavior here mimic Dolphin, that would be 
great. That means:
  
  - A thin version of the scrollbar handle is always visible
  - When the scrollbar is hovered over, it gets fatter and its background track 
becomes visible
  - The background track looks exactly the same as it does in Dolphin (i.e. 
it's the same width as the handle)
  
  If you can do all that, it would be a nice improvement. However please do 
sort out the issues with your editor; we can't have whole files changing line 
endings since it buries the real change to the file in a sea of line ending 
changes.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  In D21639#475864 , @ngraham wrote:
  
  > Ah, this change only affects those horrible scrollbars that disappear when 
not being used and leave you unable to see at a glance what position you're at 
in the view or even that the view is scrollable in the first place. This change 
seems to work fine and improves the UI for that particular use case, so +1.
  >
  > However Phab doesn't expose that this patch has significant formatting 
issues. It looks like your editor changed all the line endings for 
`src/gtk320/widgets/_scrollbar.scss`, which is an undesired change that must be 
reverted. There's also a hidden whitespace issue with the change in 
`src/gtk318/widgets/_scrollbar.scss`. It helps to do a final `git diff` before 
submitting the patch, which can help catch issues like this.
  >
  > Please fix those issues, then we can land this.
  
  
  Have you read my comment twice above, about this look? 
https://imgur.com/a/2EeF7pp Wouldn't it be better? Shoud I post it here?

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham added a comment.


  Ah, this change only affects those horrible scrollbars that disappear when 
not being used and leave you unable to see at a glance what position you're at 
in the view or even that the view is scrollable in the first place. This change 
seems to work fine and improves the UI for that particular use case, so +1.
  
  However Phab doesn't expose that this patch has significant formatting 
issues. It looks like your editor changed all the line endings for 
`src/gtk320/widgets/_scrollbar.scss`, which is an undesired change that must be 
reverted. There's also a hidden whitespace issue with the change in 
`src/gtk318/widgets/_scrollbar.scss`. It helps to do a final `git diff` before 
submitting the patch, which can help catch issues like this.
  
  Please fix those issues, then we can land this.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  In D21639#475858 , @ngraham wrote:
  
  > I didn't notice any difference with Firefox or GIMP. What app should I use 
to test this?
  
  
  GTK3-demo or pamac-manager, Firefox doesn't seem to be affected by this. And 
for Gtk 3.18, I don't know.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham added a comment.


  I didn't notice any difference with Firefox or GIMP. What app should I use to 
test this?

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  Another question is whether we don't want to keep the scrollbar background 
after all. That would make it look exactly like in Dolphin/Qt. See: 
https://imgur.com/a/2EeF7pp . The problem is I don't seem to be able to fix the 
horizontal scrollbar, see: https://imgur.com/a/cyL22MK ,  and I could use some 
help with that. TBH I didn't know anything about CSS before doing this.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw updated this revision to Diff 59336.
mthw added a comment.


  Made changes to Gtk 3.18 hopefully they work

REPOSITORY
  R98 Breeze for Gtk

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21639?vs=59324&id=59336

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

AFFECTED FILES
  src/gtk318/widgets/_scrollbar.scss
  src/gtk320/widgets/_scrollbar.scss

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  I made that change, but I don't know if it works. I don't know a way to test 
it.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham added a comment.


  Ugh, yes I do mean that! :)

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  In D21639#475790 , @ngraham wrote:
  
  > `src/gtk/widgets/_scrollbar.scss` needs the same change as well.
  
  
  Do you mean 'src/gtk318/widgets/_scrollbar.scss'? There is no 
'src/gtk/widgets/_scrollbar.scss'

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  `src/gtk/widgets/_scrollbar.scss` needs the same change as well.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze, ndavis, ngraham
Cc: ngraham, ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D21639: Remove background from scrollbars when hovering on them

[Noah Davis]

ndavis accepted this revision as: VDG, ndavis.
ndavis added a comment.
This revision is now accepted and ready to land.


  In D21639#475696 , @mthw wrote:
  
  > In D21639#475649 , @ndavis wrote:
  >
  > > This seems inconsistent with our Qt Widget scrollbars...
  >
  >
  > Do you mean by this, that there should be a "rail" behind the scrollbar? Or 
what else seems to be the problem?
  
  
  Actually, I think I was wrong, so never mind.
  
  F6876104: Screenshot_20190607_074239.png 

  
  Now I see what this fixes. Yeah, this is definitely an improvement.

REPOSITORY
  R98 Breeze for Gtk

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

To: mthw, #vdg, #breeze, ndavis
Cc: ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw updated this revision to Diff 59324.
mthw added a comment.


  None is not a valid color warning

REPOSITORY
  R98 Breeze for Gtk

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21639?vs=59317&id=59324

BRANCH
  breeze-gtk-scrollbar-no-background (branched from master)

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

AFFECTED FILES
  src/gtk320/widgets/_scrollbar.scss

To: mthw, #vdg, #breeze
Cc: ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  In D21639#475649 , @ndavis wrote:
  
  > This seems inconsistent with our Qt Widget scrollbars...
  
  
  Do you mean by this, that there should be a "rail" behind the scrollbar? Or 
what else seems to be the problem?

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze
Cc: ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D21639: Remove background from scrollbars when hovering on them

[Matej Mrenica]

mthw added a comment.


  The original idea was to make the scrollbar look more like in Dolphin and 
other Qt apps. I tested this with gtk3-demo, pamac-manager and Firefox. 
Everything works correctly and Firefox doesn't seem to be affected at all. Also 
Breeze dark doesn't seem to be affected/changed which I will look into later.

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze
Cc: ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D21639: Remove background from scrollbars when hovering on them

[Noah Davis]

ndavis added a comment.


  This seems inconsistent with our Qt Widget scrollbars. Is there no chance 
that the scrollbar would be hard to see against some backgrounds? What GTK 
themed apps did you test with? How does it affect Firefox?

REPOSITORY
  R98 Breeze for Gtk

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

To: mthw, #vdg, #breeze
Cc: ndavis, plasma-devel, LeGast00n, ericadams, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart