Add PageX/PageY/OffsetX/OffsetY to event trigger methods that creates MouseEventArgs

[egil]

[linkdotnet]

Hey Egil,

I just had a small look at this improvement and it raised a question: Does it really make sense to have a method signaturelike this in the first place?

public static void 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)

Instead of directly asking for a MouseEventArgs? like:

public static void  void Click(this IElement element, MouseEventArgs args);

Maybe for convenience we could provide a default as you did:

public static void  void Click(this IElement element) => element.Click(new MouseEventArgs { Detail = 1 };

The same applies basically to all methods in this regard.
I get the idea behind kind of correct filled MouseEventArgs. Often times they are not used and if so I don't think you want to have the default values. But that is just my take.

Advantages of using MouseEventArgs

• 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 ;)

[egil]

Hey Steven,

There are actually three overloads:

public static void 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)
  => _ = ClickAsync(element, new MouseEventArgs { Detail = detail, ScreenX = screenX, ScreenY = screenY, ClientX = clientX, ClientY = clientY, Button = button, Buttons = buttons, CtrlKey = ctrlKey, ShiftKey = shiftKey, AltKey = altKey, MetaKey = metaKey, Type = type! });
public static void Click(this IElement element, MouseEventArgs eventArgs)
  => _ = ClickAsync(element, eventArgs);
public static Task ClickAsync(this IElement element, MouseEventArgs eventArgs) 
  => element.TriggerEventAsync("onclick", eventArgs);

I decided on that pattern because I wanted to make it easier for users to invoke an event handler that required a specific part(s) of a event args to be set. In reality, most probably dont need that, and with the ability to do new() { ... } (added after I created this)` its even less trouble just passing in one of the specific event args.

So yeah, today I would not have the method with all the optional parameters and just do as you suggest. The reason for setting Detail = 1 in this particular case is because that is what Blazor does by default, so I think it makes very good sense to have that as the default as well.

However, removing the lots-of-defaults-methods would be a breaking change, so that is a good idea for clean up for a v2 release. We could definitely start collecting ideas like this for a V2.

[linkdotnet]

Hey Egil, sorry for answering a bit late. Hope you're doing well.
Do you think it would be worth considering putting ObsoleteAttribute on those methods?
Sure it only makes sense if you decide with certainty that those methods will get removed.

Or do you want to handle v2.0 stuff on a backlog?

[egil]

Hi Steven, thanks, doing good. No need to be sorry, we're all doing this in our spare time! Hope you are doing well too!

I am not sure adding Obsolete attributes to the methods now is a good idea. I actually thing we have to remove them to avoid ambiguous methods errors from the compiler, because I do not want to always force users to pass a EventArgs. There should be an overload that takes no arguments and creates an appropriate default XXXEventArgs for users.

Anyway, you are right, obsoleting those methods is a different issue from this. If you want, create it, and I will add a milestone named v2 where it can go into.

[linkdotnet]

Days are getting longer ;) That is very nice :D (at least here in Zurich)

You are right, those topics are not really related. I create a small PR for the current v1 branch which adds those additional fields. Additionally I'll add another "issue" addressing v2.