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

library: Add Scrolled Window entry #434

Merged
merged 4 commits into from
Aug 11, 2023
Merged

Conversation

AkshayWarrier
Copy link
Contributor

@AkshayWarrier AkshayWarrier commented Jul 19, 2023

Closes #419
There is an edge case where if the Start/End buttons are pressed when it's already at the edge, the buttons get disabled for the animation duration. Is that problematic and if so, what might be the simplest way to handle it?

@sonnyp sonnyp self-requested a review August 6, 2023 22:22
Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great demo 👍

Sorry for taking so long to review.

Is that problematic and if so, what might be the simplest way to handle it?

When the button is pressed, you can simply check if the scrolled area is already at the edge and do nothing.

I think an even better solution is to disable the corresponding start or end button when it's already at the edge.

start edge is adj value = adj lower
end endge is adj value = adj upper

Also please reduce animation duration to 1 sec, it's a bit too slow now.

@AkshayWarrier
Copy link
Contributor Author

start edge is adj value = adj lower
end endge is adj value = adj upper

I can't really do this because weirdly enough this is not true, I tried logging adj.upper and adj.value when the scrollbar is at the end edge and I got upper = 3360 and value = 2782. I'm not sure what adj.upper is then, but the scrollbar probably just stops scrolling after 2782. And the scrollbar doesn't have property to let me know whats the actual maximum distance it can scroll.

@AkshayWarrier
Copy link
Contributor Author

Though in my opinion, I think it's fine the way it is because I'm already reducing the animation speed to 1s so the buttons are not disabled that long, and most people looking for a Scrolled Window demo probably are only looking for the UI/Blueprint and the demo shows how to programmatically scroll if necessary. I'm concerned adding extra logic to decide when the buttons are enabled/disabled is very specific to the demo?

@sonnyp
Copy link
Contributor

sonnyp commented Aug 7, 2023

I can't really do this because weirdly enough this is not true, I tried logging adj.upper and adj.value when the scrollbar is at the end edge and I got upper = 3360 and value = 2782. I'm not sure what adj.upper is then, but the scrollbar probably just stops scrolling after 2782. And the scrollbar doesn't have property to let me know whats the actual maximum distance it can scroll.

Right, you need to do some simple maths to get positions relative to each others.

Something like
upper - page-size = value
2240 - 598 = 1642

The GtkAdjustment:value field sets the position of the thumb and must be between GtkAdjustment:lower and GtkAdjustment:upper - GtkAdjustment:page-size. The GtkAdjustment:page-size represents the size of the visible scrollable area.

See https://docs.gtk.org/gtk4/class.Scrollbar.html

. most people looking for a Scrolled Window demo probably are only looking for the UI/Blueprint

Maybe but the complexity here is in Code.

I'm concerned adding extra logic to decide when the buttons are enabled/disabled is very specific to the demo?

It's good to keep demos simple but knowing if the scroll is at the edge or not is something very common. Specially when you have a "scroll to top" button like on timelines that you want to hide or disable when you're already at the top.

I think it's worth our time.

@sonnyp sonnyp self-assigned this Aug 7, 2023
Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost, the start / end buttons should only be disabled if at the edge.

For that you should listen to the property value on the adjustment. Not sure what's best between the signals value-changed and notify::value. Would be curious to know the difference if you can figure it out.

src/Library/demos/Scrolled Window/main.js Outdated Show resolved Hide resolved
Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍 !

Did you figure out the difference between notify::value and value-changed ?

@sonnyp sonnyp merged commit 0d1a0bb into main Aug 11, 2023
@sonnyp sonnyp deleted the akshaywarrier/scrolledwindow branch August 11, 2023 13:03
@AkshayWarrier
Copy link
Contributor Author

Ah not really, I just ended up using value-changed since Gtk.Adjustment already provides that signal.

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

Successfully merging this pull request may close these issues.

Gtk Scrolled window
2 participants