Skip to content

Correct intrinsics calculation for CupertinoTextField with placeholder #174889

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
auto-submit merged 3 commits into from
Sep 3, 2025
Merged

Correct intrinsics calculation for CupertinoTextField with placeholder #174889

auto-submit merged 3 commits into from
Sep 3, 2025

Conversation

@victorsanni
Copy link
Contributor

@victorsanni victorsanni commented Sep 3, 2025

@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 Sep 3, 2025
gemini-code-assist[bot]
gemini-code-assist bot reviewed Sep 3, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses a layout issue in CupertinoTextField where its intrinsic height was not correctly calculated when a placeholder was present. The fix involves implementing computeMinIntrinsicHeight and computeMaxIntrinsicHeight in the internal _RenderBaselineAlignedStack widget. A new widget test is also added to verify that the text field has a non-zero intrinsic height with a placeholder, preventing future regressions. The changes are correct and well-targeted.

packages/flutter/lib/src/cupertino/text_field.dart
Comment on lines +1783 to +1797
@override
double computeMinIntrinsicHeight(double width) {
return math.max(
_placeholderChild?.getMinIntrinsicHeight(width) ?? 0.0,
_editableTextChild.getMinIntrinsicHeight(width),
);
}

@override
double computeMaxIntrinsicHeight(double width) {
return math.max(
_placeholderChild?.getMaxIntrinsicHeight(width) ?? 0.0,
_editableTextChild.getMaxIntrinsicHeight(width),
);
}
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Sep 3, 2025

Choose a reason for hiding this comment

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

medium

According to the Flutter style guide, all public members should have documentation.1 These overridden methods would benefit from doc comments explaining their logic, which is to determine the intrinsic height based on the taller of the placeholder and the editable text.

  @override
  /// Computes the minimum intrinsic height of the stack, which is the larger of
  /// the minimum intrinsic heights of the placeholder and the editable text.
  double computeMinIntrinsicHeight(double width) {
    return math.max(
      _placeholderChild?.getMinIntrinsicHeight(width) ?? 0.0,
      _editableTextChild.getMinIntrinsicHeight(width),
    );
  }

  @override
  /// Computes the maximum intrinsic height of the stack, which is the larger of
  /// the maximum intrinsic heights of the placeholder and the editable text.
  double computeMaxIntrinsicHeight(double width) {
    return math.max(
      _placeholderChild?.getMaxIntrinsicHeight(width) ?? 0.0,
      _editableTextChild.getMaxIntrinsicHeight(width),
    );
  }

Style Guide References

Footnotes

  1. Flutter Style Guide, line 53 ↩

Copy link
Contributor Author

@victorsanni victorsanni Sep 3, 2025

Choose a reason for hiding this comment

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

Trivial implementation for override of a private API.

LongCatIsLooong
LongCatIsLooong reviewed Sep 3, 2025
View reviewed changes
Copy link
Contributor

@LongCatIsLooong LongCatIsLooong left a comment

Choose a reason for hiding this comment

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

The implementation lgtm. Can you add the other two as well?

