-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Add HapticFeedback in IconButton #1807
base: main
Are you sure you want to change the base?
Conversation
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.
Haptic feedback should be called first. Also using HapticFeedbackType.Confirm instead of HapticFeedbackType.ToggleOn/ToggleOff is bit tricky.
onCheckedChange = onCheckedChange, | ||
onCheckedChange = { isChecked -> | ||
onCheckedChange(isChecked) | ||
if (isChecked) haptics.performHapticFeedback(HapticFeedbackType.Confirm) |
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.
I think haptic feedback should be called before line 59.
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.
@anhtuannd
Thank you for pointing this out!
If you don't mind, I'd like to know why this need to run the haptic feedback first?
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.
If you look into https://developer.android.com/reference/kotlin/androidx/compose/ui/hapticfeedback/HapticFeedbackType, there are two categories of events:
- When user executes some actions and app should feedback immediately.
- Confirmation/Reject indicates completion/failure (or reject) of a user interaction.
So when user has checked, HapticFeedbackType.ToggleOn/ToggleOff should be called first. HapticFeedbackType.Confirm/Reject can be called after onCheckedChange(isChecked) but should be based on its result.
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.
I see!
As you said, I made sure to call haptics feedback first!
What I have done and why
I thought it would be better to have haptics feedback to make it more obvious that the button was tapped, so I added haptics feedback to the commonly used IconButton
ref
Android Developers - Compose UI - 1.8.0-alpha06
Android Developers - HapticFeedbackType
Android Developers - Tap and press # long-press-show
What I have tried
I thought that
HapticFeedbackType
hadToggleOn
andToggleOff
, which seemed to be better, but on my device Pixel 6(Android 15), I didn't get any haptic feedback.So I thought
Confirm
was appropriate.