Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GUI: Add padding to devices scrollable view #2266

Closed
MattHag opened this issue Feb 11, 2024 · 8 comments · Fixed by #2268
Closed

GUI: Add padding to devices scrollable view #2266

MattHag opened this issue Feb 11, 2024 · 8 comments · Fixed by #2268

Comments

@MattHag
Copy link
Collaborator

MattHag commented Feb 11, 2024

Information

  • Solaar version: solaar 1.1.11rc1-3-ge1ac8b5
  • Distribution: Ubuntu 20.04

Is your feature request related to a problem? Please describe.
The latest GUI misses some whitespace on the scrollable view visible on the screenshot below. The

  1. The text 'Backlight' has no whitespace to the left side of the surrounding rectangle.
  2. It misses some top and bottom padding. On the screenshot shows the lack of padding on top. This leads to the impression, that there should be more content above, but there isn't. The same issue applies to the bottom padding, where input selections directly hit the surrounding rectangle too.

Screenshot from 2024-02-11

Describe the solution you'd like

  • 'Backlight' etc should get a little spacing padding between the border.
  • The scrollable view should get top and bottom padding. Thus, the viewer can see that the top or bottom end is reached.
@MattHag MattHag changed the title Improve margin/padding on scrollable view Gui: Add margin/padding on devices scrollable view Feb 11, 2024
@MattHag MattHag changed the title Gui: Add margin/padding on devices scrollable view Gui: Add paddings to devices scrollable view Feb 11, 2024
@MattHag MattHag changed the title Gui: Add paddings to devices scrollable view GUI: Add paddings to devices scrollable view Feb 11, 2024
@pfps
Copy link
Collaborator

pfps commented Feb 11, 2024

A solution is to just remove the "shadow" around the scrolling region and add a horizontal line just above it. PR #2268 does this.

@pfps pfps changed the title GUI: Add paddings to devices scrollable view GUI: Add padding to devices scrollable view Feb 11, 2024
@MattHag
Copy link
Collaborator Author

MattHag commented Feb 11, 2024

Yeah, that's absolutely possible as long as the scrollability is easy to detect.

@pfps
Copy link
Collaborator

pfps commented Feb 11, 2024

It should be the same as before. Here is a screenshot from the current version and then one from the PR
Screenshot from 2024-02-11 09-40-25
Screenshot from 2024-02-11 09-42-08

@pfps
Copy link
Collaborator

pfps commented Feb 11, 2024

As you say, the remaining issue is how to easily determine whether the configuration panel is scrollable. Just having the shadow doesn't do that well, in my opinion. If the scroll bar was visible when the window first comes up if there is non-visible bits that would do the trick.

@pfps
Copy link
Collaborator

pfps commented Feb 11, 2024

The problem is that there does not seem to be a way in GTK to have the scrollbar "line" always be visible. It fades away under various circumstances but comes back when useful.

But the "shadow" around the scrolling region isn't indicative that there is a scrolling region. So I think removing the "shadow" is a good idea as it helps with the left and top margins.

@pfps
Copy link
Collaborator

pfps commented Feb 11, 2024

When you do hit the top or bottom there is a stop indicator that shows up briefly.

@MattHag
Copy link
Collaborator Author

MattHag commented Feb 11, 2024

The second screenshots shows an improvement to the status quo. However, giving a hint that a view is scrollable seems not the strength of GTK. Let's merge this.

@MattHag
Copy link
Collaborator Author

MattHag commented Feb 12, 2024

Today I discovered, that Ubuntu seems to add a slight inner shadow to the edge, which hides scrollable content. So the solution should be perfect on Ubuntu 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants