Correct calculation for CupertinoTextSelectionToolbar vertical position #169308

victorsanni · 2025-05-22T20:48:32Z

Size CupertinoTextSelectionToolbar to children made a slight mistake when calculating if the toolbar should be above or below the text field.

Doubling the toolbar arrow height in the _isAbove calculation added a buffer of its height (7.0) within which the toolbar would be positioned below the text but have its arrow still pointing downwards. This is why I was only able to reproduce the bug within a 7.0 height range.

Before	After

Sample code

import 'package:flutter/cupertino.dart';

void main() => runApp(const TextSelectionToolbarApp());

class TextSelectionToolbarApp extends StatelessWidget {
  const TextSelectionToolbarApp({super.key});

  @override
  Widget build(BuildContext context) {
    return CupertinoApp(
      home: const CupertinoPageScaffold(
        child: SafeArea(
          child: ColoredBox(
            color: Color(0x11ff0000),
            child: Padding(
              padding: EdgeInsets.symmetric(vertical: 56.0, horizontal: 8.0),
              child: Column(
                children: [
                  CupertinoTextField(),
                ],
              ),
            ),
          ),
        ),
      ),
    );
  }
}

Fixes Cupertino Text Selection Toolbar has wrong position

flutter-dashboard · 2025-05-22T20:48:36Z

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

justinmc

LGTM 👍

justinmc · 2025-05-22T23:22:37Z

packages/flutter/test/cupertino/text_selection_toolbar_test.dart

+          pathMatcher: isPathThat(
+            excludes: <Offset>[const Offset(18.0, 0.0), const Offset(25.0, 7.0)],
+          ),


So that's how you tested which direction the arrow is pointing 👍

justinmc · 2025-05-22T23:23:34Z

packages/flutter/lib/src/cupertino/text_selection_toolbar.dart


-  bool _isAbove(double childHeight) =>
-      anchorAbove.dy >= childHeight - _kToolbarArrowSize.height * 2;
+  bool _isAbove(double childHeight) => anchorAbove.dy >= childHeight - _kToolbarArrowSize.height;


I'm not sure why this was * 2 in the first place.

…cal position (flutter/flutter#169308)

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

@OverRide

…on (flutter#169308) [Size CupertinoTextSelectionToolbar to children](flutter#133386) made a slight mistake when calculating if the toolbar should be above or below the text field. Doubling the toolbar arrow height in the `_isAbove `calculation added a buffer of its height (7.0) within which the toolbar would be positioned below the text but have its arrow still pointing downwards. This is why [I was only able to reproduce the bug within a 7.0 height range](flutter#154812 (comment)). | Before | After | | --- | --- | | <img width="377" alt="toolbar pos before" src="https://github.com/user-attachments/assets/11f63cf3-f352-4232-8230-f04da89b0b4c" /> | <img width="377" alt="toolbar pos after" src="https://github.com/user-attachments/assets/4a48c3bd-158e-468e-9c67-af125c7856a7" /> | <details> <summary>Sample code</summary> ```dart import 'package:flutter/cupertino.dart'; void main() => runApp(const TextSelectionToolbarApp()); class TextSelectionToolbarApp extends StatelessWidget { const TextSelectionToolbarApp({super.key}); @OverRide Widget build(BuildContext context) { return CupertinoApp( home: const CupertinoPageScaffold( child: SafeArea( child: ColoredBox( color: Color(0x11ff0000), child: Padding( padding: EdgeInsets.symmetric(vertical: 56.0, horizontal: 8.0), child: Column( children: [ CupertinoTextField(), ], ), ), ), ), ), ); } } ``` </details> Fixes [Cupertino Text Selection Toolbar has wrong position](flutter#154812)

* 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) ...

…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

…cal position (flutter/flutter#169308)

Correct calculation for isAbove

010297a

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 May 22, 2025

victorsanni added 3 commits May 22, 2025 14:36

Merge branch 'master' into text-selection-toolbar-arrow

9c73b8b

Add test

599579d

Merge branch 'master' into text-selection-toolbar-arrow

cf6b319

victorsanni requested review from LongCatIsLooong and justinmc May 22, 2025 23:09

justinmc approved these changes May 22, 2025

View reviewed changes

victorsanni added 3 commits May 23, 2025 12:01

Tweak test

6af0893

Merge branch 'master' into text-selection-toolbar-arrow

d2e97cc

Both includes and excludes

dc758af

victorsanni added the autosubmit Merge PR when tree becomes green via auto submit App label May 24, 2025

auto-submit bot added this pull request to the merge queue May 24, 2025

Merged via the queue into flutter:master with commit 16e5318 May 24, 2025
76 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 24, 2025

engine-flutter-autoroll mentioned this pull request May 24, 2025

Roll Flutter from 85564cbba9e7 to 60050a0b4d56 (17 revisions) flutter/packages#9318

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 24, 2025

16e5318e2 Correct calculation for CupertinoTextSelectionToolbar verti…

4b0dfff

…cal position (flutter/flutter#169308)

victorsanni deleted the text-selection-toolbar-arrow branch July 1, 2025 20:00

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025

16e5318e2 Correct calculation for CupertinoTextSelectionToolbar verti…

467aee6

…cal position (flutter/flutter#169308)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025

16e5318e2 Correct calculation for CupertinoTextSelectionToolbar verti…

ff12350

…cal position (flutter/flutter#169308)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025

16e5318e2 Correct calculation for CupertinoTextSelectionToolbar verti…

81f25cd

…cal position (flutter/flutter#169308)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025

16e5318e2 Correct calculation for CupertinoTextSelectionToolbar verti…

bb1fed6

…cal position (flutter/flutter#169308)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2025

16e5318e2 Correct calculation for CupertinoTextSelectionToolbar verti…

f244758

…cal position (flutter/flutter#169308)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Correct calculation for CupertinoTextSelectionToolbar vertical position #169308

Correct calculation for CupertinoTextSelectionToolbar vertical position #169308

Uh oh!

victorsanni commented May 22, 2025

Uh oh!

flutter-dashboard bot commented May 22, 2025

Uh oh!

justinmc left a comment

Uh oh!

justinmc May 22, 2025

Uh oh!

justinmc May 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Correct calculation for CupertinoTextSelectionToolbar vertical position #169308

Correct calculation for CupertinoTextSelectionToolbar vertical position #169308

Uh oh!

Conversation

victorsanni commented May 22, 2025

Uh oh!

flutter-dashboard bot commented May 22, 2025

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc May 22, 2025

Choose a reason for hiding this comment

Uh oh!

justinmc May 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants