Skip to content

Conversation

@victorsanni
Copy link
Contributor

@victorsanni victorsanni commented Jun 5, 2024

#83531 contains complaints that this default disabled color can no longer be overridden by setting a color of the decoration property. This regression was introduced by #78058, which was itself made to fix an issue where the color of the decoration was always used even when disabled.

Currently, when the text field is disabled, its color is set to a default disabled color. This PR makes it so that if a decoration is set when the text field is disabled, its color is used instead of the default disabled color.

Uses the sentinel value _kDefaultRoundedBorderDecoration to set the disabled color only if the user does not pass in a decoration.

Fixes #83531

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@victorsanni victorsanni changed the title Make CupertinoTextField respect decoration color when disabled Make CupertinoTextField respect decoration color when disabled Jun 5, 2024
@github-actions 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: cupertino flutter/packages/flutter/cupertino repository labels Jun 5, 2024
final BoxDecoration? effectiveDecoration = (widget._textFieldType == _TextFieldType.bordered
? widget.decoration?.copyWith(
border: resolvedBorder,
color: enabled ? decorationColor
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should check if this color is a WidgetStateColor similar to how we've done elsewhere for MaterialStateColor in the past: https://api.flutter.dev/flutter/material/TimePickerThemeData/dayPeriodColor.html.

Then they could customize it for all states easier. It'd need to be called out in the documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I can find, CupertinoTextField only has a state for when it is enabled or disabled. I can't find information on any other states (hovering, pressed, focused, etc). So would this be relevant in this case?

It might be worthwhile to create another PR to introduce widget states to CupertinoTextField, at which point this might be more relevant.

Copy link
Contributor

@LongCatIsLooong LongCatIsLooong Jun 13, 2024

Choose a reason for hiding this comment

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

The cupertino library AFAIK uses CupertinoDynamicColor which is a similar system but with potentially different input (resolveWith parameters).

@victorsanni victorsanni marked this pull request as ready for review June 12, 2024 23:58
@victorsanni victorsanni requested a review from justinmc June 12, 2024 23:59
Copy link
Contributor

@MitchellGoodwin MitchellGoodwin left a comment

Choose a reason for hiding this comment

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

LGTM!

@victorsanni victorsanni added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 14, 2024
@auto-submit auto-submit bot merged commit 1c77696 into flutter:master Jun 14, 2024
@victorsanni victorsanni deleted the cupertino-text-field-disabled-color branch June 14, 2024 19:05
victorsanni added a commit to victorsanni/flutter that referenced this pull request Jun 14, 2024
…tter#149774)

flutter#83531 contains complaints that this default disabled color can no longer be overridden by setting a color of the `decoration` property. This regression was introduced by flutter#78058, which was itself made to fix an issue where the color of the decoration was always used even when disabled.

Currently, when the text field is disabled, its color is set to a default disabled color. This PR makes it so that if a decoration is set when the text field is disabled, its color is used instead of the default disabled color.

Fixes flutter#83531
victorsanni added a commit to victorsanni/flutter that referenced this pull request Jun 14, 2024
…tter#149774)

flutter#83531 contains complaints that this default disabled color can no longer be overridden by setting a color of the `decoration` property. This regression was introduced by flutter#78058, which was itself made to fix an issue where the color of the decoration was always used even when disabled.

Currently, when the text field is disabled, its color is set to a default disabled color. This PR makes it so that if a decoration is set when the text field is disabled, its color is used instead of the default disabled color.

Fixes flutter#83531
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 15, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jun 15, 2024
flutter/flutter@349ec71...5187cab

2024-06-15 [email protected] Roll Flutter Engine from 8167dffd1914 to 9779c273aac3 (21 revisions) (flutter/flutter#150290)
2024-06-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Fix flaky complex_layout_scroll_perf__memory test (#150287)" (flutter/flutter#150293)
2024-06-15 [email protected] Fix flaky complex_layout_scroll_perf__memory test (flutter/flutter#150287)
2024-06-14 [email protected] Make `CupertinoTextField` respect decoration color when disabled (flutter/flutter#149774)
2024-06-14 [email protected] Add transparent color to Cupertino colors (flutter/flutter#150149)
2024-06-14 [email protected] Roll pub packages (flutter/flutter#150267)
2024-06-14 [email protected] Roll Packages from 7805455 to dd04ab1 (4 revisions) (flutter/flutter#150264)

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],[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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024
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 autosubmit Merge PR when tree becomes green via auto submit App f: cupertino flutter/packages/flutter/cupertino repository framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CupertinoTextField overriding decoration when disabled.

3 participants