@victorsanni victorsanni changed the title Correct intrinsic height calculation for CupertinoTextField with placeholder Correct intrinsics calculation for CupertinoTextField with placeholder Sep 3, 2025
@victorsanni victorsanni added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 3, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Sep 3, 2025
Merged via the queue into flutter:master with commit 241f6e6 Sep 3, 2025
77 of 78 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 3, 2025
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 4, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2025
@engine-flutter-autoroll
241f6e6 Correct intrinsics calculation for CupertinoTextField with pl…
0118f38 
…aceholder (flutter/flutter#174889)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 4, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2025
@engine-flutter-autoroll
241f6e6 Correct intrinsics calculation for CupertinoTextField with pl…
d835a25 
…aceholder (flutter/flutter#174889)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 4, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2025
@engine-flutter-autoroll
241f6e6 Correct intrinsics calculation for CupertinoTextField with pl…
1659a9e 
…aceholder (flutter/flutter#174889)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 4, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2025
@engine-flutter-autoroll
241f6e6 Correct intrinsics calculation for CupertinoTextField with pl…
18812e4 
…aceholder (flutter/flutter#174889)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 5, 2025
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2025
@engine-flutter-autoroll
241f6e6 Correct intrinsics calculation for CupertinoTextField with pl…
6b38aff 
…aceholder (flutter/flutter#174889)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 5, 2025
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2025
@engine-flutter-autoroll
241f6e6 Correct intrinsics calculation for CupertinoTextField with pl…
e5b47a9 
…aceholder (flutter/flutter#174889)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Sep 5, 2025
@engine-flutter-autoroll
Roll Flutter from 6b18740 to 87d5b75 (88 revisions) (#9962)
cab2ac2 
Roll Flutter from 6b18740 to 87d5b75 (88 revisions)

flutter/flutter@6b18740...87d5b75

2025-09-05 bkonyi@google.com [ Device Lab ] Add regression testing for flutter/flutter#174952 (flutter/flutter#174956)
2025-09-05 engine-flutter-autoroll@skia.org Roll Skia from 0ca53adfc6cc to 845ec125e94c (2 revisions) (flutter/flutter#174978)
2025-09-05 bruno.leroux@gmail.com [a11y-app] Fix NavigationRail leading and trailing labels (flutter/flutter#174861)
2025-09-05 engine-flutter-autoroll@skia.org Roll Skia from d7e99be07d5d to 0ca53adfc6cc (5 revisions) (flutter/flutter#174972)
2025-09-05 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from izfNA3o_2zL4Cjqmy... to xG_uERsxHvUwFHpF2... (flutter/flutter#174970)
2025-09-04 bruno.leroux@gmail.com Fix IconButton.color overrided by IconButtomTheme (flutter/flutter#174515)
2025-09-04 mdebbar@google.com [web] Reuse chrome instance to run all flutter tests (flutter/flutter#174957)
2025-09-04 pedromassango.developer@gmail.com fix(Semantics): Ensure semantics properties take priority over button's (flutter/flutter#174473)
2025-09-04 15619084+vashworth@users.noreply.github.com Make every LLDB Init error message actionable (flutter/flutter#174726)
2025-09-04 jhy03261997@gmail.com Fix table cell semantics rect alignment issues.  (flutter/flutter#174914)
2025-09-04 34465683+rkishan516@users.noreply.github.com Fix: Use route navigator for CupertinoSheetRoute pop (flutter/flutter#173103)
2025-09-04 bkonyi@google.com [ Widget Preview] Add `group` property to `Preview` (flutter/flutter#174849)
2025-09-04 47866232+chunhtai@users.noreply.github.com Allow OverlayPortal.overlayChildLayoutBuilder to choose root Overlay (flutter/flutter#174239)
2025-09-04 engine-flutter-autoroll@skia.org Roll Skia from 7c2f502e3304 to d7e99be07d5d (18 revisions) (flutter/flutter#174936)
2025-09-04 mdebbar@google.com Remove 'terms of use' wording from web_unicode (flutter/flutter#174939)
2025-09-04 bkonyi@google.com [ Tool ] Remove leftover Android x86 deprecation warning constant (flutter/flutter#174941)
2025-09-04 15619084+vashworth@users.noreply.github.com Prevent potential crash when accessing window in FlutterSceneDelegate (flutter/flutter#174873)
2025-09-04 engine-flutter-autoroll@skia.org Roll Packages from 42bb347 to 98580c6 (5 revisions) (flutter/flutter#174943)
2025-09-04 bkonyi@google.com [ Device Lab ] Fix wakefulness check to only match log entries with string values (flutter/flutter#174953)
2025-09-04 bruno.leroux@gmail.com Fix expanded DropdownMenu panel is shorter than text field (flutter/flutter#174443)
2025-09-04 31859944+LongCatIsLooong@users.noreply.github.com Add a `viewController` property to the ios/macOS `FlutterPluginRegistrar` protocol  (flutter/flutter#174168)
2025-09-03 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from J3T_wZqL_57mRfWky... to izfNA3o_2zL4Cjqmy... (flutter/flutter#174908)
2025-09-03 47866232+chunhtai@users.noreply.github.com Wires up Android API to set section locale (flutter/flutter#173364)
2025-09-03 chinmaygarde@google.com Delete impeller::SPrintF. (flutter/flutter#174900)
2025-09-03 ahmedsameha1@gmail.com Make sure that a DropdownMenuFormField doesn't crash in 0x0 environment (flutter/flutter#174777)
2025-09-03 mdebbar@google.com Remove unnecessary `presubmit_max_attempts` from .ci.yaml (flutter/flutter#174885)
2025-09-03 mdebbar@google.com Use local canvaskit in `dart_data_asset_test.dart` (flutter/flutter#174891)
2025-09-03 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Mark Linux web_canvaskit_tests_7_last as bringup (#174878)" (flutter/flutter#174897)
2025-09-03 victorsanniay@gmail.com Correct intrinsics calculation for CupertinoTextField with placeholder (flutter/flutter#174889)
2025-09-03 11901536+romaingyh@users.noreply.github.com Considers large title height in CupertinoNavigationBar's preferred size (flutter/flutter#173722)
2025-09-03 victorsanniay@gmail.com [A11y] Add semantics for CupertinoExpansionTile (flutter/flutter#174480)
2025-09-03 sokolovskyi.konstantin@gmail.com Fix LinearProgressIndicator track painting. (flutter/flutter#173108)
2025-09-03 matt.boetger@gmail.com update triage documentation to include team-android (flutter/flutter#174850)
2025-09-03 mdebbar@google.com Update `test_timeout_secs` to match `timeout` for `Linux web_skwasm_tests_*` and `Linux web_canvaskit_tests_*` (flutter/flutter#174881)
2025-09-03 engine-flutter-autoroll@skia.org Roll Packages from 5d785a0 to 42bb347 (10 revisions) (flutter/flutter#174876)
2025-09-03 chinmaygarde@google.com Fixup formatting of gn files in the old buildroot. (flutter/flutter#174852)
2025-09-03 iinozemtsev@google.com Roll Dart SDK to 3.10.0-162.1.beta (flutter/flutter#174834)
2025-09-03 mdebbar@google.com Mark Linux web_canvaskit_tests_7_last as bringup (flutter/flutter#174878)
2025-09-02 flar@google.com [Impeller] Fix overdraw in DrawRect geometry (flutter/flutter#174735)
2025-09-02 chinmaygarde@google.com Patch .clang-format files to specify C++20. (flutter/flutter#174848)
2025-09-02 mosum@google.com Add data assets (flutter/flutter#174685)
2025-09-02 mohellebiabdessalem@gmail.com refactors `FlutterPlugin.kt` to use one line statement in the `into` bloc (flutter/flutter#174759)
2025-09-02 47866232+chunhtai@users.noreply.github.com Ensures initial semantics state is sent to engine (flutter/flutter#174845)
2025-09-02 43054281+camsim99@users.noreply.github.com [Android] Break up plugin_test integration tests (flutter/flutter#174728)
2025-09-02 34465683+rkishan516@users.noreply.github.com Fix: Assertion failure in RawScrollbarState with dynamic controller assignment (flutter/flutter#173156)
2025-09-02 engine-flutter-autoroll@skia.org Roll Skia from 359f3d7cc7ed to 7c2f502e3304 (1 revision) (flutter/flutter#174844)
...
mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025
@victorsanni @mboetger
Correct intrinsics calculation for CupertinoTextField with placeholder (
c3169c3 
flutter#174889)

Adds `computeMaxIntrinsicHeight` and `computeMinIntrinsicHeight` to
`_RenderBaselineAlignedStack`. This should have been added in
[Baseline-align CupertinoTextField
placeholder](flutter#166952).

Fixes [CupertinoTextFormFieldRow with placeholder causes unnecessary
scrolling in
CupertinoAlertDialog](flutter#174797)
Fixes [CupertinoAlertDialog actions obstructs CupertinoTextField content
with placeholder text](flutter#174774)
Jaineel-Mamtora pushed a commit to Jaineel-Mamtora/flutter_forked that referenced this pull request Sep 24, 2025
@victorsanni @Jaineel-Mamtora
Correct intrinsics calculation for CupertinoTextField with placeholder (
7cb11e5 
flutter#174889)

Adds `computeMaxIntrinsicHeight` and `computeMinIntrinsicHeight` to
`_RenderBaselineAlignedStack`. This should have been added in
[Baseline-align CupertinoTextField
placeholder](flutter#166952).

Fixes [CupertinoTextFormFieldRow with placeholder causes unnecessary
scrolling in
CupertinoAlertDialog](flutter#174797)
Fixes [CupertinoAlertDialog actions obstructs CupertinoTextField content
with placeholder text](flutter#174774)
danferreira pushed a commit to danferreira/packages that referenced this pull request Oct 22, 2025
@engine-flutter-autoroll @danferreira
Roll Flutter from 6b18740 to 87d5b75 (88 revisions) (flutter#9962)
7646de2 
Roll Flutter from 6b18740 to 87d5b75 (88 revisions)

flutter/flutter@6b18740...87d5b75

2025-09-05 bkonyi@google.com [ Device Lab ] Add regression testing for flutter/flutter#174952 (flutter/flutter#174956)
2025-09-05 engine-flutter-autoroll@skia.org Roll Skia from 0ca53adfc6cc to 845ec125e94c (2 revisions) (flutter/flutter#174978)
2025-09-05 bruno.leroux@gmail.com [a11y-app] Fix NavigationRail leading and trailing labels (flutter/flutter#174861)
2025-09-05 engine-flutter-autoroll@skia.org Roll Skia from d7e99be07d5d to 0ca53adfc6cc (5 revisions) (flutter/flutter#174972)
2025-09-05 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from izfNA3o_2zL4Cjqmy... to xG_uERsxHvUwFHpF2... (flutter/flutter#174970)
2025-09-04 bruno.leroux@gmail.com Fix IconButton.color overrided by IconButtomTheme (flutter/flutter#174515)
2025-09-04 mdebbar@google.com [web] Reuse chrome instance to run all flutter tests (flutter/flutter#174957)
2025-09-04 pedromassango.developer@gmail.com fix(Semantics): Ensure semantics properties take priority over button's (flutter/flutter#174473)
2025-09-04 15619084+vashworth@users.noreply.github.com Make every LLDB Init error message actionable (flutter/flutter#174726)
2025-09-04 jhy03261997@gmail.com Fix table cell semantics rect alignment issues.  (flutter/flutter#174914)
2025-09-04 34465683+rkishan516@users.noreply.github.com Fix: Use route navigator for CupertinoSheetRoute pop (flutter/flutter#173103)
2025-09-04 bkonyi@google.com [ Widget Preview] Add `group` property to `Preview` (flutter/flutter#174849)
2025-09-04 47866232+chunhtai@users.noreply.github.com Allow OverlayPortal.overlayChildLayoutBuilder to choose root Overlay (flutter/flutter#174239)
2025-09-04 engine-flutter-autoroll@skia.org Roll Skia from 7c2f502e3304 to d7e99be07d5d (18 revisions) (flutter/flutter#174936)
2025-09-04 mdebbar@google.com Remove 'terms of use' wording from web_unicode (flutter/flutter#174939)
2025-09-04 bkonyi@google.com [ Tool ] Remove leftover Android x86 deprecation warning constant (flutter/flutter#174941)
2025-09-04 15619084+vashworth@users.noreply.github.com Prevent potential crash when accessing window in FlutterSceneDelegate (flutter/flutter#174873)
2025-09-04 engine-flutter-autoroll@skia.org Roll Packages from 42bb347 to 98580c6 (5 revisions) (flutter/flutter#174943)
2025-09-04 bkonyi@google.com [ Device Lab ] Fix wakefulness check to only match log entries with string values (flutter/flutter#174953)
2025-09-04 bruno.leroux@gmail.com Fix expanded DropdownMenu panel is shorter than text field (flutter/flutter#174443)
2025-09-04 31859944+LongCatIsLooong@users.noreply.github.com Add a `viewController` property to the ios/macOS `FlutterPluginRegistrar` protocol  (flutter/flutter#174168)
2025-09-03 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from J3T_wZqL_57mRfWky... to izfNA3o_2zL4Cjqmy... (flutter/flutter#174908)
2025-09-03 47866232+chunhtai@users.noreply.github.com Wires up Android API to set section locale (flutter/flutter#173364)
2025-09-03 chinmaygarde@google.com Delete impeller::SPrintF. (flutter/flutter#174900)
2025-09-03 ahmedsameha1@gmail.com Make sure that a DropdownMenuFormField doesn't crash in 0x0 environment (flutter/flutter#174777)
2025-09-03 mdebbar@google.com Remove unnecessary `presubmit_max_attempts` from .ci.yaml (flutter/flutter#174885)
2025-09-03 mdebbar@google.com Use local canvaskit in `dart_data_asset_test.dart` (flutter/flutter#174891)
2025-09-03 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Mark Linux web_canvaskit_tests_7_last as bringup (#174878)" (flutter/flutter#174897)
2025-09-03 victorsanniay@gmail.com Correct intrinsics calculation for CupertinoTextField with placeholder (flutter/flutter#174889)
2025-09-03 11901536+romaingyh@users.noreply.github.com Considers large title height in CupertinoNavigationBar's preferred size (flutter/flutter#173722)
2025-09-03 victorsanniay@gmail.com [A11y] Add semantics for CupertinoExpansionTile (flutter/flutter#174480)
2025-09-03 sokolovskyi.konstantin@gmail.com Fix LinearProgressIndicator track painting. (flutter/flutter#173108)
2025-09-03 matt.boetger@gmail.com update triage documentation to include team-android (flutter/flutter#174850)
2025-09-03 mdebbar@google.com Update `test_timeout_secs` to match `timeout` for `Linux web_skwasm_tests_*` and `Linux web_canvaskit_tests_*` (flutter/flutter#174881)
2025-09-03 engine-flutter-autoroll@skia.org Roll Packages from 5d785a0 to 42bb347 (10 revisions) (flutter/flutter#174876)
2025-09-03 chinmaygarde@google.com Fixup formatting of gn files in the old buildroot. (flutter/flutter#174852)
2025-09-03 iinozemtsev@google.com Roll Dart SDK to 3.10.0-162.1.beta (flutter/flutter#174834)
2025-09-03 mdebbar@google.com Mark Linux web_canvaskit_tests_7_last as bringup (flutter/flutter#174878)
2025-09-02 flar@google.com [Impeller] Fix overdraw in DrawRect geometry (flutter/flutter#174735)
2025-09-02 chinmaygarde@google.com Patch .clang-format files to specify C++20. (flutter/flutter#174848)
2025-09-02 mosum@google.com Add data assets (flutter/flutter#174685)
2025-09-02 mohellebiabdessalem@gmail.com refactors `FlutterPlugin.kt` to use one line statement in the `into` bloc (flutter/flutter#174759)
2025-09-02 47866232+chunhtai@users.noreply.github.com Ensures initial semantics state is sent to engine (flutter/flutter#174845)
2025-09-02 43054281+camsim99@users.noreply.github.com [Android] Break up plugin_test integration tests (flutter/flutter#174728)
2025-09-02 34465683+rkishan516@users.noreply.github.com Fix: Assertion failure in RawScrollbarState with dynamic controller assignment (flutter/flutter#173156)
2025-09-02 engine-flutter-autoroll@skia.org Roll Skia from 359f3d7cc7ed to 7c2f502e3304 (1 revision) (flutter/flutter#174844)
...
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025
@engine-flutter-autoroll
241f6e6 Correct intrinsics calculation for CupertinoTextField with pl…
bc6dca1 
…aceholder (flutter/flutter#174889)
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
@victorsanni @lucaantonelli
Correct intrinsics calculation for CupertinoTextField with placeholder (
02f3c02 
flutter#174889)

Adds `computeMaxIntrinsicHeight` and `computeMinIntrinsicHeight` to
`_RenderBaselineAlignedStack`. This should have been added in
[Baseline-align CupertinoTextField
placeholder](flutter#166952).

Fixes [CupertinoTextFormFieldRow with placeholder causes unnecessary
scrolling in
CupertinoAlertDialog](flutter#174797)
Fixes [CupertinoAlertDialog actions obstructs CupertinoTextField content
with placeholder text](flutter#174774)
@victorsanni victorsanni deleted the placeholder-fix branch January 12, 2026 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LongCatIsLooong LongCatIsLooong LongCatIsLooong approved these changes

@justinmc justinmc Awaiting requested review from justinmc

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

2 participants

@victorsanni @LongCatIsLooong