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

Click and DoubleClick should directly use MouseEventArgs #617

Open
linkdotnet opened this issue Jan 31, 2022 · 0 comments
Open

Click and DoubleClick should directly use MouseEventArgs #617

linkdotnet opened this issue Jan 31, 2022 · 0 comments
Milestone

Comments

@linkdotnet
Copy link
Collaborator

linkdotnet commented Jan 31, 2022

Current Situation

The current state offers multiple overloads with a dozen of properties which are mapped one to one to the MouseEventArgs:

Click(this IElement element, long detail = 1, double screenX = default, double screenY = default, double clientX = default, double clientY = default, long button = default, long buttons = default, bool ctrlKey = default, bool shiftKey = default, bool altKey = default, bool metaKey = default, string? type = default)

The motivation behind is to have a "valid" argument passed down which behaves like Blazor would in a "real" scenario (detail = 1 for example for Click and detail = 2 for DoubleClick).

Proposed Solution

bUnit already offers an overload which takes the MouseEventArgs. This decouples bUnit from the supported .NET versions. For example in .net5 OffsetX and OffsetY was introduced. In .net6 PageX and PageY was introduced. Thus this "simple" version just works.

Furthermore the suggestion would be to delete Click and DoubleClick with all it's overloads and just offer 2 versions:

public void Click(this IElement element, MouseEventArgs mouseEventArgs); // Already exists

public void Click(this IElement element) => Click(element, new MouseEventArgs { detail = 1}); // New one to be in line with Blazor

In any way a small migration guide should be added to the docs.

Advantages:

  • If Microsoft extends that class we don't have to do anything (you could also argue that is a bad thing)
  • In my opinion more readable than n parameters. Also documentation for free from Microsoft
  • Also more cohesive as a lot of the default parameters are often times not used
  • ClickAsync which is also public does use the MouseEventArgs. That seems a bit in-cohesive as well. Normal expectation is that the sync and async version have the same parameter (except maybe CancellationToken or similar stuff)
  • The enhancement would come for free ;)

Disadvantages:

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

No branches or pull requests

2 participants