Skip to content

Conversation

@victorsanni
Copy link
Contributor

@victorsanni victorsanni commented Apr 10, 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 Apr 10, 2025
@victorsanni
Copy link
Contributor Author

@LongCatIsLooong just confirming this is the correct approach.

@victorsanni victorsanni marked this pull request as ready for review April 15, 2025 01:26
@victorsanni victorsanni marked this pull request as draft April 15, 2025 22:49
@victorsanni victorsanni marked this pull request as ready for review May 15, 2025 23:01
@victorsanni victorsanni changed the title Change CupertinoTextField placeholder alignment from center to topStart Baseline-align CupertinoTextField placeholder May 15, 2025
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.

Nice use of a RenderObjectWidget here, this looks good. I think you might be able to make this slightly cleaner user slots, see my comment below. Otherwise LGTM.

textDirection:
widget.textDirection ?? Directionality.maybeOf(context) ?? TextDirection.ltr,
child: _BaselineAlignedStack(
children: <Widget>[if (placeholder != null) placeholder, editableText],
Copy link
Contributor

Choose a reason for hiding this comment

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

If it is known that this widget will only ever take a maximum of two children (placeholder and editableText), then this can be implemented as "slots" rather than a list of children. See SlottedMultiChildRenderObjectWidget. Sorry for not pointing you in that direction in the first place if so!

Comment on lines 1731 to 1746
double? getDistanceToBaseline(TextBaseline baseline, {bool onlyReal = false}) {
final RenderBox? editableText = _editableTextChild;
if (editableText == null) {
return super.getDistanceToBaseline(baseline, onlyReal: onlyReal);
}
final double? editableTextBaseline = editableText.getDistanceToBaseline(
baseline,
onlyReal: onlyReal,
);
if (editableTextBaseline == null) {
return super.getDistanceToBaseline(baseline, onlyReal: onlyReal);
}
final _BaselineAlignedStackParentData editableTextParentData =
editableText.parentData! as _BaselineAlignedStackParentData;
return editableTextParentData.offset.dy + editableTextBaseline;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it the case that the placeholder must be smaller than the editableText? Maybe leave a comment about this somewhere if it makes sense.

I was thinking about the case where the placeholder is much larger than the editableText, so that aligning the placeholder to the editableText's baseline makes the placeholder overflow out the top. But I'm assuming that's not possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that was also confusing for me. I saw 3 options:

  1. EditableText baseline
  2. Placeholder baseline
  3. The max of 1) and 2)

The current implementation is 1), but I honestly do not know which to go with, I need further guidance here. I can document this in the docs for CupertinoTextField.placeholder and/or CupertinoTextField.placeholderStyle.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok I say keep it 1 actually. Looking at your screenshots up in the PR description, I feel like the EditableText baseline is supposed to be "the" baseline for the field. If someone makes the placeholder so big that it overflows, then that's their own fault.

We can always reconsider if someone opens an issue with a good argument.

@victorsanni victorsanni requested a review from justinmc May 16, 2025 20:46
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 👍

Looks good with slots, thanks for changing that.

@victorsanni victorsanni added the autosubmit Merge PR when tree becomes green via auto submit App label May 24, 2025
@auto-submit auto-submit bot added this pull request to the merge queue May 24, 2025
Merged via the queue into flutter:master with commit 3df4a3c May 24, 2025
76 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 24, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 24, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request May 24, 2025
flutter/flutter@85564cb...60050a0

