~eliasnaur/gio-patches

gio: widget: constrain drag offset to [0, 1] v1 PROPOSED

Dominik Honnef: 6
 widget: constrain drag offset to [0, 1]
 widget: correctly set s.dragging to false when releasing drag
 widget: clicking on the scrollbar indicator shouldn't jump
 widget: consider size of indicator when limiting scrollbar dragging
 widget: move scrollbar indicator if dragging starts outside of it
 widget: when clicking on scrollbar, center on that point

 6 files changed, 69 insertions(+), 16 deletions(-)
#790900 apple.yml success
#790901 freebsd.yml success
#790902 linux.yml success
#790903 openbsd.yml success
Thanks, merged with small tweak.


On Thu Jun 30, 2022 at 01:21, Dominik Honnef wrote:
Next
Thanks, merged.

Elias
Thanks, merged.

Elias
Thanks, merged.

Elias
gio/patches: SUCCESS in 19m26s

[widget: constrain drag offset to [0, 1]][0] from [Dominik Honnef][1]

[0]: https://lists.sr.ht/~eliasnaur/gio-patches/patches/33396
[1]: mailto:dominik@honnef.co

✓ #790902 SUCCESS gio/patches/linux.yml   https://builds.sr.ht/~eliasnaur/job/790902
✓ #790900 SUCCESS gio/patches/apple.yml   https://builds.sr.ht/~eliasnaur/job/790900
✓ #790903 SUCCESS gio/patches/openbsd.yml https://builds.sr.ht/~eliasnaur/job/790903
✓ #790901 SUCCESS gio/patches/freebsd.yml https://builds.sr.ht/~eliasnaur/job/790901
Thanks, merged.

Elias
Thank you for fixing these Dominik! I finally had a chance to look
through them in depth.

Cheers,
Chris
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~eliasnaur/gio-patches/patches/33396/mbox | git am -3
Learn more about email & git

[PATCH gio 1/6] widget: constrain drag offset to [0, 1] Export this patch

Once the user begins dragging, the cursor can move outside the clip
area (or even the window on at least X11), leading to events with
positions that are either negative, or larger than the clip area.

Negative values outright break the delta tracking and cause the
scrollbar to misbehave. Positive values "only" break the invariant of
Scrollbar.ScrollDistance that the returned value is in the range [-1, 1].

Signed-off-by: Dominik Honnef <dominik@honnef.co>
---
 widget/list.go | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/widget/list.go b/widget/list.go
index 962eea7e..224b8321 100644
--- a/widget/list.go
+++ b/widget/list.go
@@ -74,6 +74,12 @@ func (s *Scrollbar) Layout(gtx layout.Context, axis layout.Axis, viewportStart,
			continue
		}
		dragOffset := axis.FConvert(event.Position).X
		// The user can drag outside of the constraints, or even the window. Limit dragging to within the scrollbar.
		if dragOffset < 0 {
			dragOffset = 0
		} else if dragOffset > trackHeight {
			dragOffset = trackHeight
		}
		normalizedDragOffset := dragOffset / trackHeight

		if !s.dragging {
-- 
2.36.1
Thanks, merged.

Elias

[PATCH gio 2/6] widget: correctly set s.dragging to false when releasing drag Export this patch

Before, we would set s.dragging to false on pointer.Release and then
immediately set it back to true because we were processing the event and
saw that s.dragging was false.

Signed-off-by: Dominik Honnef <dominik@honnef.co>
---
 widget/list.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/widget/list.go b/widget/list.go
index 224b8321..ccbde107 100644
--- a/widget/list.go
+++ b/widget/list.go
@@ -67,6 +67,7 @@ func (s *Scrollbar) Layout(gtx layout.Context, axis layout.Axis, viewportStart,
		case pointer.Drag:
		case pointer.Release:
			s.dragging = false
			continue
		case pointer.Cancel:
			s.dragging = false
			continue
-- 
2.36.1
Thanks, merged with small tweak.


On Thu Jun 30, 2022 at 01:21, Dominik Honnef wrote:

[PATCH gio 3/6] widget: clicking on the scrollbar indicator shouldn't jump Export this patch

Signed-off-by: Dominik Honnef <dominik@honnef.co>
---
 widget/list.go | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/widget/list.go b/widget/list.go
index ccbde107..42425db2 100644
--- a/widget/list.go
+++ b/widget/list.go
@@ -58,7 +58,11 @@ func (s *Scrollbar) Layout(gtx layout.Context, axis layout.Axis, viewportStart,
			Y: int(event.Position.Y),
		})
		normalizedPos := float32(pos.X) / trackHeight
		s.delta += normalizedPos - viewportStart
		// Clicking on the indicator should not jump to that position on the track. The user might've just intended to
		// drag and changed their mind.
		if !(normalizedPos >= viewportStart && normalizedPos <= viewportEnd) {
			s.delta += normalizedPos - viewportStart
		}
	}

	// Offset to account for any drags.
