~eliasnaur/gio-patches

Re: [PATCH gio 1/2] widget,widget/material: add Float and Slider

Details
Message ID
<C3NJZDUM7OY2.3PVKS57GM93YS@themachine>
DKIM signature
pass
Download raw message
On Sun Jun 21, 2020 at 13:50, Gordon Klaus wrote:
> On Sun, Jun 21, 2020 at 10:47 AM Elias Naur <mail@eliasnaur.com> wrote:
> >
> > On Sat Jun 20, 2020 at 20:11, Gordon Klaus wrote:
> > > On Sat, Jun 20, 2020 at 7:04 PM Elias Naur <mail@eliasnaur.com> wrote:
> > > >
> > > > On Sat Jun 20, 2020 at 13:33, Gordon Klaus wrote:
> > > > > Thanks for the feedback!  I managed to clean things up quite a bit.
> > > > >
> > > > >
> > > > > On Fri, Jun 19, 2020 at 12:11 PM Elias Naur <mail@eliasnaur.com> wrote:
> > > > >
> > > > > >
> > > > > > > +type Drag struct {
> > > > > > > +     dragging bool
> > > > > > > +     pid      pointer.ID
> > > > > > > +     start    f32.Point
> > > > > > > +     grab     bool
> > > > > > > +}
> > > > > > > +
> > > > > > >  // Scroll detects scroll gestures and reduces them to
> > > > > > >  // scroll distances. Scroll recognizes mouse wheel
> > > > > > >  // movements as well as drag and fling touch gestures.
> > > > > > > +func (d *Drag) Events(cfg unit.Metric, q event.Queue, axis Axis) []pointer.Event {
> > > > > >
> > > > > > Why not return an int that collects the axis-aligned dragging, like Scroll?
> > > > >
> > > > > Two reasons:
> > > > > 1.  widget.Float is interested in position, not delta.
> > > > > 2.  I assume other clients might be interested in knowing when a drag
> > > > > is released.
> > > > >
> > > > > This can also be achieved by adding Drag.Pos and Drag.Released
> > > > > methods.  I'm not sure how useful it will be to return a delta from
> > > > > Drag.Events, since I assume most clients will be interested in
> > > > > position.  But Drag.Pos should simplify widget.Float.  I'll give it a
> > > > > try.
> > > > >
> > > > > ... After having given it a try, I found that I would also need to add
> > > > > something like Drag.Dragged, because widget.Float only wants Drag.Pos
> > > > > if it has changed (otherwise Float will always overwrite any external
> > > > > changes to Float.Value).  Drag.Dragged also is not quite the right
> > > > > name, because Float wants it to be true even if only a Press event was
> > > > > received.
> > > > >
> > > > > Thinking further, clients (including Float!) will also be interested
> > > > > in knowing when a drag is canceled.  I don't think a Drag.Canceled
> > > > > method is the right API for this – consider if a new drag is started
> > > > > in the same frame, however unlikely (same applies to Release).
> > > > >
> > > > > Maybe a better API will emerge as other widgets start using
> > > > > gesture.Drag.  I'd rather not speculate until we have more concrete
> > > > > use cases.
> > > > >
> > > >
> > > > Ok. FWIW, I'm not entirely happy with the API of the other gesture types
> > > > either.  They seem to behave both like filters (such as your Drag gesture) and
> > > > and injects new events. Perhaps they should inject their higher level events
> > > > in between the (filtered) low-level events.
> > > >
> > > > One thing though: I still think you should have gesture.Drag take an Axis
> > > > and then filter the events to only report drags in that direction. Then you
> > > > can make Drag grab the pointer when it detects drag for longer than touchSlop.
> > >
> > > It already does exactly this.  In the code where you suggested a
> > > switch instead of two ifs :)  Like two lines down from this very
> > > sentence ;D
> > >
> >
> > Indeed, my bad. What confused me was that I had filtering in mind when writing the
> > above. In other words, I believe gesture.Drag should force non-axis aligned pointer.Drags
> > to zero (set scroll x to 0 when axis == Vertical and vice versa). If it doesn't, I worry
> > that someone will set axis wrong and not notice until there are outer scrollers.
> >
> > WDYT?
>
> Yep, that's essentially what I did in the patch I submitted this
> morning.  (There's no notion of scroll in gesture.Drag.  I lock one of
> the coordinates to its starting value, depending on axis.)
>

Indeed. Both patches merged, thanks alot for your patience :)
Export thread (mbox)