1911369 - [Toolbar redesign] Migrate from rememberRipple() to ripple()

Status: RESOLVED FIXED

(Firefox for Android :: Toolbar, enhancement, P3)

Description

[Roger Yang [:royang]]

The reason as follows:

1. Visual feedback ripple it cant be seen because of menu and user's thumb/finger.
2. Haptic feedback on long pressing.
3. Performance gain with no ripple effect set (due to rememberRipple)

Comment 1

[Roger Yang [:royang]]

Maybe a better solution is to move from rememberRipple to ripple
https://developer.android.com/develop/ui/compose/touch-input/user-interactions/migrate-indication-ripple#using-material

Comment 2

[Roger Yang [:royang]]

https://developer.android.com/jetpack/androidx/releases/compose-material

Comment 3

[Chris Peterson [:cpeterson]]

Waiting for Compose Material version 1.7

Comment 4

[Ryan VanderMeulen [:RyanVM]]

(In reply to Chris Peterson [:cpeterson] from comment #3)

Waiting for Compose Material version 1.7

Which is now conveniently available.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1911369 - Update AndroidX Compose BOM to version 2024.09.00.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1911369 - Revert bug 1915975.

It should no longer be needed with Compose 1.7 and Ripple API changes.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1911369 - Use quadraticTo() instead of quadraticBezierTo().

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1911369 - Adapt to Ripple API changes.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1911369 - Fix SuspiciousModifierThen errors.

Modifier.kt:49: Error: Using Modifier.then with a Modifier factory function with an implicit receiver [SuspiciousModifierThen from androidx.compose.ui]
ReorderableList.kt:283: Error: Using Modifier.then with a Modifier factory function with an implicit receiver [SuspiciousModifierThen from androidx.compose.ui]

https://googlesamples.github.io/android-custom-lint-rules/checks/SuspiciousModifierThen.md.html

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1911369 - Suppress ClickableText deprecation warnings.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Here's a perfherder comparison link for perftest-android-hw-a51-startup-fenix-cold for this patchset vs. plain m-c tip (which includes the fix from bug 1915975):
https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=ecb01ec517d59b7967c84a62b48aee02d8e36ca3&newProject=try&newRevision=9d52896824eabb7f0d4e78ccc21c769e78040669&framework=15&page=1

Looks like it's pretty neutral to a slight improvement. Over to you for more testing, Roger!

Comment 12

[Roger Yang [:royang]]

looks great. I did a profile with your changes and the rememberRipple performance cost is definitely gone. Please feel free to move forward with this. Thanks!

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1911369 - Remove some obsolete ExperimentalMaterialApi annotations.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1deb428dd28d
Update AndroidX Compose BOM to version 2024.09.00. r=android-reviewers,Roger
https://hg.mozilla.org/integration/autoland/rev/c36dc110dba3
Revert bug 1915975. r=android-reviewers,Roger
https://hg.mozilla.org/integration/autoland/rev/d37d7be320ce
Use quadraticTo() instead of quadraticBezierTo(). r=android-reviewers,Roger
https://hg.mozilla.org/integration/autoland/rev/2801135d0ebe
Adapt to Ripple API changes. r=android-reviewers,mcarare,Roger
https://hg.mozilla.org/integration/autoland/rev/fd44a9da188a
Fix SuspiciousModifierThen errors. r=android-reviewers,Roger
https://hg.mozilla.org/integration/autoland/rev/4fe85c6bb778
Suppress ClickableText deprecation warnings. r=android-reviewers,Roger
https://hg.mozilla.org/integration/autoland/rev/c37b6b3302f8
Remove some obsolete ExperimentalMaterialApi annotations. r=android-reviewers,Roger

Comment 15

[tszentpeteri]

https://hg.mozilla.org/mozilla-central/rev/1deb428dd28d
https://hg.mozilla.org/mozilla-central/rev/c36dc110dba3
https://hg.mozilla.org/mozilla-central/rev/d37d7be320ce
https://hg.mozilla.org/mozilla-central/rev/2801135d0ebe
https://hg.mozilla.org/mozilla-central/rev/fd44a9da188a
https://hg.mozilla.org/mozilla-central/rev/4fe85c6bb778
https://hg.mozilla.org/mozilla-central/rev/c37b6b3302f8