fyne icon indicating copy to clipboard operation
fyne copied to clipboard

Extending a Tappable with DoubleTappable causes a loss of Tappability for containing widget

Open kontza opened this issue 3 years ago • 10 comments

Describe the bug:

I have an app that contains a list where list items are Labels. My plan was to extend those list items with double tappability. This didn't work out. My double tap handler is being called successfully, but single taps, i.e. list item selections don't work anymore.

To Reproduce:

Steps to reproduce the behaviour:

  1. Clone https://github.com/kontza/fyne_doubletap_test
  2. $ go run . -d
  3. If selections would work, it would print out "Selected ..." on stdout when an item is tapped.
  4. Re-run, this time without -d. This causes the list to be created with unextended Labels.
  5. Tap an item, and notice how "Selected ..." is printed out to stdout.

Screenshots:

N/A

Example code:

See https://github.com/kontza/fyne_doubletap_test

Device (please complete the following information):

  • OS: macOS
  • Version: 11.6 (Big Sur)
  • Go version: go1.17.2 darwin/amd64
  • Fyne version: 2.1.0

kontza avatar Oct 15 '21 05:10 kontza

Just a friendly FYI, I would have suggested putting the code in a code block inside the issue description in the future (for readability and ease of reproduction). However, I see that you made sure to have a minimal code example, so I’m happy either way :)

Jacalz avatar Oct 15 '21 06:10 Jacalz

I had a very similar problem in #2470 . One possible workaround is to add a field "id-in-list" and a reference to the list to the extended widget, keep them up-to-date in the list updateFunc() callback, then extend the Tapped callback as follows:

func (w *extendedWidget) Tapped(_ *fyne.PointEvent) {
        w.ExtendBaseWidget(w)
        w.<REFERENCE TO LIST>.Select(w.<ID IN LIST>)
}

nullst avatar Oct 15 '21 07:10 nullst

Just a friendly FYI, I would have suggested putting the code in a code block inside the issue description in the future (for readability and ease of reproduction). However, I see that you made sure to have a minimal code example, so I’m happy either way :)

I contemplated between inline and external, and decided to use the external repo in fear of inline code being tl;dr 😁

kontza avatar Oct 15 '21 14:10 kontza

I had an idea with the test app: what if I stop extending Label, and add logic to OnSelected as follows:

doubleTapDelay := time.Millisecond * 300
doubleTapTimeout := time.Now()
previousItemId := -1
list.OnSelected = func(i widget.ListItemID) {
	singleTapper := func() {
		previousItemId = i
		doubleTapTimeout = time.Now().Add(doubleTapDelay)
		log.Printf("Selected %v", i)
	}
	if time.Now().After(doubleTapTimeout) {
		singleTapper()
	} else {
		if i == previousItemId {
			doubleTapTimeout = time.Now()
			log.Printf("DoubleTapped %v", i)
		} else {
			singleTapper()
		}
	}
}

The basic idea is to handle all selects "regularly", but also add a timeout value into future (could Fyne export the default doubleClickDelay?). On next OnSelect we check if we're within the double-click period, and act accordingly if we are, i.e. do our own DoubleTap-processing. Otherwise, or if selected item's ID has changed, act as a new selection.

This didn't work because list.go has this check

// Select add the item identified by the given ID to the selection.
func (l *List) Select(id ListItemID) {
	if len(l.selected) > 0 && id == l.selected[0] {
		return
	}
	...

If I comment that out, then my logic in OnSelect works.

Next I'll try extending the List with a double tap to see, if I can workaround my double tappability problem without touching the framework.

kontza avatar Oct 17 '21 08:10 kontza

Making a DoubleTappable List didn't work either. Or, it works when I double-click inside the list on the area where there are no items. If I double-click on an item, the processing for a tap on a label swallows the double tap and it doesn't bubble up to the List to be processed.

kontza avatar Oct 17 '21 09:10 kontza

Probably I'll have to persuade our designer to accept the fact that there'll be no double-clicking list items.

kontza avatar Oct 17 '21 11:10 kontza

Probably I'll have to persuade our designer to accept the fact that there'll be no double-clicking list items.

What about the workaround I suggested above? Since you are extending labels with DoubleTapped() callback, also implement a Tapped callback that would execute List.Select on your list, selecting the element with the corresponding ListID. For this you'll need your extended widget to also store the reference to the list and the ListID of the element, but you can easily keep this information up-to-date in updateFunc callback of the list. That's not a nice workaround, but it solved the issue for me.

nullst avatar Oct 17 '21 13:10 nullst

... but it solved the issue for me.

Thanks for the tip, I'll try that. (I totally forgot about your earlier suggestion.)

Ok, I tried that. It works, as you say, but there's a delay between a tap and a selection. From my earlier tests today, it seems very much like the default 300 ms delay. But, perhaps we can live with that.

kontza avatar Oct 17 '21 14:10 kontza

You can use MouseDown() and/or TouchDown() callbacks instead to avoid the delay.

nullst avatar Oct 18 '21 14:10 nullst

I also encoutered this bug.

I noticed something else, that could help to fix it: when clicked something Tappable, there is a delay before the Tapped handler is getting called, it's like fyne "waits" to see if this is a Tapped or DoubleTapped event.

The correct behaviour in my opinion is when something is Tapped, immediatly call OnTapped (to avoid the delay), then if there is a DoubleTap, call the DoubleTapped handler. Like this, the tapped event won't be "lost"

matwachich avatar Jun 26 '22 22:06 matwachich