-
-
Notifications
You must be signed in to change notification settings - Fork 932
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
feat: Add onReleased
action to AdvancedButtonComponent
which will be called within onTapUp
#3152
Conversation
`AdvancedButtonComponent` doesn't show the `downSkin` if the action of `onPressed` changes the router to another page. That is because the router is changed before the `downSkin` is shown as the `onPressed` action is called in `onTapDown` instead of `onTapUp`. In this PR, calling `onPressed` action is moved to `onTapUp` method instead of `onTapDown`. That makes the `AdvancedButtonComponent` more natural and allows users to skip the action if they move the mouse pointer or their finger outside the button box.
The failed test case seems unrelated to my changes and all tests are passed locally with |
I don't think this is worth a breaking change, it's better to expose a callback for onTapUp. In UI's buttons often trigger in onTapUp, but in games, when mashing a fire button for example, you usually want onTapDown. |
Yeah, that makes sense. I didn't think about it, since I'm developing mainly board games, which are kind of UI apps. |
Does it worth adding a callback to the |
I think it would be worth adding! I guess we should add |
…ng onPressed behaviour
I have added |
AdvancedButtonComponent
call onPressed
in onTapUp
instead of onTapDown
onReleased
action to AdvancedButtonComponent
which will be called within onTapUp
Sounds good! Then we can refactor it to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, thanks for your contribution!
Thanks a lot for the fast review |
Description
Add
onReleased
action which will be called withinonTapUp
. That fixed some scenarios which require the pressed action to be called after the user taps up the button.For example,
AdvancedButtonComponent
doesn't show thedownSkin
if the action ofonPressed
changes the router to another page. That is because the router is changed before thedownSkin
is shown as theonPressed
action is called inonTapDown
.Checklist
docs
and added dartdoc comments with///
.examples
ordocs
.Breaking Change?
Related Issues
Fixes #3150