Fix context menu button color on Android when textButtonTheme is set #133271

Renzo-Olivares · 2023-08-24T18:42:33Z

When setting a textButtonTheme it should not override the native context menu colors.

  theme: ThemeData(
    textButtonTheme: const TextButtonThemeData(
      style: ButtonStyle(
        backgroundColor: MaterialStatePropertyAll<Color>(
            Color(0xff05164d)), // blue color
      ),
    ),
  ),

Before	After

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 · 2023-08-24T21:18:15Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #133271 at sha a4e0bcca9c16282ae2bf0275f84d5a1e228b1fcc

justinmc

One idea about how to simplify this, but if I'm wrong then it looks fine as-is.

justinmc · 2023-08-25T17:23:35Z

packages/flutter/test/material/text_selection_toolbar_text_button_test.dart

Are we supposed to use testWidgetsWithLeakTracking for everything now?

Not sure about that. @polina-c do you have any thoughts on when we should use testWidgetsWithLeakTracing?

justinmc · 2023-08-25T17:31:20Z

packages/flutter/lib/src/material/text_selection_toolbar_text_button.dart

What if you just hardcode this backgroundColor to transparent?

In my context menu color PR, I set the context menu's background color to surface on the TextSelectionToolbar under the assumption that these TextSelectionToolbarButtons would be transparent and so look like the color behind them. It looks like the problem in this issue is that setting textButtonTheme's backgroundColor changes the buttons to be non-transparent.

Just a hunch, let me know if it works. If so, maybe it's a preferable solution since the logic about the color of these buttons isn't stored in two places.

That makes sense! Changing the backgroundColor to Colors.transparent works as expected.

justinmc

LGTM 👍