-- 
2.36.1
Thanks, merged.

Elias

[PATCH gio 4/6] widget: consider size of indicator when limiting scrollbar dragging Export this patch

Signed-off-by: Dominik Honnef <dominik@honnef.co>
---
 widget/list.go | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/widget/list.go b/widget/list.go
index 42425db2..43852196 100644
--- a/widget/list.go
+++ b/widget/list.go
@@ -92,6 +92,18 @@ func (s *Scrollbar) Layout(gtx layout.Context, axis layout.Axis, viewportStart,
			s.oldDragPos = normalizedDragOffset
		}
		s.delta += normalizedDragOffset - s.oldDragPos

		if viewportStart+s.delta < 0 {
			// Adjust normalizedDragOffset - and thus the future s.oldDragPos - so that futile dragging up has to be
			// countered with dragging down again. Otherwise, dragging up would have no effect, but dragging down would
			// immediately start scrolling. We want the user to undo their ineffective drag first.
			normalizedDragOffset -= viewportStart + s.delta
			// Limit s.delta to the maximum amount scrollable
			s.delta = -viewportStart
		} else if viewportEnd+s.delta > 1 {
			normalizedDragOffset += (1 - viewportEnd) - s.delta
			s.delta = 1 - viewportEnd
		}
		s.oldDragPos = normalizedDragOffset
	}

-- 
2.36.1
Thanks, merged.

Elias

[PATCH gio 5/6] widget: move scrollbar indicator if dragging starts outside of it Export this patch

Signed-off-by: Dominik Honnef <dominik@honnef.co>
---
 widget/list.go | 42 +++++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/widget/list.go b/widget/list.go
index 43852196..56dc25b0 100644
--- a/widget/list.go
+++ b/widget/list.go
@@ -90,21 +90,37 @@ func (s *Scrollbar) Layout(gtx layout.Context, axis layout.Axis, viewportStart,
		if !s.dragging {
			s.dragging = true
			s.oldDragPos = normalizedDragOffset
		}
		s.delta += normalizedDragOffset - s.oldDragPos

		if viewportStart+s.delta < 0 {
			// Adjust normalizedDragOffset - and thus the future s.oldDragPos - so that futile dragging up has to be
			// countered with dragging down again. Otherwise, dragging up would have no effect, but dragging down would
			// immediately start scrolling. We want the user to undo their ineffective drag first.
			normalizedDragOffset -= viewportStart + s.delta
			// Limit s.delta to the maximum amount scrollable
			s.delta = -viewportStart
		} else if viewportEnd+s.delta > 1 {
			normalizedDragOffset += (1 - viewportEnd) - s.delta
			s.delta = 1 - viewportEnd
			if normalizedDragOffset < viewportStart || normalizedDragOffset > viewportEnd {
				// The user started dragging somewhere on the track that isn't covered by the indicator. Consider this a
				// click in addition to a drag and jump to the clicked point.
				//
				// TODO(dh): this isn't perfect. We only get the pointer.Drag event once the user has actually dragged,
				// which means that if the user presses the mouse button and neither releases it nor drags it, nothing
				// will happen.
				pos := axis.Convert(image.Point{
					X: int(event.Position.X),
					Y: int(event.Position.Y),
				})
				normalizedPos := float32(pos.X) / trackHeight
				s.delta += normalizedPos - viewportStart
			}
		} else {
			s.delta += normalizedDragOffset - s.oldDragPos

			if viewportStart+s.delta < 0 {
				// Adjust normalizedDragOffset - and thus the future s.oldDragPos - so that futile dragging up has to be
				// countered with dragging down again. Otherwise, dragging up would have no effect, but dragging down would
				// immediately start scrolling. We want the user to undo their ineffective drag first.
				normalizedDragOffset -= viewportStart + s.delta
				// Limit s.delta to the maximum amount scrollable
				s.delta = -viewportStart
			} else if viewportEnd+s.delta > 1 {
				normalizedDragOffset += (1 - viewportEnd) - s.delta
				s.delta = 1 - viewportEnd
			}
			s.oldDragPos = normalizedDragOffset
		}
		s.oldDragPos = normalizedDragOffset
	}

	// Process events from the indicator so that hover is
