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

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

Open
kontza opened this issue Oct 15, 2021 · 10 comments
Labels
bug Something isn't working

Comments

@kontza
Copy link

kontza commented Oct 15, 2021

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 kontza added the unverified A bug that has been reported but not verified label Oct 15, 2021
@Jacalz
Copy link
Member

Jacalz commented Oct 15, 2021

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 :)

@nullst
Copy link
Contributor

nullst commented Oct 15, 2021

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>)
}

@kontza
Copy link
Author

kontza commented Oct 15, 2021

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
Copy link
Author

kontza commented Oct 17, 2021

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
Copy link
Author

kontza commented Oct 17, 2021

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
Copy link
Author

kontza commented Oct 17, 2021

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

@nullst
Copy link
Contributor

nullst commented Oct 17, 2021

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.

@kontza
Copy link
Author

kontza commented Oct 17, 2021

... 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.

@nullst
Copy link
Contributor

nullst commented Oct 18, 2021

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

@andydotxyz andydotxyz changed the title Extending a Tappable with DoubleTappable causes a loss of Tappability Extending a Tappable with DoubleTappable causes a loss of Tappability for containing widget Oct 25, 2021
@andydotxyz andydotxyz added bug Something isn't working and removed unverified A bug that has been reported but not verified labels Oct 25, 2021
@andydotxyz andydotxyz removed this from the Bowmore Release (early 2022) milestone Jun 3, 2022
@matwachich
Copy link
Contributor

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"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants