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?

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.

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?

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'

Ugh, yes I do mean that! :)

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

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.

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

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.

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.

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? Should I post it here?

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.

You can also turn on the option to show trailing whitespace in Kate:

In D21639#475884, @filipf wrote:

You can also turn on the option to show trailing whitespace in Kate:

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.

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:

It doesn't print the changes in the diff, but every line has changed line endings. Gotta undo that before this can land.

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.

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.:

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!

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

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.

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

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.

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.

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

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.

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.

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?

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 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.

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!

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

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. :)