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

New component events #297

Merged
merged 5 commits into from
May 5, 2024
Merged

Conversation

GPrimola
Copy link
Contributor

@GPrimola GPrimola commented Oct 13, 2022

Description

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

  • Scenic.Component.Button

    • {:btn_pressed, id} - sent when a button is pressed
    • {:btn_released, id} - sent after the button is released
  • Scenic.Component.Input.TextField

    • {:focus, id} - sent when a text field is "active", i.e., is ready to receive inputs
    • {:blur, id} - sent when a text field becomes "inactive", i.e., will not receive inputs
  • Scenic.Component.Input.Dropdown

    • {:dropdown_opened, id} - sent when the dropdown opens
    • {:dropdown_closed, id} - sent when the dropdown closes
    • {:dropdown_item_hover, id, item_id} - sent when and item is hovered

Motivation and Context

Those new events are very helpful in certain scenarios.

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to
    not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any feature
    but make things better)

Checklist

  • Check other PRs and make sure that the changes are not done yet.
  • The PR title is no longer than 64 characters.

@crertel
Copy link
Contributor

crertel commented Oct 15, 2022

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

@crertel
Copy link
Contributor

crertel commented Oct 15, 2022

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

(Great work, btw!)

@GPrimola
Copy link
Contributor Author

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
Copy link
Contributor

crertel commented Oct 30, 2022

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 GPrimola force-pushed the new-component-events branch from a866c56 to 9a736ae Compare August 2, 2023 07:44
@GPrimola
Copy link
Contributor Author

GPrimola commented Aug 4, 2023

@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
Copy link
Contributor Author

GPrimola commented Aug 8, 2023

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 🙂

@GPrimola GPrimola force-pushed the new-component-events branch from 9a736ae to 1b98178 Compare February 7, 2024 16:00
@crertel
Copy link
Contributor

crertel commented Feb 15, 2024

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.

@crertel crertel merged commit aa51267 into ScenicFramework:main May 5, 2024
4 checks passed
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.

2 participants