Doubleclick doesn't work anymore #275
Conversation
…amps as ticks, not milliseconds. The avalonia code is expeciting timestamp in milliseconds and calculates double click period by milliseconds between clicks. The delta of ticks was way larger than the millisecond value being passed. Switched code from using Stopwatch.GetTimestamp() to DateTimeOffset.UtcNow.ToUnixTimeMilliseconds()
tomlm
changed the title
📝 Walkthrough""" WalkthroughThe pull request introduces a consistent change across multiple classes in the Consolonia project, focusing on timestamp generation for input events. In classes such as Possibly related PRs
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Consolonia.PlatformSupport/CursesConsole.cs (1)
489-492: Verify timestamp consistency across key press events.The timestamps for key press and release events should be as close as possible to ensure accurate timing calculations. Consider capturing the timestamp once before both calls to avoid potential timing discrepancies.
+ var timestamp = (ulong)DateTimeOffset.UtcNow.ToUnixTimeMilliseconds(); - RaiseKeyPress(convertToKey, character, modifiers, true, (ulong)DateTimeOffset.UtcNow.ToUnixTimeMilliseconds()); + RaiseKeyPress(convertToKey, character, modifiers, true, timestamp); Thread.Yield(); - RaiseKeyPress(convertToKey, character, modifiers, false, (ulong)DateTimeOffset.UtcNow.ToUnixTimeMilliseconds()); + RaiseKeyPress(convertToKey, character, modifiers, false, timestamp);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/Consolonia.Core/Infrastructure/ConsoleWindow.cs(1 hunks)src/Consolonia.Core/Infrastructure/DefaultNetConsole.cs(1 hunks)src/Consolonia.NUnit/UnitTestConsole.cs(2 hunks)src/Consolonia.PlatformSupport/CursesConsole.cs(2 hunks)src/Consolonia.PlatformSupport/Win32Console.cs(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (5)
src/Consolonia.Core/Infrastructure/DefaultNetConsole.cs (1)
92-92: LGTM! Timestamp generation correctly updated to milliseconds.The change from
Stopwatch.GetTimestamp()toDateTimeOffset.UtcNow.ToUnixTimeMilliseconds()aligns with the PR objective of fixing double-click detection by using millisecond-based timestamps.Also applies to: 95-95
src/Consolonia.NUnit/UnitTestConsole.cs (1)
124-124: LGTM! Test code updated to use millisecond timestamps.The timestamp generation in test methods has been correctly updated to use milliseconds, maintaining consistency with the production code changes.
Also applies to: 163-163
src/Consolonia.Core/Infrastructure/ConsoleWindow.cs (1)
320-320: LGTM! Mouse event timestamps updated to milliseconds.The timestamp generation for mouse events has been correctly updated to use milliseconds, which is essential for proper double-click detection.
src/Consolonia.PlatformSupport/Win32Console.cs (1)
197-197: LGTM! Platform-specific timestamp generation updated to milliseconds.The Win32-specific implementation has been correctly updated to use millisecond timestamps for both text input and key press events, maintaining consistency with other platform implementations.
Also applies to: 367-367
src/Consolonia.PlatformSupport/CursesConsole.cs (1)
371-371: LGTM: Timestamp conversion aligns with PR objectives.The change from ticks to milliseconds using
DateTimeOffset.UtcNow.ToUnixTimeMilliseconds()correctly addresses the double-click detection issue.
Co-authored-by: <[email protected]>
|
We should not use |
Co-authored-by: <[email protected]>
https://learn.microsoft.com/en-us/dotnet/api/system.environment.tickcount64?view=net-9.0 > Gets the number of milliseconds elapsed since the system started.
|
I switched to Environment.TickCount64 Definition:
|
Co-authored-by: <[email protected]>
When we consolidated the code for dispatching inputs we started sending event timestamps as ticks, not milliseconds. The avalonia code is expecting timestamp to be units of milliseconds and calculates double click period by milliseconds between clicks. The delta of ticks based timestamps was way larger than the millisecond value for double click detection and so no translation to double click was happening.
This fix changes code from using Stopwatch.GetTimestamp() to DateTimeOffset.UtcNow.ToUnixTimeMilliseconds()