Unverified Commit 00b0d550 authored by Justin McCandless's avatar Justin McCandless Committed by GitHub

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
parent 96c8c697
...@@ -19,9 +19,6 @@ const double _kToolbarHeight = 43.0; ...@@ -19,9 +19,6 @@ const double _kToolbarHeight = 43.0;
// Vertical distance between the tip of the arrow and the line of text the arrow // Vertical distance between the tip of the arrow and the line of text the arrow
// is pointing to. The value used here is eyeballed. // is pointing to. The value used here is eyeballed.
const double _kToolbarContentDistance = 8.0; const double _kToolbarContentDistance = 8.0;
// Minimal padding from all edges of the selection toolbar to all edges of the
// screen.
const double _kToolbarScreenPadding = 8.0;
const Size _kToolbarArrowSize = Size(14.0, 7.0); const Size _kToolbarArrowSize = Size(14.0, 7.0);
// Minimal padding from tip of the selection toolbar arrow to horizontal edges of the // Minimal padding from tip of the selection toolbar arrow to horizontal edges of the
...@@ -123,6 +120,16 @@ class CupertinoTextSelectionToolbar extends StatelessWidget { ...@@ -123,6 +120,16 @@ class CupertinoTextSelectionToolbar extends StatelessWidget {
/// default Cupertino toolbar. /// default Cupertino toolbar.
final CupertinoToolbarBuilder toolbarBuilder; final CupertinoToolbarBuilder toolbarBuilder;
/// Minimal padding from all edges of the selection toolbar to all edges of the
/// viewport.
///
/// See also:
///
/// * [SpellCheckSuggestionsToolbar], which uses this same value for its
/// padding from the edges of the viewport.
/// * [TextSelectionToolbar], which uses this same value as well.
static const double kToolbarScreenPadding = 8.0;
// Add the visual vertical line spacer between children buttons. // Add the visual vertical line spacer between children buttons.
static List<Widget> _addChildrenSpacers(List<Widget> children) { static List<Widget> _addChildrenSpacers(List<Widget> children) {
final List<Widget> nextChildren = <Widget>[]; final List<Widget> nextChildren = <Widget>[];
...@@ -163,7 +170,7 @@ class CupertinoTextSelectionToolbar extends StatelessWidget { ...@@ -163,7 +170,7 @@ class CupertinoTextSelectionToolbar extends StatelessWidget {
assert(debugCheckHasMediaQuery(context)); assert(debugCheckHasMediaQuery(context));
final EdgeInsets mediaQueryPadding = MediaQuery.paddingOf(context); final EdgeInsets mediaQueryPadding = MediaQuery.paddingOf(context);
final double paddingAbove = mediaQueryPadding.top + _kToolbarScreenPadding; final double paddingAbove = mediaQueryPadding.top + kToolbarScreenPadding;
final double toolbarHeightNeeded = paddingAbove final double toolbarHeightNeeded = paddingAbove
+ _kToolbarContentDistance + _kToolbarContentDistance
+ _kToolbarHeight; + _kToolbarHeight;
...@@ -180,15 +187,15 @@ class CupertinoTextSelectionToolbar extends StatelessWidget { ...@@ -180,15 +187,15 @@ class CupertinoTextSelectionToolbar extends StatelessWidget {
); );
final Offset anchorBelowAdjusted = Offset( final Offset anchorBelowAdjusted = Offset(
clampDouble(anchorBelow.dx, leftMargin, rightMargin), clampDouble(anchorBelow.dx, leftMargin, rightMargin),
anchorBelow.dy - _kToolbarContentDistance + paddingAbove, anchorBelow.dy + _kToolbarContentDistance - paddingAbove,
); );
return Padding( return Padding(
padding: EdgeInsets.fromLTRB( padding: EdgeInsets.fromLTRB(
_kToolbarScreenPadding, kToolbarScreenPadding,
paddingAbove, paddingAbove,
_kToolbarScreenPadding, kToolbarScreenPadding,
_kToolbarScreenPadding, kToolbarScreenPadding,
), ),
child: CustomSingleChildLayout( child: CustomSingleChildLayout(
delegate: TextSelectionToolbarLayoutDelegate( delegate: TextSelectionToolbarLayoutDelegate(
......
...@@ -2,8 +2,8 @@ ...@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'package:flutter/cupertino.dart';
import 'package:flutter/services.dart' show SuggestionSpan; import 'package:flutter/services.dart' show SuggestionSpan;
import 'package:flutter/widgets.dart';
import 'adaptive_text_selection_toolbar.dart'; import 'adaptive_text_selection_toolbar.dart';
import 'colors.dart'; import 'colors.dart';
...@@ -142,16 +142,16 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget { ...@@ -142,16 +142,16 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
anchor + const Offset(0.0, kToolbarContentDistanceBelow); anchor + const Offset(0.0, kToolbarContentDistanceBelow);
final MediaQueryData mediaQueryData = MediaQuery.of(context); final MediaQueryData mediaQueryData = MediaQuery.of(context);
final double softKeyboardViewInsetsBottom = mediaQueryData.viewInsets.bottom; final double softKeyboardViewInsetsBottom = mediaQueryData.viewInsets.bottom;
final double paddingAbove = mediaQueryData.padding.top + TextSelectionToolbar.kToolbarScreenPadding; final double paddingAbove = mediaQueryData.padding.top + CupertinoTextSelectionToolbar.kToolbarScreenPadding;
// Makes up for the Padding. // Makes up for the Padding.
final Offset localAdjustment = Offset(TextSelectionToolbar.kToolbarScreenPadding, paddingAbove); final Offset localAdjustment = Offset(CupertinoTextSelectionToolbar.kToolbarScreenPadding, paddingAbove);
return Padding( return Padding(
padding: EdgeInsets.fromLTRB( padding: EdgeInsets.fromLTRB(
TextSelectionToolbar.kToolbarScreenPadding, CupertinoTextSelectionToolbar.kToolbarScreenPadding,
kToolbarContentDistanceBelow, kToolbarContentDistanceBelow,
TextSelectionToolbar.kToolbarScreenPadding, CupertinoTextSelectionToolbar.kToolbarScreenPadding,
TextSelectionToolbar.kToolbarScreenPadding + softKeyboardViewInsetsBottom, CupertinoTextSelectionToolbar.kToolbarScreenPadding + softKeyboardViewInsetsBottom,
), ),
child: CustomSingleChildLayout( child: CustomSingleChildLayout(
delegate: SpellCheckSuggestionsToolbarLayoutDelegate( delegate: SpellCheckSuggestionsToolbarLayoutDelegate(
......
...@@ -4,9 +4,9 @@ ...@@ -4,9 +4,9 @@
import 'dart:math' as math; import 'dart:math' as math;
import 'package:flutter/cupertino.dart';
import 'package:flutter/foundation.dart' show listEquals; import 'package:flutter/foundation.dart' show listEquals;
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
import 'package:flutter/widgets.dart';
import 'debug.dart'; import 'debug.dart';
import 'icon_button.dart'; import 'icon_button.dart';
...@@ -76,15 +76,6 @@ class TextSelectionToolbar extends StatelessWidget { ...@@ -76,15 +76,6 @@ class TextSelectionToolbar extends StatelessWidget {
/// {@endtemplate} /// {@endtemplate}
final ToolbarBuilder toolbarBuilder; final ToolbarBuilder toolbarBuilder;
/// Minimal padding from all edges of the selection toolbar to all edges of the
/// viewport.
///
/// See also:
///
/// * [SpellCheckSuggestionsToolbar], which uses this same value for its
/// padding from the edges of the viewport.
static const double kToolbarScreenPadding = 8.0;
/// The size of the text selection handles. /// The size of the text selection handles.
/// ///
/// See also: /// See also:
...@@ -111,19 +102,20 @@ class TextSelectionToolbar extends StatelessWidget { ...@@ -111,19 +102,20 @@ class TextSelectionToolbar extends StatelessWidget {
final Offset anchorBelowPadded = final Offset anchorBelowPadded =
anchorBelow + const Offset(0.0, kToolbarContentDistanceBelow); anchorBelow + const Offset(0.0, kToolbarContentDistanceBelow);
const double screenPadding = CupertinoTextSelectionToolbar.kToolbarScreenPadding;
final double paddingAbove = MediaQuery.paddingOf(context).top final double paddingAbove = MediaQuery.paddingOf(context).top
+ kToolbarScreenPadding; + screenPadding;
final double availableHeight = anchorAbovePadded.dy - _kToolbarContentDistance - paddingAbove; final double availableHeight = anchorAbovePadded.dy - _kToolbarContentDistance - paddingAbove;
final bool fitsAbove = _kToolbarHeight <= availableHeight; final bool fitsAbove = _kToolbarHeight <= availableHeight;
// Makes up for the Padding above the Stack. // Makes up for the Padding above the Stack.
final Offset localAdjustment = Offset(kToolbarScreenPadding, paddingAbove); final Offset localAdjustment = Offset(screenPadding, paddingAbove);
return Padding( return Padding(
padding: EdgeInsets.fromLTRB( padding: EdgeInsets.fromLTRB(
kToolbarScreenPadding, screenPadding,
paddingAbove, paddingAbove,
kToolbarScreenPadding, screenPadding,
kToolbarScreenPadding, screenPadding,
), ),
child: CustomSingleChildLayout( child: CustomSingleChildLayout(
delegate: TextSelectionToolbarLayoutDelegate( delegate: TextSelectionToolbarLayoutDelegate(
......
...@@ -218,6 +218,7 @@ void main() { ...@@ -218,6 +218,7 @@ void main() {
const double height = _kToolbarHeight; const double height = _kToolbarHeight;
const double anchorBelowY = 500.0; const double anchorBelowY = 500.0;
double anchorAboveY = 0.0; double anchorAboveY = 0.0;
const double paddingAbove = 12.0;
await tester.pumpWidget( await tester.pumpWidget(
CupertinoApp( CupertinoApp(
...@@ -225,14 +226,26 @@ void main() { ...@@ -225,14 +226,26 @@ void main() {
child: StatefulBuilder( child: StatefulBuilder(
builder: (BuildContext context, StateSetter setter) { builder: (BuildContext context, StateSetter setter) {
setState = setter; setState = setter;
return CupertinoTextSelectionToolbar( final MediaQueryData data = MediaQuery.of(context);
anchorAbove: Offset(50.0, anchorAboveY), // Add some custom vertical padding to make this test more strict.
anchorBelow: const Offset(50.0, anchorBelowY), // By default in the testing environment, _kToolbarContentDistance
children: <Widget>[ // and the built-in padding from CupertinoApp can end up canceling
Container(color: const Color(0xffff0000), width: 50.0, height: height), // each other out.
Container(color: const Color(0xff00ff00), width: 50.0, height: height), return MediaQuery(
Container(color: const Color(0xff0000ff), width: 50.0, height: height), data: data.copyWith(
], padding: data.viewPadding.copyWith(
top: paddingAbove,
),
),
child: CupertinoTextSelectionToolbar(
anchorAbove: Offset(50.0, anchorAboveY),
anchorBelow: const Offset(50.0, anchorBelowY),
children: <Widget>[
Container(color: const Color(0xffff0000), width: 50.0, height: height),
Container(color: const Color(0xff00ff00), width: 50.0, height: height),
Container(color: const Color(0xff0000ff), width: 50.0, height: height),
],
),
); );
}, },
), ),
...@@ -244,10 +257,14 @@ void main() { ...@@ -244,10 +257,14 @@ void main() {
// belowAnchor. // belowAnchor.
double toolbarY = tester.getTopLeft(findToolbar()).dy; double toolbarY = tester.getTopLeft(findToolbar()).dy;
expect(toolbarY, equals(anchorBelowY + _kToolbarContentDistance)); expect(toolbarY, equals(anchorBelowY + _kToolbarContentDistance));
expect(find.byType(CustomSingleChildLayout), findsOneWidget);
final CustomSingleChildLayout layout = tester.widget(find.byType(CustomSingleChildLayout));
final TextSelectionToolbarLayoutDelegate delegate = layout.delegate as TextSelectionToolbarLayoutDelegate;
expect(delegate.anchorBelow.dy, anchorBelowY - paddingAbove);
// Even when it barely doesn't fit. // Even when it barely doesn't fit.
setState(() { setState(() {
anchorAboveY = 50.0; anchorAboveY = 70.0;
}); });
await tester.pump(); await tester.pump();
toolbarY = tester.getTopLeft(findToolbar()).dy; toolbarY = tester.getTopLeft(findToolbar()).dy;
...@@ -255,7 +272,7 @@ void main() { ...@@ -255,7 +272,7 @@ void main() {
// When it does fit above aboveAnchor, it positions itself there. // When it does fit above aboveAnchor, it positions itself there.
setState(() { setState(() {
anchorAboveY = 60.0; anchorAboveY = 80.0;
}); });
await tester.pump(); await tester.pump();
toolbarY = tester.getTopLeft(findToolbar()).dy; toolbarY = tester.getTopLeft(findToolbar()).dy;
......
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'package:flutter/cupertino.dart' show CupertinoTextSelectionToolbar;
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
...@@ -48,7 +49,7 @@ void main() { ...@@ -48,7 +49,7 @@ void main() {
testWidgets('positions toolbar below anchor when it fits above bottom view padding', (WidgetTester tester) async { testWidgets('positions toolbar below anchor when it fits above bottom view padding', (WidgetTester tester) async {
// We expect the toolbar to be positioned right below the anchor with padding accounted for. // We expect the toolbar to be positioned right below the anchor with padding accounted for.
const double expectedToolbarY = const double expectedToolbarY =
_kAnchor + (2 * SpellCheckSuggestionsToolbar.kToolbarContentDistanceBelow) - TextSelectionToolbar.kToolbarScreenPadding; _kAnchor + (2 * SpellCheckSuggestionsToolbar.kToolbarContentDistanceBelow) - CupertinoTextSelectionToolbar.kToolbarScreenPadding;
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
...@@ -68,7 +69,7 @@ void main() { ...@@ -68,7 +69,7 @@ void main() {
testWidgets('re-positions toolbar higher below anchor when it does not fit above bottom view padding', (WidgetTester tester) async { testWidgets('re-positions toolbar higher below anchor when it does not fit above bottom view padding', (WidgetTester tester) async {
// We expect the toolbar to be positioned _kTestToolbarOverlap pixels above the anchor with padding accounted for. // We expect the toolbar to be positioned _kTestToolbarOverlap pixels above the anchor with padding accounted for.
const double expectedToolbarY = const double expectedToolbarY =
_kAnchor + (2 * SpellCheckSuggestionsToolbar.kToolbarContentDistanceBelow) - TextSelectionToolbar.kToolbarScreenPadding - _kTestToolbarOverlap; _kAnchor + (2 * SpellCheckSuggestionsToolbar.kToolbarContentDistanceBelow) - CupertinoTextSelectionToolbar.kToolbarScreenPadding - _kTestToolbarOverlap;
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment