Idea: Better, more Rusty, MouseData

[rMazeiks]

Currently, MouseData feels very "javascripty", probably because it's a straight copy-paste from the web event.

For Dioxus, we can improve this a bit by grouping data logically and providing the appropriate types. With something like euclid, we can provide coordinates that the users can easily do math on (e.g. difference between points), and statically ensure they don't accidentally mix different coordinate spaces.

Also, it seems that currently, mouse coordinates are i32; they should be f64, since that is how MDN defines them.

This PR

• Add nice and rusty accessor methods for MouseData with correct types
• Deprecates the fields, since the accessors are a much better way to use them
• Simple usage example in mouse_event (formerly coordinates)
• Rename offset -> element coordinates, because offset is weird and ambiguous

look how beautiful it is:

Benefits

• Type safety & ergonomics
• Hides some internal details
• Easier to navigate structs and docs

Todo

• [x] The now deprecated fields are still used throughout the codebase. Remove these.

• [x] Provide a way to construct MouseData without using the deprecated fields. Even with the restructuring, MouseData still logically consists of 7 components. I would love to completely hide the internal representation from users, but I'm not sure how we can provide a constructor that doesn't look like a monstrosity, especially since Rust doesn't have named arguments. The builder pattern? urgh. Or maybe a Coordinates struct with 4 public fields for each of the coordinates and then we can have a decent 4-argument constructor?

Discussion

• [x] Is there an existing crate we can use for structs like a set of modifier keys? This would give better interoperability and we wouldn't reinvent the wheel.

• [x] I brought in a new dependency, euclid, hopefully that's ok? I'm just using it for euclid::Point2D to avoid reinventing the wheel, but maybe it can be used for other stuff later.

• [x] I'm tempted to provide the held modifier keys and mouse buttons as enumset::EnumSet. It would bring in another dependency, but then it would be easy and efficient to perform various operations on them (union, intersection, etc.).

• While reading docs, I found out that button is only "reliable" for events involving mouse up or mouse down. This is bound to cause bugs. I'm guessing it would be best to keep track of the event type and return an Option that is None for irrelevant events.

• ~Maybe we shouldn't deprecate but instead note that it will be deprecated in the future?~

Future work

Maybe some of the other structs can be rustified too. PointerData for example. No promises from my part but I might if this is seen as a Good Change

[jkelleyrtp]

I think there's a lot of value in Dioxus becoming its own "platform" where we rely on some of our own types instead of the types given to us in JS land - especially with the ongoing work in TUI and Native. I see this as a "Good Change". Plus, the 0.3 release is revamping quite a few details in the elements/events/component land. A lot of this stuff isn't majorly breaking, but is still breaking. I see you deprecated the fields which is a good start.

A few thoughts:

• Bringing in euclid is fine. I hate it when libraries make their own bespoke Point2D.
• I don't know about modifier keys. This tends to be a bespoke thing. I know almost all TUI libraries implement their own. I'd prefer to use a popular Rust crate than our own bespoke impl.
• Deprecating is fine since the fields are still available. We can just leave it deprecated for a while.
• EnumSet is good. I personally value interoperability over dependency count. The Rust compiler is always getting faster, so invest in the ecosystem where you can.

I like where this PR is going! Thanks for taking this on!

[rMazeiks]

Cool, thanks for the feedback!

I don't know about modifier keys. This tends to be a bespoke thing. I know almost all TUI libraries implement their own. I'd prefer to use a popular Rust crate than our own bespoke impl.

I found keyboard_types, which seems good and is the most popular of its kind. Didn't find anything for mouse buttons though, so using enumset for that.

A minor drawback of enumset is that we can't use it to represent custom unknown mouse buttons, but this is probably fine – it's unlikely anyone will need a 6th or 7th mouse button in their app. I've made it so that these get converted into an ::Unknown variant – sadly we can't put the unknown code inside if we want to use it in a bit set.

I have now updated the TUI code and the deprecated fields are no longer used (except internally).

I also modified the TUI hover example for testing the mouse coordinates.

I noticed that, when handling mouse move events, TUI gets laggy. Not sure why.

---

(merged #401 into this one for a bugfix, and to prevent failing tests)

[rMazeiks]

yay compiler bug. only on windows/mac though. something about "no MIR available for DefId"

similar issue already recently discussed here https://github.com/rust-lang/rust/issues/84455

[ealmloff]

I noticed that, when handling mouse move events, TUI gets laggy. Not sure why.

Currently, all events are triggering a redraw. Mouse move happens the most, so it is most noticeable. #402 should fix it if it only conditionally updates the state. Aside from that, a bounding volume higharched should help with faster collisions and allow for partial re-rendering.

[rMazeiks]

402 should fix it if it only conditionally updates the state.

Awesome, much smoother now!

[rMazeiks]

what do we do about the internal compiler errors?

[rMazeiks]

regarding compiler error:

i was caused by this as i found after some investigation. to avoid this, i renamed the module, avoiding conflict with the input element. probably not a good idea to have that conflict anyway.

[jkelleyrtp]

regarding compiler error:

i was caused by this as i found after some investigation. to avoid this, i renamed the module, avoiding conflict with the input element. probably not a good idea to have that conflict anyway.

That is weird... I think this is ICE #3 we've ran into as part of Dioxus. Now that it seems fixed I think it looks good to merge?

[rMazeiks]

I think this is ICE https://github.com/DioxusLabs/dioxus/pull/3 we've ran into as part of Dioxus

interesting, big project i guess xd

Now that it seems fixed I think it looks good to merge?

tests pass. manually tested mouse data on desktop and tui, seems ok. outstanding issues resolved and im happy with it.

the only remaining concern i have is that trigger_button may return nonsense for events which have no trigger button. but that was also kinda true before this pr. the way to fix it would probably involve updating the internal representation of MouseData which will be a breaking change but im hoping to do once it's been deprecated for long enough. i left a todo.

if you (and anyone else) are happy with the API, it should be good to merge!

[jkelleyrtp]

Thank you! :) It'll be good to have the conveniences of Euclid for canvas-y type operations.