Skip to content

TextField should support disabled input text style #119216

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

Jump to bottom
Merged
Renzo-Olivares merged 19 commits into from
Mar 17, 2023
Merged

TextField should support disabled input text style #119216

Renzo-Olivares merged 19 commits into from
Mar 17, 2023

Conversation

@Renzo-Olivares
Copy link
Contributor

@Renzo-Olivares Renzo-Olivares commented Jan 26, 2023

Fixes #118568

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 Jan 26, 2023
justinmc
justinmc approved these changes Jan 28, 2023
View reviewed changes
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 👍

packages/flutter/lib/src/material/text_field.dart Outdated
Copy link
Contributor

@justinmc justinmc Jan 28, 2023

Choose a reason for hiding this comment

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

Nit: Could this be private?

Copy link
Contributor Author

@Renzo-Olivares Renzo-Olivares Jan 30, 2023

Choose a reason for hiding this comment

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

Mostly did this because input_decorator does the same here

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

Line 2136 in 92df6b4

Set<MaterialState> get materialState {
. Not sure if that was intentional or not but I don't see why it can't be private.

@LongCatIsLooong
Copy link
Contributor

LongCatIsLooong commented Jan 30, 2023

I think in #118568 the suggestion was that style should support MaterialStateProperty?

@Renzo-Olivares
Copy link
Contributor Author

Renzo-Olivares commented Jan 30, 2023

I think in #118568 the suggestion was that style should support MaterialStateProperty?

You are right! Should be using MaterialStateProperty now.

justinmc
justinmc approved these changes Jan 30, 2023
View reviewed changes
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, thanks for the changes. I agree it's probably safer to make _materialState private for now. We can easily open it up if people ask for it for some reason.

@Renzo-Olivares
Copy link
Contributor Author

Renzo-Olivares commented Jan 31, 2023

CC @Hangyujin Wondering if you had any thoughts on this implementation since you migrated TextField to Material3.

hannah-hyj
hannah-hyj approved these changes Jan 31, 2023
View reviewed changes
Copy link
Member

@hannah-hyj hannah-hyj left a comment

Choose a reason for hiding this comment

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

LGTM

@Renzo-Olivares Renzo-Olivares added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 31, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 31, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Jan 31, 2023

auto label is removed for flutter/flutter, pr: 119216, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@Renzo-Olivares
Copy link
Contributor Author

Renzo-Olivares commented Jan 31, 2023 •
edited
Loading

Looks like failing tests are related to users implementing their own method to have a disabled input style in a text field. I'll need to go through the tests and see whats the best way forward.

@Renzo-Olivares Renzo-Olivares force-pushed the disabled-input branch 3 times, most recently from 3104fca to de113c3 Compare February 8, 2023 02:43
@Renzo-Olivares Renzo-Olivares force-pushed the disabled-input branch 4 times, most recently from fec0198 to 42cce4a Compare February 16, 2023 23:37
@Renzo-Olivares
Copy link
Contributor Author

Renzo-Olivares commented Feb 16, 2023

This one needs another review because a few changes have been made. Mostly refactoring and using tokens and generators. Thank you!

@Renzo-Olivares Renzo-Olivares force-pushed the disabled-input branch 2 times, most recently from b162fb3 to ae07f6a Compare February 17, 2023 19:14
@Renzo-Olivares Renzo-Olivares force-pushed the disabled-input branch 2 times, most recently from d1312fc to 458dbec Compare March 10, 2023 19:41
@Renzo-Olivares
Copy link
Contributor Author

Renzo-Olivares commented Mar 17, 2023

Merging this since all failures are expected.

@Renzo-Olivares Renzo-Olivares merged commit 358d79c into flutter:master Mar 17, 2023
@HansMuller HansMuller mentioned this pull request Mar 17, 2023
Closed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 18, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2023
@engine-flutter-autoroll
358d79c TextField should support disabled input text style (flutter/f…
5913bb5 
…lutter#119216)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
@engine-flutter-autoroll
358d79c TextField should support disabled input text style (flutter/f…
7fa4b35 
…lutter#119216)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
@engine-flutter-autoroll
358d79c TextField should support disabled input text style (flutter/f…
d423e9b 
…lutter#119216)
@slaci slaci mentioned this pull request Aug 9, 2023
Closed
2 tasks
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

@LongCatIsLooong LongCatIsLooong Awaiting requested review from LongCatIsLooong

@hannah-hyj hannah-hyj Awaiting requested review from hannah-hyj

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TextFields don't support disabled input text styling

4 participants

@Renzo-Olivares @LongCatIsLooong @justinmc @hannah-hyj