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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add HapticFeedback in IconButton #1807

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d7c4cea
feature: add ConfirmHapticFeedback in IconButton
kosenda Jan 13, 2025
ff6fde6
fix: change of call sequence
kosenda Jan 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion ...ain/kotlin/com/google/samples/apps/nowinandroid/core/designsystem/component/IconButton.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import androidx.compose.material3.MaterialTheme
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.hapticfeedback.HapticFeedbackType
import androidx.compose.ui.platform.LocalHapticFeedback
import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons
import com.google.samples.apps.nowinandroid.core.designsystem.theme.NiaTheme

Expand All @@ -48,11 +50,15 @@ fun NiaIconToggleButton(
icon: @Composable () -> Unit,
checkedIcon: @Composable () -> Unit = icon,
) {
val haptics = LocalHapticFeedback.current
// TODO: File bug
// Can't use regular IconToggleButton as it doesn't include a shape (appears square)
FilledIconToggleButton(
checked = checked,
onCheckedChange = onCheckedChange,
onCheckedChange = { isChecked ->
if (isChecked) haptics.performHapticFeedback(HapticFeedbackType.Confirm)
Copy link

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.

Copy link
Author

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?

Copy link

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.

Copy link
Author

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!

onCheckedChange(isChecked)
},
modifier = modifier,
enabled = enabled,
colors = IconButtonDefaults.iconToggleButtonColors(
Expand Down