Skip to content

Conversation

@darrenaustin
Copy link
Contributor

@darrenaustin darrenaustin commented Mar 30, 2022

Part of: #91605

Part of the Material 3 migration for the common buttons is to use the sparkle splash effect if running on Android. This will be handled by the full button migration in #100794, but that is delayed due to an API issue we need to resolve. In the mean time this PR just turns on the sparkle effect (#99731) for buttons when ThemeData.useMaterial3 is turned on and the platform is non-web Android.

Added tests to check that the right splash factory is used for each button.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Mar 30, 2022
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨ LGTM ✨

I need to try this out sometime, I still haven't seen Flutter actually do the sparkle live. Exciting.

Just one potential problem with the test text, and one question:

So if I've got an app that builds to both iOS and Android that uses Material buttons on both, and I set useMaterial3 to true, it will not sparkle on iOS? Just out of curiosity, is it not possible to get iOS to sparkle even if I want to?

@darrenaustin
Copy link
Contributor Author

So if I've got an app that builds to both iOS and Android that uses Material buttons on both, and I set useMaterial3 to true, it will not sparkle on iOS? Just out of curiosity, is it not possible to get iOS to sparkle even if I want to?

It is intended only for Android devices so that is why we have the defaults set up that way. I don't know enough about the implementation whether it would work on iOS. @clocksmith would know more about that.

Also changed the default for useMaterial3 splashFactory when it isn't sparkle to ripple instead of splash.
@fluttergithubbot fluttergithubbot merged commit a8e6fd5 into flutter:master Mar 31, 2022
@darrenaustin darrenaustin deleted the m3_button_ripple branch March 31, 2022 03:31
@clocksmith
Copy link
Contributor

So if I've got an app that builds to both iOS and Android that uses Material buttons on both, and I set useMaterial3 to true, it will not sparkle on iOS? Just out of curiosity, is it not possible to get iOS to sparkle even if I want to?

It is intended only for Android devices so that is why we have the defaults set up that way. I don't know enough about the implementation whether it would work on iOS. @clocksmith would know more about that.

It will currently work on any platform that uses the non-web engine (try manually setting splashFactory for theme to InkSparkle), and eventually it will also work on the CanvasKit engine!

@clocksmith
Copy link
Contributor

But yes, it is correct that the Material guidance is to only use on Android. I will be following up with a post on making custom ripples with the same tech

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 31, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 31, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 31, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 31, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 1, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 1, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 1, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 1, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 1, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 2, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 2, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 4, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 4, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 4, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 4, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 4, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 4, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 4, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 4, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 5, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 5, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 5, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 5, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 5, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 5, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 5, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 5, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: text input Entering text in a text field or keyboard related problems f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants