Skip to content

Conversation

pujagani
Copy link
Contributor

@pujagani pujagani commented Jan 5, 2023

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Allow setting a different pointer, keyboard, or wheel on input device without using Action builder

Motivation and Context

Related to proposal 1 described in #10724

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
@pujagani pujagani added the C-dotnet .NET Bindings label Jan 5, 2023
@pujagani pujagani marked this pull request as draft January 5, 2023 09:04
@pujagani pujagani force-pushed the add-default-device branch from aab1520 to 71fe796 Compare January 6, 2023 06:37
@pujagani pujagani marked this pull request as ready for review January 6, 2023 06:41
@pujagani pujagani requested a review from titusfortner January 6, 2023 06:41
@titusfortner titusfortner added this to the 4.9 milestone Jan 14, 2023
@titusfortner
Copy link
Member

One interesting side effect of this implementation is that devices aren't created by default when class is loaded, but lazily generated (which is what Ruby does and I like it)... but I can't remember how it works to create a new device from the last time I was in the .NET actions code. If we've taken keyboard actions and then create a new pointer device, does it create an array of the size of the keyboard device actions list and add pauses, or will the first pointer action happen at the same time as the first keyboard action?

I'm not sure if we have a test covering this use case, and if not we need to add them.

@pujagani
Copy link
Contributor Author

Thank you for taking a look. I am not very familiar with the actions code. I am digging into it and will provide more information. I don't think the test exists for the use case mentioned, I think adding it will help me answer your question.

@pujagani
Copy link
Contributor Author

pujagani commented Jan 31, 2023

"If we've taken keyboard actions and then create a new pointer device, does it create an array of the size of the keyboard device actions list and add pauses" - Based on debugging and my understanding, this is what is happening. I have also added a test to confirm the same. Is that the expected behavior? @titusfortner

@diemol diemol modified the milestones: 4.9, 4.10 Apr 17, 2023
@titusfortner titusfortner self-assigned this May 18, 2023
@titusfortner titusfortner modified the milestones: 4.10, 4.11 Jun 6, 2023
@titusfortner titusfortner removed this from the 4.11 milestone Jun 27, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 11, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (4468622) 57.11% compared to head (5ae0782) 57.11%.

❗ Current head 5ae0782 differs from pull request most recent head 16c09a2. Consider uploading reports for the commit 16c09a2 to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@ Coverage Diff @@ ## trunk #11513 +/- ## ======================================= Coverage 57.11% 57.11% ======================================= Files 86 86 Lines 5365 5365 Branches 193 193 ======================================= Hits 3064 3064 Misses 2108 2108 Partials 193 193 

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@titusfortner titusfortner merged commit fd36c53 into SeleniumHQ:trunk Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-dotnet .NET Bindings

4 participants