Skip to content

TextSelectionTheme support (step 2 of 3) - #65044

Merged
fluttergithubbot merged 5 commits into
flutter:masterfrom
darrenaustin:text_selection_theme_opt_in
Sep 17, 2020
Merged

TextSelectionTheme support (step 2 of 3)#65044
fluttergithubbot merged 5 commits into
flutter:masterfrom
darrenaustin:text_selection_theme_opt_in

Conversation

@darrenaustin

@darrenaustin darrenaustin commented Sep 1, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

This PR continues the migration to the new TextSelectionTheme started in #62014 and is described in the design doc.

This step changes the default for ThemeData.useTextSelectionTheme to true, thus opting into the text selection theme by default. This will mean that the default colors for the cursor color, text selection and selection handles are all now based on the primary color of the color scheme. The following parameters of the ThemeData will no longer be used and will soon be deprecated:

  • cursorColor
  • textSelectionColor
  • textSelectionHandleColor

Related Issues

#17635
#56082
#61227

Tests

I updated text selection theme tests and the editable text tests to match the new default colors.

Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

This step is not a breaking, but is part of a soft landing breaking change (see #62014 for more details).

  • No, no existing tests failed, so this is not a breaking change.

@flutter-dashboard flutter-dashboard Bot added p: material_ui material_ui package in flutter/packages framework flutter/packages/flutter repository. See also f: labels. work in progress; do not review labels Sep 1, 2020
@skia-gold

skia-gold commented Sep 1, 2020

Copy link
Copy Markdown

Gold has detected about 1 untriaged digest(s) on patchset 1.
View them at https://flutter-gold.skia.org/cl/github/65044

@darrenaustin darrenaustin changed the title WIP: TextSelectionTheme support (step 2 of 3) TextSelectionTheme support (step 2 of 3) Sep 8, 2020
@darrenaustin
darrenaustin marked this pull request as ready for review September 8, 2020 21:32
@skia-gold

skia-gold commented Sep 9, 2020

Copy link
Copy Markdown

Gold has detected about 2 untriaged digest(s) on patchset 2.
View them at https://flutter-gold.skia.org/cl/github/65044

@skia-gold

skia-gold commented Sep 9, 2020

Copy link
Copy Markdown

Gold has detected about 21 untriaged digest(s) on patchset 2.
View them at https://flutter-gold.skia.org/cl/github/65044

@skia-gold

skia-gold commented Sep 9, 2020

Copy link
Copy Markdown

Gold has detected about 26 untriaged digest(s) on patchset 3.
View them at https://flutter-gold.skia.org/cl/github/65044

@flutter-dashboard

flutter-dashboard Bot commented Sep 9, 2020

Copy link
Copy Markdown

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 #65044 at sha 2e31278e2bec24d58755212141cbdbda9df2bd30

@flutter-dashboard flutter-dashboard Bot added the will affect goldens Changes to golden files label Sep 9, 2020
HansMuller
HansMuller reviewed Sep 9, 2020
View reviewed changes
Comment thread packages/flutter/lib/src/material/theme_data.dart Outdated
justinmc
justinmc approved these changes Sep 9, 2020
View reviewed changes

@justinmc justinmc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

Comment thread packages/flutter/test/material/text_selection_theme_test.dart Outdated
HansMuller
HansMuller reviewed Sep 9, 2020
View reviewed changes
Comment thread packages/flutter/test/material/text_selection_theme_test.dart Outdated
Comment thread packages/flutter/test/material/text_selection_theme_test.dart Outdated
@darrenaustin darrenaustin mentioned this pull request Sep 16, 2020
Open
HansMuller
HansMuller approved these changes Sep 16, 2020
View reviewed changes

@HansMuller HansMuller left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@skia-gold

skia-gold commented Sep 16, 2020

Copy link
Copy Markdown

Gold has detected about 4 untriaged digest(s) on patchset 4.
View them at https://flutter-gold.skia.org/cl/github/65044

@fluttergithubbot
fluttergithubbot merged commit cb92ffc into flutter:master Sep 17, 2020
@darrenaustin
darrenaustin deleted the text_selection_theme_opt_in branch September 17, 2020 00:25
@jmagman jmagman mentioned this pull request Sep 17, 2020
Closed
jonahwilliams pushed a commit that referenced this pull request Sep 17, 2020
Jonah Williams
Revert "TextSelectionTheme support (step 2 of 3) (#65044)"
910838b 
This reverts commit cb92ffc.
@jonahwilliams jonahwilliams mentioned this pull request Sep 17, 2020
Merged
jonahwilliams pushed a commit that referenced this pull request Sep 17, 2020
Jonah Williams
Revert "TextSelectionTheme support (step 2 of 3) (#65044)" (#66051)
a3aeab0 
This reverts commit cb92ffc.
@jmagman

jmagman commented Sep 17, 2020

Copy link
Copy Markdown
Member

This was reverted due to a post-submit test failure. See #66046 for details.

@darrenaustin
darrenaustin restored the text_selection_theme_opt_in branch September 17, 2020 19:24
@darrenaustin darrenaustin mentioned this pull request Sep 17, 2020
Merged
darrenaustin added a commit that referenced this pull request Sep 17, 2020
@darrenaustin
Reland ##65044 (#66061)
1f62776 
Turn the opt-in default for ThemeData.useTextSelectionTheme to true so that everything uses the new TextSelectionTheme by default.
goderbauer pushed a commit to goderbauer/flutter that referenced this pull request Sep 18, 2020
Jonah Williams @goderbauer
Revert "TextSelectionTheme support (step 2 of 3) (flutter#65044)" (fl…
e00050e 
…utter#66051)

This reverts commit cb92ffc.
goderbauer pushed a commit to goderbauer/flutter that referenced this pull request Sep 18, 2020
@darrenaustin @goderbauer
Reland #flutter#65044 (flutter#66061)
1e2a627 
Turn the opt-in default for ThemeData.useTextSelectionTheme to true so that everything uses the new TextSelectionTheme by default.
This was referenced Sep 22, 2020
Merged
Merged
Merged
@darrenaustin
darrenaustin deleted the text_selection_theme_opt_in branch April 26, 2021 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@justinmc justinmc justinmc approved these changes
+1 more reviewer
@HansMuller HansMuller HansMuller approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages will affect goldens Changes to golden files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@darrenaustin @skia-gold @jmagman @justinmc @HansMuller @googlebot @fluttergithubbot