… is set (flutter/flutter#133271)

flutter/flutter@ec387a4...6c95737 2023-08-29 [email protected] Roll Flutter Engine from d84763e04496 to 65438c7bb46a (1 revision) (flutter/flutter#133563) 2023-08-29 [email protected] Roll Flutter Engine from 0625049b5ee3 to d84763e04496 (6 revisions) (flutter/flutter#133559) 2023-08-29 [email protected] Roll Flutter Engine from c2ec683c4ea8 to 0625049b5ee3 (2 revisions) (flutter/flutter#133538) 2023-08-29 [email protected] Roll Flutter Engine from 3d1faae1bc2f to c2ec683c4ea8 (4 revisions) (flutter/flutter#133535) 2023-08-29 [email protected] Roll Flutter Engine from c7c61ff9732c to 3d1faae1bc2f (1 revision) (flutter/flutter#133530) 2023-08-29 [email protected] Fix bug in setPreferredOrientations example (flutter/flutter#133503) 2023-08-29 [email protected] Roll Flutter Engine from 9778c2cb1d51 to c7c61ff9732c (2 revisions) (flutter/flutter#133527) 2023-08-29 [email protected] Roll Flutter Engine from ffda7e3cfc7f to 9778c2cb1d51 (1 revision) (flutter/flutter#133526) 2023-08-29 [email protected] Roll Flutter Engine from 1e821961e96a to ffda7e3cfc7f (2 revisions) (flutter/flutter#133523) 2023-08-29 [email protected] Roll Flutter Engine from 45e2b41e5ae5 to 1e821961e96a (2 revisions) (flutter/flutter#133520) 2023-08-29 [email protected] Roll Flutter Engine from bd2132a0814f to 45e2b41e5ae5 (2 revisions) (flutter/flutter#133518) 2023-08-29 [email protected] add missing forwards of local-engine-host in benchmark runners (flutter/flutter#133517) 2023-08-28 [email protected] Roll Flutter Engine from a7a4c1c70bad to bd2132a0814f (3 revisions) (flutter/flutter#133516) 2023-08-28 [email protected] FocusNode and FocusManager should dispatch creation in constructor. (flutter/flutter#133490) 2023-08-28 [email protected] Use flutter pub get instead of dart pub get in create_api_docs.dart (flutter/flutter#133493) 2023-08-28 [email protected] PlatformRouteInformationProvider should dispatch creation in constructor. (flutter/flutter#133492) 2023-08-28 [email protected] Roll Flutter Engine from 91522a188bda to a7a4c1c70bad (11 revisions) (flutter/flutter#133508) 2023-08-28 [email protected] Revert "Remove `ImageProvider.load`, `DecoderCallback` and `PaintingBâ�¦ (flutter/flutter#133482) 2023-08-28 [email protected] Fix context menu button color on Android when textButtonTheme is set (flutter/flutter#133271) 2023-08-28 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.21.4 to 2.21.5 (flutter/flutter#133504) 2023-08-28 [email protected] Fix `DatePickerDialog` & `DateRangePickerDialog` overflow when resized from landscape to portrait (flutter/flutter#133327) 2023-08-28 [email protected] Empty commit to re-trigger tests (flutter/flutter#133495) 2023-08-28 [email protected] Revert "PlatformRouteInformationProvider should dispatch creation in constructor." (flutter/flutter#133479) 2023-08-28 [email protected] Revert "FocusNode and FocusManager should dispatch creation in constructor." (flutter/flutter#133474) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

… is set (flutter/flutter#133271)

github-actions bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Aug 24, 2023

Renzo-Olivares changed the title ~~Fix Android Context Menu when textButtonTheme is set~~ Fix context menu color on Android when textButtonTheme is set Aug 24, 2023

Renzo-Olivares marked this pull request as ready for review August 24, 2023 20:37

Renzo-Olivares force-pushed the material-context-menu-fix branch from fed4fa2 to a4e0bcc Compare August 24, 2023 20:38

Renzo-Olivares requested a review from justinmc August 24, 2023 20:38

Renzo-Olivares changed the title ~~Fix context menu color on Android when textButtonTheme is set~~ Fix context menu button color on Android when textButtonTheme is set Aug 24, 2023

flutter-dashboard bot added the will affect goldens Changes to golden files label Aug 24, 2023

justinmc reviewed Aug 25, 2023

View reviewed changes

Renzo-Olivares added 3 commits August 28, 2023 13:45

material TextSelectionToolbarTextButton should set its backgroundColor

108c730

Add tests

537367d

use Colors.transparent so text button color is the same as its container

78d3b75

Renzo-Olivares force-pushed the material-context-menu-fix branch from 865a025 to 78d3b75 Compare August 28, 2023 20:45

Add comments

0dec936

Renzo-Olivares requested a review from justinmc August 28, 2023 20:48

justinmc approved these changes Aug 28, 2023

View reviewed changes

Renzo-Olivares added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels Aug 28, 2023

auto-submit bot merged commit ca33836 into flutter:master Aug 28, 2023

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 29, 2023

ca33836 Fix context menu button color on Android when textButtonTheme…

153ac2f

… is set (flutter/flutter#133271)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 29, 2023

ca33836 Fix context menu button color on Android when textButtonTheme…

7943bdf

… is set (flutter/flutter#133271)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 29, 2023

ca33836 Fix context menu button color on Android when textButtonTheme…

dc1dfd0

… is set (flutter/flutter#133271)

HansMuller mentioned this pull request Sep 1, 2023

Flutter Framework Team: Notable Commits #121415

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2023

ca33836 Fix context menu button color on Android when textButtonTheme…

3550e70

… is set (flutter/flutter#133271)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix context menu button color on Android when textButtonTheme is set #133271

Fix context menu button color on Android when textButtonTheme is set #133271

Uh oh!

Renzo-Olivares commented Aug 24, 2023 •

edited

Loading

Uh oh!

flutter-dashboard bot commented Aug 24, 2023

Uh oh!

justinmc left a comment

Uh oh!

justinmc Aug 25, 2023

Uh oh!

Renzo-Olivares Aug 28, 2023

Uh oh!

justinmc Aug 25, 2023

Uh oh!

Renzo-Olivares Aug 28, 2023

Uh oh!

justinmc left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix context menu button color on Android when textButtonTheme is set #133271

Fix context menu button color on Android when textButtonTheme is set #133271

Uh oh!

Conversation

Renzo-Olivares commented Aug 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Aug 24, 2023

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc Aug 25, 2023

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares Aug 28, 2023

Choose a reason for hiding this comment

Uh oh!

justinmc Aug 25, 2023

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares Aug 28, 2023

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Renzo-Olivares commented Aug 24, 2023 •

edited

Loading