Skip to content

Conversation

@xorgy
Copy link
Member

@xorgy xorgy commented Jun 24, 2025

No description provided.

@xorgy xorgy requested a review from waywardmonkeys June 24, 2025 15:21
@xorgy xorgy force-pushed the ui-events-0.2.0 branch from 39583ab to 7298203 Compare June 25, 2025 23:59
@waywardmonkeys
Copy link
Contributor

Can you rebase this forward?

Copy link
Member

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good (once this is released)

delta: ScrollDelta::PixelDelta(PhysicalPosition::<f64> { x: 0., y: 25. }),
state: PointerState::default(),
});
harness.mouse_wheel(Vec2 { x: 0., y: 25. });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this!

@xorgy
Copy link
Member Author

xorgy commented Aug 13, 2025

Would like a yes/no on the destructuring-based version (second commit) vs the opaque version (first commit), as we're about to do an ui-events-0.2.0 so we need to choose one. :+ )

@waywardmonkeys
Copy link
Contributor

@DJMcNab That's a plea for you to comment on that. :)

@waywardmonkeys waywardmonkeys requested a review from DJMcNab August 13, 2025 10:13
@DJMcNab
Copy link
Member

DJMcNab commented Aug 13, 2025

Destructuring is fine by me. It's certainly better than extremely short names.
The lines like PointerEvent::Up(PointerButtonEvent { button, ..}) => having two instances of each of Pointer and Event isn't great, but I don't see that being avoidable (without inline structs or equivalent).

@xorgy xorgy force-pushed the ui-events-0.2.0 branch 2 times, most recently from a632a69 to 5f27425 Compare September 3, 2025 14:25
@xorgy xorgy marked this pull request as ready for review October 28, 2025 03:22
@xorgy
Copy link
Member Author

xorgy commented Oct 28, 2025

Alright, we've now published ui-events and ui-events-winit v0.2.0, and this branch is up to date for that. :+ )

Copy link
Contributor

@waywardmonkeys waywardmonkeys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would still like to get feedback from @DJMcNab on this.

Copy link
Member

@DJMcNab DJMcNab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the blocking rebase artifact, this looks good. I do have a couple of other thoughts, but nothing too significant.

Thanks!

@xorgy xorgy enabled auto-merge October 28, 2025 15:12
@xorgy xorgy added this pull request to the merge queue Oct 28, 2025
Merged via the queue into linebender:main with commit de88321 Oct 28, 2025
17 checks passed
@xorgy xorgy deleted the ui-events-0.2.0 branch October 28, 2025 15:30
github-merge-queue bot pushed a commit that referenced this pull request Oct 29, 2025
See discussion in [#xilem >
v0.4.0](https://xi.zulipchat.com/#narrow/channel/354396-xilem/topic/v0.2E4.2E0/with/547457987).
Target date is tomorrow to just let everything settle (e.g. #1103).

https://linebender.org/wiki/process/release/#checklist

I have run a publish dry run, using the command:
```sh
cargo publish --dry-run -p xilem -p xilem_core -p masonry -p masonry_core -p masonry_winit -p masonry_testing -p tree_arena -p xilem_web
```
We don't have a changelog (yet, plan is to start immediately after this
releasee), we have bumped dependencies (pending #1103).
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

Successfully merging this pull request may close these issues.

3 participants