2025-05-24 [email protected] Roll Skia from 0834eea9de33 to 91dc88dc70e5 (1 revision) (flutter/flutter#169414)
2025-05-24 [email protected] Roll Dart SDK from 0a6b16a55b9e to 7dab9bffe1f7 (1 revision) (flutter/flutter#169406)
2025-05-24 [email protected] Correct calculation for CupertinoTextSelectionToolbar vertical position (flutter/flutter#169308)
2025-05-24 [email protected] Baseline-align CupertinoTextField placeholder (flutter/flutter#166952)
2025-05-24 [email protected] Roll Dart SDK from 8354914ef97b to 0a6b16a55b9e (3 revisions) (flutter/flutter#169403)
2025-05-24 [email protected] Start removing Observatory support and references (flutter/flutter#169216)
2025-05-23 [email protected] Roll Skia from f42bb59753fe to 0834eea9de33 (3 revisions) (flutter/flutter#169393)
2025-05-23 [email protected] Roll Skia from 956fd8b14e22 to f42bb59753fe (2 revisions) (flutter/flutter#169379)
2025-05-23 [email protected] [Engine] Fast blurring algorithm for RSuperellipse (flutter/flutter#169187)
2025-05-23 [email protected] Add a page describing best CI practices for `flutter`-org repos (flutter/flutter#169364)
2025-05-23 [email protected] Revert "Mark web_tool_tests_1_2 as bringup." (flutter/flutter#169361)
2025-05-23 [email protected] Add changelog section for 3.32.0 and 3.32.1, and add note for ndk checking cherry pick (flutter/flutter#169369)
2025-05-23 [email protected] Roll Dart SDK from 085f110ecf33 to 8354914ef97b (1 revision) (flutter/flutter#169349)
2025-05-23 [email protected] Remove handling of the legacy `.flutter-plugins` file in `PluginHandler.kt`. (flutter/flutter#169317)
2025-05-23 [email protected] Use baseUri of WebAssetServer for reload_scripts.json (flutter/flutter#169267)
2025-05-23 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Use pub workspace (#168662)" (flutter/flutter#169357)
2025-05-23 [email protected] Use pub workspace (flutter/flutter#168662)

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://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
stan-at-work pushed a commit to stan-at-work/flutter that referenced this pull request May 26, 2025
| Before | After | 
| --- | --- |
| <img width="348" alt="non baseline aligned placeholder"
src="https://github.com/user-attachments/assets/62f779cc-1d16-41f3-92f8-a2de16a04895"
/> | <img width="348" alt="baseline aligned placeholder"
src="https://github.com/user-attachments/assets/e43d200f-ef47-4d10-a74a-a2c6998b44f5"
/> |


Fixes [Can't align placeholder of CupertinoTextField with min lines to
top](flutter#138794)
github-merge-queue bot pushed a commit that referenced this pull request Jun 3, 2025
… fidelity updates (#169708)

This PR does the following:

## Show large title mid-transition if automaticallyImplyLeading is false

(see
#169708 (comment))

## Correct color for the CupertinoSearchTextField placeholder 

(see
#163020 (comment))

| Dark mode | Light mode | 
| --- | --- |
| <img width="381" alt="placeholder color dark mode"
src="https://github.com/user-attachments/assets/e37e23fa-9f4f-495e-8f02-b9c38a4faffb"
/> | <img width="381" alt="placeholder color light mode"
src="https://github.com/user-attachments/assets/16e24a61-2528-44e0-9afa-8431487cf5ff"
/> |

And also:

- Removes flaky mid-transition goldens
- Fixes a CupertinoTextField crash caused by
#166952 where size > constraints

Fixes [Improve fidelity of CupertinoSliverNavigationBar.search and
CupertinoSearchTextField](#163020)
zeqinjie pushed a commit to zeqinjie/flutter that referenced this pull request Jun 5, 2025
* master: (125 commits)
  Roll Fuchsia Linux SDK from XtPp9bBW49iDJ0wbA... to -eo2JqnJBauuGSzoW... (flutter#169424)
  Roll Skia from 91dc88dc70e5 to 443f5257f382 (1 revision) (flutter#169422)
  Roll Skia from 0834eea9de33 to 91dc88dc70e5 (1 revision) (flutter#169414)
  Roll Dart SDK from 0a6b16a55b9e to 7dab9bffe1f7 (1 revision) (flutter#169406)
  Correct calculation for CupertinoTextSelectionToolbar vertical position (flutter#169308)
  Baseline-align CupertinoTextField placeholder (flutter#166952)
  Roll Dart SDK from 8354914ef97b to 0a6b16a55b9e (3 revisions) (flutter#169403)
  Start removing Observatory support and references (flutter#169216)
  Roll Skia from f42bb59753fe to 0834eea9de33 (3 revisions) (flutter#169393)
  Roll Skia from 956fd8b14e22 to f42bb59753fe (2 revisions) (flutter#169379)
  [Engine] Fast blurring algorithm for RSuperellipse (flutter#169187)
  Add a page describing best CI practices for `flutter`-org repos (flutter#169364)
  Revert "Mark web_tool_tests_1_2 as bringup." (flutter#169361)
  Add changelog section for 3.32.0 and 3.32.1, and add note for ndk checking cherry pick (flutter#169369)
  Roll Dart SDK from 085f110ecf33 to 8354914ef97b (1 revision) (flutter#169349)
  Remove handling of the legacy `.flutter-plugins` file in `PluginHandler.kt`. (flutter#169317)
  Use baseUri of WebAssetServer for reload_scripts.json (flutter#169267)
  Reverts "Use pub workspace (flutter#168662)" (flutter#169357)
  Use pub workspace (flutter#168662)
  Reverts "[Reland2] Implements UISceneDelegate dynamically w/ FlutterLaunchEngine (flutter#169276)" (flutter#169347)
  ...
FMorschel pushed a commit to FMorschel/packages that referenced this pull request Jun 9, 2025
…r#9318)

flutter/flutter@85564cb...60050a0

2025-05-24 [email protected] Roll Skia from 0834eea9de33 to 91dc88dc70e5 (1 revision) (flutter/flutter#169414)
2025-05-24 [email protected] Roll Dart SDK from 0a6b16a55b9e to 7dab9bffe1f7 (1 revision) (flutter/flutter#169406)
2025-05-24 [email protected] Correct calculation for CupertinoTextSelectionToolbar vertical position (flutter/flutter#169308)
2025-05-24 [email protected] Baseline-align CupertinoTextField placeholder (flutter/flutter#166952)
2025-05-24 [email protected] Roll Dart SDK from 8354914ef97b to 0a6b16a55b9e (3 revisions) (flutter/flutter#169403)
2025-05-24 [email protected] Start removing Observatory support and references (flutter/flutter#169216)
2025-05-23 [email protected] Roll Skia from f42bb59753fe to 0834eea9de33 (3 revisions) (flutter/flutter#169393)
2025-05-23 [email protected] Roll Skia from 956fd8b14e22 to f42bb59753fe (2 revisions) (flutter/flutter#169379)
2025-05-23 [email protected] [Engine] Fast blurring algorithm for RSuperellipse (flutter/flutter#169187)
2025-05-23 [email protected] Add a page describing best CI practices for `flutter`-org repos (flutter/flutter#169364)
2025-05-23 [email protected] Revert "Mark web_tool_tests_1_2 as bringup." (flutter/flutter#169361)
2025-05-23 [email protected] Add changelog section for 3.32.0 and 3.32.1, and add note for ndk checking cherry pick (flutter/flutter#169369)
2025-05-23 [email protected] Roll Dart SDK from 085f110ecf33 to 8354914ef97b (1 revision) (flutter/flutter#169349)
2025-05-23 [email protected] Remove handling of the legacy `.flutter-plugins` file in `PluginHandler.kt`. (flutter/flutter#169317)
2025-05-23 [email protected] Use baseUri of WebAssetServer for reload_scripts.json (flutter/flutter#169267)
2025-05-23 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Use pub workspace (#168662)" (flutter/flutter#169357)
2025-05-23 [email protected] Use pub workspace (flutter/flutter#168662)

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://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
Ortes pushed a commit to Ortes/packages that referenced this pull request Jun 25, 2025
…r#9318)

flutter/flutter@85564cb...60050a0

2025-05-24 [email protected] Roll Skia from 0834eea9de33 to 91dc88dc70e5 (1 revision) (flutter/flutter#169414)
2025-05-24 [email protected] Roll Dart SDK from 0a6b16a55b9e to 7dab9bffe1f7 (1 revision) (flutter/flutter#169406)
2025-05-24 [email protected] Correct calculation for CupertinoTextSelectionToolbar vertical position (flutter/flutter#169308)
2025-05-24 [email protected] Baseline-align CupertinoTextField placeholder (flutter/flutter#166952)
2025-05-24 [email protected] Roll Dart SDK from 8354914ef97b to 0a6b16a55b9e (3 revisions) (flutter/flutter#169403)
2025-05-24 [email protected] Start removing Observatory support and references (flutter/flutter#169216)
2025-05-23 [email protected] Roll Skia from f42bb59753fe to 0834eea9de33 (3 revisions) (flutter/flutter#169393)
2025-05-23 [email protected] Roll Skia from 956fd8b14e22 to f42bb59753fe (2 revisions) (flutter/flutter#169379)
2025-05-23 [email protected] [Engine] Fast blurring algorithm for RSuperellipse (flutter/flutter#169187)
2025-05-23 [email protected] Add a page describing best CI practices for `flutter`-org repos (flutter/flutter#169364)
2025-05-23 [email protected] Revert "Mark web_tool_tests_1_2 as bringup." (flutter/flutter#169361)
2025-05-23 [email protected] Add changelog section for 3.32.0 and 3.32.1, and add note for ndk checking cherry pick (flutter/flutter#169369)
2025-05-23 [email protected] Roll Dart SDK from 085f110ecf33 to 8354914ef97b (1 revision) (flutter/flutter#169349)
2025-05-23 [email protected] Remove handling of the legacy `.flutter-plugins` file in `PluginHandler.kt`. (flutter/flutter#169317)
2025-05-23 [email protected] Use baseUri of WebAssetServer for reload_scripts.json (flutter/flutter#169267)
2025-05-23 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Use pub workspace (#168662)" (flutter/flutter#169357)
2025-05-23 [email protected] Use pub workspace (flutter/flutter#168662)

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://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
@victorsanni victorsanni deleted the align-placeholder branch July 1, 2025 20:02
@WolfMobileDev
Copy link

In which tag should the submission be merged?

justinmc added a commit to justinmc/flutter that referenced this pull request Jul 18, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2025
github-merge-queue bot pushed a commit that referenced this pull request Sep 3, 2025
#174889)

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

Fixes [CupertinoTextFormFieldRow with placeholder causes unnecessary
scrolling in
CupertinoAlertDialog](#174797)
Fixes [CupertinoAlertDialog actions obstructs CupertinoTextField content
with placeholder text](#174774)
mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025
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
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)
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
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)
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 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.

[Cupertino]: Can't align placeholder of CupertinoTextField with min lines to top

4 participants