Skip to content

DispatcherQueueTimerExtensions.Debounce does not work with leading edge action #143

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

Closed
2 of 13 tasks
huynhsontung opened this issue Jul 5, 2023 · 6 comments · Fixed by #569
Closed
2 of 13 tasks

Comments

@huynhsontung
Copy link

huynhsontung commented Jul 5, 2023

Describe the bug

Debounce(immediate: true) does not work with a default timer. When a timer is created, IsRepeating is set to true by default. This breaks the logic of Debounce when immediate: true. Setting IsRepeating to false on the timer fixes this issue.

Steps to reproduce

  1. Create a DispatcherQueueTimer using DispatcherQueue.GetForCurrentThread().CreateTimer()
  2. Use Debounce() on that timer with immediate: true
  3. The action is only triggered once and never again.

Expected behavior

Subsequent actions should trigger once the timer expires just like without the immediate flag.

Windows Build Number

  • Windows 10 1809 (Build 17763)
  • Windows 10 1903 (Build 18362)
  • Windows 10 1909 (Build 18363)
  • Windows 10 2004 (Build 19041)
  • Windows 10 20H2 (Build 19042)
  • Windows 10 21H1 (Build 19043)
  • Windows 11 21H2 (Build 22000)
  • Other (specify)

App minimum and target SDK version

  • Windows 10, version 1809 (Build 17763)
  • Windows 10, version 1903 (Build 18362)
  • Windows 10, version 1909 (Build 18363)
  • Windows 10, version 2004 (Build 19041)
  • Other (specify)

Visual Studio Version

2022

Help us help you

Yes, I'd like to be assigned to work on this item.

@ghost
Copy link

ghost commented Jul 5, 2023

Hello huynhsontung, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

@michael-hawker michael-hawker transferred this issue from CommunityToolkit/WindowsCommunityToolkit Jul 14, 2023
@michael-hawker
Copy link
Member

Thanks for the issue @huynhsontung, moved this to the new repo. Seems like we just have a couple of simple tests and not one for this scenario: https://github.com/CommunityToolkit/Windows/blob/main/components/Extensions/tests/DispatcherQueueTimerExtensionTests.cs

In immediate mode though, there should be only one firing of the event at the start. It'll only fire again if a request comes in after the elapsed period of time. For my clarity, is that not what you're observing? Or you're saying that it never fires again after the first time?

The docs don't say what the default should be, I filed an issue: MicrosoftDocs/winrt-api#2386

I would have thought IsRepeating was false by default, as DispatcherTimer didn't have it. I wonder if this was a change in the WASDK version, which platform are you seeing this behavior on? If the two platforms are different defaults, then that seems like a regression we should file against the platform?

In either case, seems like we should add some tests here for this scenario to our matrix to be sure.

@huynhsontung
Copy link
Author

Hello @michael-hawker,

In immediate mode, it never fires again after the first time.

I am developing for UWP (min version 18362, target version 22000) but I can confirm that IsRepeating is true by default for both UWP and WASDK. I can work on a patch for this issue from our end.

@michael-hawker
Copy link
Member

michael-hawker commented Jul 14, 2023

I'm surprised IsRepeating is true. I wonder if we missed this fact when we converted these from DispatcherTimer helpers to DispatcherQueueTimer helpers?

Is there value in these if IsRepeating is true? Do we just have to require/document that IsRepeating should be turned off for this function to work properly you think? (Or I suppose we can have the function turn it off itself if required...)

@huynhsontung
Copy link
Author

I think the feature should continue to work regardless of IsRepeating setting. Users may rely on IsRepeating for their own timer logic. Like its non-immediate mode, we can handle the DispatcherQueueTimer.Tick event for triggering action as well in this case.

@michael-hawker
Copy link
Member

Hey @huynhsontung not sure why we didn't resolve or fix this. I had some other Debounce clean-up to do, so I have fixed this in #569 - there was some issues in one of the original tests, so maybe that's why I was confused here in the past.

Feel free to take a look. I'll add a bit more to the new docs page and a test or custom Tick call you mention here as well (which I think should work and just get called when the debouncer fires).

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 a pull request may close this issue.

2 participants