-- 
2.36.1
Thanks, merged.

Elias

[PATCH gio 6/6] widget: when clicking on scrollbar, center on that point Export this patch

Previously, we'd scroll so the new viewportStart corresponded to the
clicked position. This felt okay if clicking above the current
indicator, but felt jarring when clicking below it. Centering gives a
consistent behavior regardless of the scroll direction.

Signed-off-by: Dominik Honnef <dominik@honnef.co>
---
 widget/list.go | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/widget/list.go b/widget/list.go
index 56dc25b0..ae395736 100644
--- a/widget/list.go
+++ b/widget/list.go
@@ -46,6 +46,20 @@ func (s *Scrollbar) Layout(gtx layout.Context, axis layout.Axis, viewportStart,
	trackHeight := float32(axis.Convert(gtx.Constraints.Max).X)
	s.delta = 0

	centerOnClick := func(normalizedPos float32) {
		// When the user clicks on the scrollbar we center on that point, respecting the limits of the beginning and end
		// of the scrollbar.
		//
		// Centering gives a consistent experience whether the user clicks above or below the indicator.
		target := normalizedPos - (viewportEnd-viewportStart)/2
		s.delta += target - viewportStart
		if s.delta < -viewportStart {
			s.delta = -viewportStart
		} else if s.delta > 1-viewportEnd {
			s.delta = 1 - viewportEnd
		}
	}

	// Jump to a click in the track.
	for _, event := range s.track.Events(gtx) {
		if event.Type != gesture.TypeClick ||
@@ -61,7 +75,7 @@ func (s *Scrollbar) Layout(gtx layout.Context, axis layout.Axis, viewportStart,
		// Clicking on the indicator should not jump to that position on the track. The user might've just intended to
		// drag and changed their mind.
		if !(normalizedPos >= viewportStart && normalizedPos <= viewportEnd) {
			s.delta += normalizedPos - viewportStart
			centerOnClick(normalizedPos)
		}
	}

@@ -103,7 +117,7 @@ func (s *Scrollbar) Layout(gtx layout.Context, axis layout.Axis, viewportStart,
					Y: int(event.Position.Y),
				})
				normalizedPos := float32(pos.X) / trackHeight
				s.delta += normalizedPos - viewportStart
				centerOnClick(normalizedPos)
			}
		} else {
			s.delta += normalizedDragOffset - s.oldDragPos
-- 
2.36.1
gio/patches: SUCCESS in 19m26s

[widget: constrain drag offset to [0, 1]][0] from [Dominik Honnef][1]

[0]: https://lists.sr.ht/~eliasnaur/gio-patches/patches/33396
[1]: mailto:dominik@honnef.co

✓ #790902 SUCCESS gio/patches/linux.yml   https://builds.sr.ht/~eliasnaur/job/790902
✓ #790900 SUCCESS gio/patches/apple.yml   https://builds.sr.ht/~eliasnaur/job/790900
✓ #790903 SUCCESS gio/patches/openbsd.yml https://builds.sr.ht/~eliasnaur/job/790903
✓ #790901 SUCCESS gio/patches/freebsd.yml https://builds.sr.ht/~eliasnaur/job/790901
Thanks, merged.

Elias