Unverified Commit 3c12279f authored by includecmath's avatar includecmath Committed by GitHub

[widgets] Refactor Overlay insert methods entries assert (#62407)

parent bef6d154
...@@ -34,7 +34,7 @@ import 'ticker_provider.dart'; ...@@ -34,7 +34,7 @@ import 'ticker_provider.dart';
/// follows the user's finger across the screen after the drag begins. Using the /// follows the user's finger across the screen after the drag begins. Using the
/// overlay to display the drag avatar lets the avatar float over the other /// overlay to display the drag avatar lets the avatar float over the other
/// widgets in the app. As the user's finger moves, draggable calls /// widgets in the app. As the user's finger moves, draggable calls
/// [markNeedsBuild] on the overlay entry to cause it to rebuild. It its build, /// [markNeedsBuild] on the overlay entry to cause it to rebuild. In its build,
/// the entry includes a [Positioned] with its top and left property set to /// the entry includes a [Positioned] with its top and left property set to
/// position the drag avatar near the user's finger. When the drag is over, /// position the drag avatar near the user's finger. When the drag is over,
/// [Draggable] removes the entry from the overlay to remove the drag avatar /// [Draggable] removes the entry from the overlay to remove the drag avatar
...@@ -209,7 +209,7 @@ class Overlay extends StatefulWidget { ...@@ -209,7 +209,7 @@ class Overlay extends StatefulWidget {
/// [OverlayState] is initialized. /// [OverlayState] is initialized.
/// ///
/// Rather than creating an overlay, consider using the overlay that is /// Rather than creating an overlay, consider using the overlay that is
/// created by the [WidgetsApp] or the [MaterialApp] for the application. /// created by the [Navigator] in a [WidgetsApp] or a [MaterialApp] for the application.
const Overlay({ const Overlay({
Key key, Key key,
this.initialEntries = const <OverlayEntry>[], this.initialEntries = const <OverlayEntry>[],
...@@ -308,18 +308,7 @@ class OverlayState extends State<Overlay> with TickerProviderStateMixin { ...@@ -308,18 +308,7 @@ class OverlayState extends State<Overlay> with TickerProviderStateMixin {
/// ///
/// It is an error to specify both `above` and `below`. /// It is an error to specify both `above` and `below`.
void insert(OverlayEntry entry, { OverlayEntry below, OverlayEntry above }) { void insert(OverlayEntry entry, { OverlayEntry below, OverlayEntry above }) {
assert( assert(_debugVerifyInsertPosition(above, below));
above == null || below == null,
'Only one of `above` and `below` may be specified.',
);
assert(
above == null || (above._overlay == this && _entries.contains(above)),
'The provided entry for `above` is not present in the Overlay.',
);
assert(
below == null || (below._overlay == this && _entries.contains(below)),
'The provided entry for `below` is not present in the Overlay.',
);
assert(!_entries.contains(entry), 'The specified entry is already present in the Overlay.'); assert(!_entries.contains(entry), 'The specified entry is already present in the Overlay.');
assert(entry._overlay == null, 'The specified entry is already present in another Overlay.'); assert(entry._overlay == null, 'The specified entry is already present in another Overlay.');
entry._overlay = this; entry._overlay = this;
...@@ -336,18 +325,7 @@ class OverlayState extends State<Overlay> with TickerProviderStateMixin { ...@@ -336,18 +325,7 @@ class OverlayState extends State<Overlay> with TickerProviderStateMixin {
/// ///
/// It is an error to specify both `above` and `below`. /// It is an error to specify both `above` and `below`.
void insertAll(Iterable<OverlayEntry> entries, { OverlayEntry below, OverlayEntry above }) { void insertAll(Iterable<OverlayEntry> entries, { OverlayEntry below, OverlayEntry above }) {
assert( assert(_debugVerifyInsertPosition(above, below));
above == null || below == null,
'Only one of `above` and `below` may be specified.',
);
assert(
above == null || (above._overlay == this && _entries.contains(above)),
'The provided entry for `above` is not present in the Overlay.',
);
assert(
below == null || (below._overlay == this && _entries.contains(below)),
'The provided entry for `below` is not present in the Overlay.',
);
assert( assert(
entries.every((OverlayEntry entry) => !_entries.contains(entry)), entries.every((OverlayEntry entry) => !_entries.contains(entry)),
'One or more of the specified entries are already present in the Overlay.' 'One or more of the specified entries are already present in the Overlay.'
...@@ -367,6 +345,22 @@ class OverlayState extends State<Overlay> with TickerProviderStateMixin { ...@@ -367,6 +345,22 @@ class OverlayState extends State<Overlay> with TickerProviderStateMixin {
}); });
} }
bool _debugVerifyInsertPosition(OverlayEntry above, OverlayEntry below, { Iterable<OverlayEntry> newEntries }) {
assert(
above == null || below == null,
'Only one of `above` and `below` may be specified.',
);
assert(
above == null || (above._overlay == this && _entries.contains(above) && (newEntries?.contains(above) ?? true)),
'The provided entry used for `above` must be present in the Overlay${newEntries != null ? ' and in the `newEntriesList`' : ''}.',
);
assert(
below == null || (below._overlay == this && _entries.contains(below) && (newEntries?.contains(below) ?? true)),
'The provided entry used for `below` must be present in the Overlay${newEntries != null ? ' and in the `newEntriesList`' : ''}.',
);
return true;
}
/// Remove all the entries listed in the given iterable, then reinsert them /// Remove all the entries listed in the given iterable, then reinsert them
/// into the overlay in the given order. /// into the overlay in the given order.
/// ///
...@@ -386,18 +380,7 @@ class OverlayState extends State<Overlay> with TickerProviderStateMixin { ...@@ -386,18 +380,7 @@ class OverlayState extends State<Overlay> with TickerProviderStateMixin {
/// It is an error to specify both `above` and `below`. /// It is an error to specify both `above` and `below`.
void rearrange(Iterable<OverlayEntry> newEntries, { OverlayEntry below, OverlayEntry above }) { void rearrange(Iterable<OverlayEntry> newEntries, { OverlayEntry below, OverlayEntry above }) {
final List<OverlayEntry> newEntriesList = newEntries is List<OverlayEntry> ? newEntries : newEntries.toList(growable: false); final List<OverlayEntry> newEntriesList = newEntries is List<OverlayEntry> ? newEntries : newEntries.toList(growable: false);
assert( assert(_debugVerifyInsertPosition(above, below, newEntries: newEntriesList));
above == null || below == null,
'Only one of `above` and `below` may be specified.',
);
assert(
above == null || (above._overlay == this && _entries.contains(above) && newEntriesList.contains(above)),
'The entry used for `above` must be in the Overlay and in the `newEntriesList`.'
);
assert(
below == null || (below._overlay == this && _entries.contains(below) && newEntriesList.contains(below)),
'The entry used for `below` must be in the Overlay and in the `newEntriesList`.'
);
assert( assert(
newEntriesList.every((OverlayEntry entry) => entry._overlay == null || entry._overlay == this), newEntriesList.every((OverlayEntry entry) => entry._overlay == null || entry._overlay == this),
'One or more of the specified entries are already present in another Overlay.' 'One or more of the specified entries are already present in another Overlay.'
......
...@@ -619,6 +619,87 @@ void main() { ...@@ -619,6 +619,87 @@ void main() {
expect(buildOrder, <int>[3, 4, 1, 2, 0]); expect(buildOrder, <int>[3, 4, 1, 2, 0]);
}); });
testWidgets('debugVerifyInsertPosition', (WidgetTester tester) async {
final GlobalKey overlayKey = GlobalKey();
OverlayEntry base;
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: Overlay(
key: overlayKey,
initialEntries: <OverlayEntry>[
base = OverlayEntry(
builder: (BuildContext context) {
return Container();
},
),
],
),
),
);
final OverlayState overlay = overlayKey.currentState as OverlayState;
try {
overlay.insert(
OverlayEntry(builder: (BuildContext context) {
return Container();
}),
above: OverlayEntry(
builder: (BuildContext context) {
return Container();
},
),
below: OverlayEntry(
builder: (BuildContext context) {
return Container();
},
),
);
} catch (e) {
expect(e, isAssertionError);
expect(e.message, 'Only one of `above` and `below` may be specified.');
}
expect(() => overlay.insert(
OverlayEntry(builder: (BuildContext context) {
return Container();
}),
above: base,
), isNot(throwsAssertionError));
try {
overlay.insert(
OverlayEntry(builder: (BuildContext context) {
return Container();
}),
above: OverlayEntry(
builder: (BuildContext context) {
return Container();
},
),
);
} catch (e) {
expect(e, isAssertionError);
expect(e.message, 'The provided entry used for `above` must be present in the Overlay.');
}
try {
overlay.rearrange(<OverlayEntry>[base], above: OverlayEntry(
builder: (BuildContext context) {
return Container();
},
));
} catch (e) {
expect(e, isAssertionError);
expect(e.message, 'The provided entry used for `above` must be present in the Overlay and in the `newEntriesList`.');
}
await tester.pump();
});
testWidgets('OverlayState.of() called without Overlay being exist', (WidgetTester tester) async { testWidgets('OverlayState.of() called without Overlay being exist', (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
Directionality( Directionality(
......
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