ScenicFramework / scenic

Core Scenic library
Apache License 2.0
1.97k stars 137 forks source link

New component events #297

Closed GPrimola closed 6 months ago

GPrimola commented 2 years ago

Description

This PR adds new events to the following built in components:

Motivation and Context

Those new events are very helpful in certain scenarios.

Types of changes

Checklist

crertel commented 2 years ago

Would you consider :focus instead of :focus_in and :blur instead of :focus_out?

crertel commented 2 years ago

Also, for the over events, how do you feel about also perhaps sending a cursor position?

(Great work, btw!)

GPrimola commented 2 years ago

Hey @crertel sorry for the long delay to reply you and thanks for your suggestions!

Sure, :focus and :blur are totally fine too, I'll push this change.

About sending the cursor position on the :hover event, I'm not sure. Could you give me any use case for that? Would you imagine this cursor position being local cursor position (relative to the component or to the item box) or global position (the position on the viewport)?

My thoughts about this is should anyone need the cursor position they should request the :cursor_position input and combine the event's data using assigns on the scene. But this may be because I don't have any use case in mind.

crertel commented 2 years ago

I'm pretty flexible on this...I'm thinking that it might be helpful for cases where we want to handle something local to the component (say, updating a gradient or some other UI effect) and doing it elsewhere could be clunky. Not super wed to it though--what do you think? :)

GPrimola commented 1 year ago

@crertel I tried to run mix test locally to see what's going on, and it succeeded, maybe this is a flaky test?

Screenshot 2023-08-04 at 11 18 25
GPrimola commented 1 year ago

Hey @crertel would you mind to run the CI/build action again? I've checked and it seems all good:

Screenshot 2023-08-08 at 10 06 07

Many thanks and sorry for the pings 🙂

crertel commented 8 months ago

Looks green! Great work, I bet folks will get some good use out of these. Very sorry for the delay on our side. Lemme page @axelson in too, but I'm liking this.