Skip to content

Commit 00b0d55

Browse files
justinmcjustinmc
authored
Fix iOS context menu position when flipped below (#119565)
* Fix anchorBelow calculation, and share toolbar padding constant * Fix constant references in test * Test below position when padding is not offset by content distance
1 parent 96c8c69 commit 00b0d55

File tree

5 files changed

+58
-41
lines changed

5 files changed

+58
-41
lines changed

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

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,6 @@ const double _kToolbarHeight = 43.0;
1919
// Vertical distance between the tip of the arrow and the line of text the arrow
2020
// is pointing to. The value used here is eyeballed.
2121
const double _kToolbarContentDistance = 8.0;
22-
// Minimal padding from all edges of the selection toolbar to all edges of the
23-
// screen.
24-
const double _kToolbarScreenPadding = 8.0;
2522
const Size _kToolbarArrowSize = Size(14.0, 7.0);
2623

2724
// Minimal padding from tip of the selection toolbar arrow to horizontal edges of the
@@ -123,6 +120,16 @@ class CupertinoTextSelectionToolbar extends StatelessWidget {
123120
/// default Cupertino toolbar.
124121
final CupertinoToolbarBuilder toolbarBuilder;
125122

123+
/// Minimal padding from all edges of the selection toolbar to all edges of the
124+
/// viewport.
125+
///
126+
/// See also:
127+
///
128+
/// * [SpellCheckSuggestionsToolbar], which uses this same value for its
129+
/// padding from the edges of the viewport.
130+
/// * [TextSelectionToolbar], which uses this same value as well.
131+
static const double kToolbarScreenPadding = 8.0;
132+
126133
// Add the visual vertical line spacer between children buttons.
127134
static List<Widget> _addChildrenSpacers(List<Widget> children) {
128135
final List<Widget> nextChildren = <Widget>[];
@@ -163,7 +170,7 @@ class CupertinoTextSelectionToolbar extends StatelessWidget {
163170
assert(debugCheckHasMediaQuery(context));
164171
final EdgeInsets mediaQueryPadding = MediaQuery.paddingOf(context);
165172

166-
final double paddingAbove = mediaQueryPadding.top + _kToolbarScreenPadding;
173+
final double paddingAbove = mediaQueryPadding.top + kToolbarScreenPadding;
167174
final double toolbarHeightNeeded = paddingAbove
168175
+ _kToolbarContentDistance
169176
+ _kToolbarHeight;
@@ -180,15 +187,15 @@ class CupertinoTextSelectionToolbar extends StatelessWidget {
180187
);
181188
final Offset anchorBelowAdjusted = Offset(
182189
clampDouble(anchorBelow.dx, leftMargin, rightMargin),
183-
anchorBelow.dy - _kToolbarContentDistance + paddingAbove,
190+
anchorBelow.dy + _kToolbarContentDistance - paddingAbove,
184191
);
185192

186193
return Padding(
187194
padding: EdgeInsets.fromLTRB(
188-
_kToolbarScreenPadding,
195+
kToolbarScreenPadding,
189196
paddingAbove,
190-
_kToolbarScreenPadding,
191-
_kToolbarScreenPadding,
197+
kToolbarScreenPadding,
198+
kToolbarScreenPadding,
192199
),
193200
child: CustomSingleChildLayout(
194201
delegate: TextSelectionToolbarLayoutDelegate(

‎packages/flutter/lib/src/material/spell_check_suggestions_toolbar.dart

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import 'package:flutter/cupertino.dart';
56
import 'package:flutter/services.dart' show SuggestionSpan;
6-
import 'package:flutter/widgets.dart';
77

88
import 'adaptive_text_selection_toolbar.dart';
99
import 'colors.dart';
@@ -142,16 +142,16 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
142142
anchor + const Offset(0.0, kToolbarContentDistanceBelow);
143143
final MediaQueryData mediaQueryData = MediaQuery.of(context);
144144
final double softKeyboardViewInsetsBottom = mediaQueryData.viewInsets.bottom;
145-
final double paddingAbove = mediaQueryData.padding.top + TextSelectionToolbar.kToolbarScreenPadding;
145+
final double paddingAbove = mediaQueryData.padding.top + CupertinoTextSelectionToolbar.kToolbarScreenPadding;
146146
// Makes up for the Padding.
147-
final Offset localAdjustment = Offset(TextSelectionToolbar.kToolbarScreenPadding, paddingAbove);
147+
final Offset localAdjustment = Offset(CupertinoTextSelectionToolbar.kToolbarScreenPadding, paddingAbove);
148148

149149
return Padding(
150150
padding: EdgeInsets.fromLTRB(
151-
TextSelectionToolbar.kToolbarScreenPadding,
151+
CupertinoTextSelectionToolbar.kToolbarScreenPadding,
152152
kToolbarContentDistanceBelow,
153-
TextSelectionToolbar.kToolbarScreenPadding,
154-
TextSelectionToolbar.kToolbarScreenPadding + softKeyboardViewInsetsBottom,
153+
CupertinoTextSelectionToolbar.kToolbarScreenPadding,
154+
CupertinoTextSelectionToolbar.kToolbarScreenPadding + softKeyboardViewInsetsBottom,
155155
),
156156
child: CustomSingleChildLayout(
157157
delegate: SpellCheckSuggestionsToolbarLayoutDelegate(

‎packages/flutter/lib/src/material/text_selection_toolbar.dart

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44

55
import 'dart:math' as math;
66

7+
import 'package:flutter/cupertino.dart';
78
import 'package:flutter/foundation.dart' show listEquals;
89
import 'package:flutter/rendering.dart';
9-
import 'package:flutter/widgets.dart';
1010

1111
import 'debug.dart';
1212
import 'icon_button.dart';
@@ -76,15 +76,6 @@ class TextSelectionToolbar extends StatelessWidget {
7676
/// {@endtemplate}
7777
final ToolbarBuilder toolbarBuilder;
7878

79-
/// Minimal padding from all edges of the selection toolbar to all edges of the
80-
/// viewport.
81-
///
82-
/// See also:
83-
///
84-
/// * [SpellCheckSuggestionsToolbar], which uses this same value for its
85-
/// padding from the edges of the viewport.
86-
static const double kToolbarScreenPadding = 8.0;
87-
8879
/// The size of the text selection handles.
8980
///
9081
/// See also:
@@ -111,19 +102,20 @@ class TextSelectionToolbar extends StatelessWidget {
111102
final Offset anchorBelowPadded =
112103
anchorBelow + const Offset(0.0, kToolbarContentDistanceBelow);
113104

105+
const double screenPadding = CupertinoTextSelectionToolbar.kToolbarScreenPadding;
114106
final double paddingAbove = MediaQuery.paddingOf(context).top
115-
+ kToolbarScreenPadding;
107+
+ screenPadding;
116108
final double availableHeight = anchorAbovePadded.dy - _kToolbarContentDistance - paddingAbove;
117109
final bool fitsAbove = _kToolbarHeight <= availableHeight;
118110
// Makes up for the Padding above the Stack.
119-
final Offset localAdjustment = Offset(kToolbarScreenPadding, paddingAbove);
111+
final Offset localAdjustment = Offset(screenPadding, paddingAbove);
120112

121113
return Padding(
122114
padding: EdgeInsets.fromLTRB(
123-
kToolbarScreenPadding,
115+
screenPadding,
124116
paddingAbove,
125-
kToolbarScreenPadding,
126-
kToolbarScreenPadding,
117+
screenPadding,
118+
screenPadding,
127119
),
128120
child: CustomSingleChildLayout(
129121
delegate: TextSelectionToolbarLayoutDelegate(

‎packages/flutter/test/cupertino/text_selection_toolbar_test.dart

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -218,21 +218,34 @@ void main() {
218218
const double height = _kToolbarHeight;
219219
const double anchorBelowY = 500.0;
220220
double anchorAboveY = 0.0;
221+
const double paddingAbove = 12.0;
221222

222223
await tester.pumpWidget(
223224
CupertinoApp(
224225
home: Center(
225226
child: StatefulBuilder(
226227
builder: (BuildContext context, StateSetter setter) {
227228
setState = setter;
228-
return CupertinoTextSelectionToolbar(
229-
anchorAbove: Offset(50.0, anchorAboveY),
230-
anchorBelow: const Offset(50.0, anchorBelowY),
231-
children: <Widget>[
232-
Container(color: const Color(0xffff0000), width: 50.0, height: height),
233-
Container(color: const Color(0xff00ff00), width: 50.0, height: height),
234-
Container(color: const Color(0xff0000ff), width: 50.0, height: height),
235-
],
229+
final MediaQueryData data = MediaQuery.of(context);
230+
// Add some custom vertical padding to make this test more strict.
231+
// By default in the testing environment, _kToolbarContentDistance
232+
// and the built-in padding from CupertinoApp can end up canceling
233+
// each other out.
234+
return MediaQuery(
235+
data: data.copyWith(
236+
padding: data.viewPadding.copyWith(
237+
top: paddingAbove,
238+
),
239+
),
240+
child: CupertinoTextSelectionToolbar(
241+
anchorAbove: Offset(50.0, anchorAboveY),
242+
anchorBelow: const Offset(50.0, anchorBelowY),
243+
children: <Widget>[
244+
Container(color: const Color(0xffff0000), width: 50.0, height: height),
245+
Container(color: const Color(0xff00ff00), width: 50.0, height: height),
246+
Container(color: const Color(0xff0000ff), width: 50.0, height: height),
247+
],
248+
),
236249
);
237250
},
238251
),
@@ -244,18 +257,22 @@ void main() {
244257
// belowAnchor.
245258
double toolbarY = tester.getTopLeft(findToolbar()).dy;
246259
expect(toolbarY, equals(anchorBelowY + _kToolbarContentDistance));
260+
expect(find.byType(CustomSingleChildLayout), findsOneWidget);
261+
final CustomSingleChildLayout layout = tester.widget(find.byType(CustomSingleChildLayout));
262+
final TextSelectionToolbarLayoutDelegate delegate = layout.delegate as TextSelectionToolbarLayoutDelegate;
263+
expect(delegate.anchorBelow.dy, anchorBelowY - paddingAbove);
247264

248265
// Even when it barely doesn't fit.
249266
setState(() {
250-
anchorAboveY = 50.0;
267+
anchorAboveY = 70.0;
251268
});
252269
await tester.pump();
253270
toolbarY = tester.getTopLeft(findToolbar()).dy;
254271
expect(toolbarY, equals(anchorBelowY + _kToolbarContentDistance));
255272

256273
// When it does fit above aboveAnchor, it positions itself there.
257274
setState(() {
258-
anchorAboveY = 60.0;
275+
anchorAboveY = 80.0;
259276
});
260277
await tester.pump();
261278
toolbarY = tester.getTopLeft(findToolbar()).dy;

‎packages/flutter/test/material/spell_check_suggestions_toolbar_test.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import 'package:flutter/cupertino.dart' show CupertinoTextSelectionToolbar;
56
import 'package:flutter/material.dart';
67
import 'package:flutter_test/flutter_test.dart';
78

@@ -48,7 +49,7 @@ void main() {
4849
testWidgets('positions toolbar below anchor when it fits above bottom view padding', (WidgetTester tester) async {
4950
// We expect the toolbar to be positioned right below the anchor with padding accounted for.
5051
const double expectedToolbarY =
51-
_kAnchor + (2 * SpellCheckSuggestionsToolbar.kToolbarContentDistanceBelow) - TextSelectionToolbar.kToolbarScreenPadding;
52+
_kAnchor + (2 * SpellCheckSuggestionsToolbar.kToolbarContentDistanceBelow) - CupertinoTextSelectionToolbar.kToolbarScreenPadding;
5253

5354
await tester.pumpWidget(
5455
MaterialApp(
@@ -68,7 +69,7 @@ void main() {
6869
testWidgets('re-positions toolbar higher below anchor when it does not fit above bottom view padding', (WidgetTester tester) async {
6970
// We expect the toolbar to be positioned _kTestToolbarOverlap pixels above the anchor with padding accounted for.
7071
const double expectedToolbarY =
71-
_kAnchor + (2 * SpellCheckSuggestionsToolbar.kToolbarContentDistanceBelow) - TextSelectionToolbar.kToolbarScreenPadding - _kTestToolbarOverlap;
72+
_kAnchor + (2 * SpellCheckSuggestionsToolbar.kToolbarContentDistanceBelow) - CupertinoTextSelectionToolbar.kToolbarScreenPadding - _kTestToolbarOverlap;
7273

7374
await tester.pumpWidget(
7475
MaterialApp(

0 commit comments

Comments
 (0)