Skip to content

Commit

Permalink
Allow routes other than NavigationSheetRoute to be used in Navigation…
Browse files Browse the repository at this point in the history
…Sheet (#165)

Fixes #139.

Essentially the issue was fixed by the changes in #164, so this PR just
adds some missing pieces to fully support non-`NavigationSheetRoute`s in
`NavigationSheet`.

- Relaxed assertion to allow routes other than `NavigationSheetRoute`.
- Removed equals and hashCode to reduce unnecessary complexity in code.
- Implemented `canTransitionFrom` and `canTransitionTo` of
`NavigationSheetRoute`.
- Added a regression test of #139.
- Disabled `unneecssary_parenthesis` rule.
- Bumped the version to 0.7.3.
  • Loading branch information
fujidaiti authored Jun 9, 2024
1 parent 0cae9c1 commit fc28743
Show file tree
Hide file tree
Showing 7 changed files with 155 additions and 78 deletions.
4 changes: 4 additions & 0 deletions package/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## 0.7.3 Jun 9, 2024

- Fix: DropdownButton doesn't work in NavigationSheet (#139)

## 0.7.2 Jun 9, 2024

- Fix: Attaching SheetController to NavigationSheet causes "Null check operator used on a null value" (#151)
Expand Down
1 change: 1 addition & 0 deletions package/analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ linter:
cascade_invocations: false
join_return_with_assignment: false
prefer_relative_imports: true
unnecessary_parenthesis: false
72 changes: 15 additions & 57 deletions package/lib/src/internal/transition_observer.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'package:flutter/foundation.dart';
import 'package:flutter/widgets.dart';

class TransitionObserver extends NavigatorObserver {
Expand Down Expand Up @@ -167,15 +168,8 @@ class NoTransition extends Transition {
final ModalRoute<dynamic> currentRoute;

@override
bool operator ==(Object other) {
return identical(this, other) ||
(other is NoTransition &&
runtimeType == other.runtimeType &&
currentRoute == other.currentRoute);
}

@override
int get hashCode => Object.hash(runtimeType, currentRoute);
String toString() =>
'$NoTransition${(currentRoute: describeIdentity(currentRoute))}';
}

class ForwardTransition extends Transition {
Expand All @@ -190,22 +184,10 @@ class ForwardTransition extends Transition {
final Animation<double> animation;

@override
bool operator ==(Object other) {
return identical(this, other) ||
(other is ForwardTransition &&
runtimeType == other.runtimeType &&
originRoute == other.originRoute &&
destinationRoute == other.destinationRoute &&
animation == other.animation);
}

@override
int get hashCode => Object.hash(
runtimeType,
originRoute,
destinationRoute,
animation,
);
String toString() => '$ForwardTransition${(
originRoute: describeIdentity(originRoute),
destinationRoute: describeIdentity(destinationRoute),
)}';
}

class BackwardTransition extends Transition {
Expand All @@ -220,22 +202,10 @@ class BackwardTransition extends Transition {
final Animation<double> animation;

@override
bool operator ==(Object other) {
return identical(this, other) ||
(other is BackwardTransition &&
runtimeType == other.runtimeType &&
originRoute == other.originRoute &&
destinationRoute == other.destinationRoute &&
animation == other.animation);
}

@override
int get hashCode => Object.hash(
runtimeType,
originRoute,
destinationRoute,
animation,
);
String toString() => '$BackwardTransition${(
originRoute: describeIdentity(originRoute),
destinationRoute: describeIdentity(destinationRoute),
)}';
}

class UserGestureTransition extends Transition {
Expand All @@ -250,20 +220,8 @@ class UserGestureTransition extends Transition {
final Animation<double> animation;

@override
bool operator ==(Object other) {
return identical(this, other) ||
(other is UserGestureTransition &&
runtimeType == other.runtimeType &&
currentRoute == other.currentRoute &&
previousRoute == other.previousRoute &&
animation == other.animation);
}

@override
int get hashCode => Object.hash(
runtimeType,
currentRoute,
previousRoute,
animation,
);
String toString() => '$UserGestureTransition${(
currentRoute: describeIdentity(currentRoute),
previousRoute: describeIdentity(previousRoute),
)}';
}
10 changes: 10 additions & 0 deletions package/lib/src/navigation/navigation_route.dart
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ abstract class NavigationSheetRoute<T, E extends SheetExtent>

RouteTransitionsBuilder? get transitionsBuilder;

@override
bool canTransitionFrom(TransitionRoute<dynamic> previousRoute) {
return previousRoute is NavigationSheetRoute;
}

@override
bool canTransitionTo(TransitionRoute<dynamic> nextRoute) {
return nextRoute is NavigationSheetRoute;
}

@override
Widget buildTransitions(
BuildContext context,
Expand Down
43 changes: 36 additions & 7 deletions package/lib/src/navigation/navigation_sheet_extent.dart
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,43 @@ class NavigationSheetExtent extends SheetExtent {
assert(
() {
switch ((_lastReportedTransition, activity)) {
case (NoTransition(), ProxySheetActivity()):
case (ForwardTransition(), TransitionSheetActivity()):
case (BackwardTransition(), TransitionSheetActivity()):
case (UserGestureTransition(), TransitionSheetActivity()):
case (null, _):
// Allowed patterns.
case (
NoTransition(currentRoute: NavigationSheetRoute()),
ProxySheetActivity(),
):
case (
ForwardTransition(
originRoute: NavigationSheetRoute(),
destinationRoute: NavigationSheetRoute(),
),
TransitionSheetActivity(),
):
case (
BackwardTransition(
originRoute: NavigationSheetRoute(),
destinationRoute: NavigationSheetRoute(),
),
TransitionSheetActivity(),
):
case (
UserGestureTransition(
currentRoute: NavigationSheetRoute(),
previousRoute: NavigationSheetRoute(),
),
TransitionSheetActivity(),
):
case (_, final activity) when activity is! NavigationSheetActivity:
return true;
case _:
return false;

// Other patterns are not allowed.
case (final transition, final activity):
throw FlutterError(
'There is an inconsistency between the current transition state '
'and the current sheet activity type.\n'
' Transition: $transition\n'
' Activity: ${describeIdentity(activity)}',
);
}
}(),
);
Expand Down
2 changes: 1 addition & 1 deletion package/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: smooth_sheets
description: Sheet widgets with smooth motion and great flexibility. Also supports nested navigation in both imperative and declarative ways.
version: 0.7.2
version: 0.7.3
repository: https://github.com/fujidaiti/smooth_sheets
screenshots:
- description: Practical examples of smooth_sheets.
Expand Down
101 changes: 88 additions & 13 deletions package/test/navigation/navigation_sheet_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ class _TestWidget extends StatelessWidget {
required this.routes,
this.onTapBackgroundText,
this.sheetController,
this.useMaterialApp = false,
});

final String initialRoute;
final Map<String, ValueGetter<Route<dynamic>>> routes;
final VoidCallback? onTapBackgroundText;
final SheetController? sheetController;
final NavigationSheetTransitionObserver sheetTransitionObserver;
final bool useMaterialApp;

@override
Widget build(BuildContext context) {
Expand All @@ -32,21 +34,26 @@ class _TestWidget extends StatelessWidget {
),
);

return Directionality(
textDirection: TextDirection.ltr,
child: MediaQuery(
data: const MediaQueryData(),
child: Stack(
children: [
TextButton(
onPressed: onTapBackgroundText,
child: const Text('Background text'),
),
navigationSheet,
],
final content = Stack(
children: [
TextButton(
onPressed: onTapBackgroundText,
child: const Text('Background text'),
),
),
navigationSheet,
],
);

return switch (useMaterialApp) {
true => MaterialApp(home: content),
false => Directionality(
textDirection: TextDirection.ltr,
child: MediaQuery(
data: const MediaQueryData(),
child: content,
),
),
};
}
}

Expand Down Expand Up @@ -236,4 +243,72 @@ void main() {
expect(lastBoundaryValues, equals((200, 500)));
},
);

// Regression test for https://github.com/fujidaiti/smooth_sheets/issues/139
testWidgets(
'Works with DropdownButton without crashing',
(tester) async {
String? selectedOption = 'Option 1';
final routeWithDropdownButton = DraggableNavigationSheetRoute<dynamic>(
builder: (context) {
return Scaffold(
body: Center(
child: StatefulBuilder(
builder: (_, setState) {
return DropdownButton(
value: selectedOption,
menuMaxHeight: 150,
// Ensure all the items are visible at once.
itemHeight: 50,
onChanged: (newValue) =>
setState(() => selectedOption = newValue),
items: [
for (final option in const [
'Option 1',
'Option 2',
'Option 3',
])
DropdownMenuItem(
value: option,
child: Text(option),
),
],
);
},
),
),
);
},
);

await tester.pumpWidget(
_TestWidget(
transitionObserver,
initialRoute: 'first',
useMaterialApp: true,
routes: {'first': () => routeWithDropdownButton},
),
);

// 'Option 1' is selected at first.
expect(find.text('Option 1'), findsOneWidget);

// Tapping 'Option 1' should display a popup menu.
await tester.tap(find.text('Option 1'));
await tester.pumpAndSettle();
// There are two 'Option 1' texts at this point:
// one in the dropdown button and the other in the popup menu.
expect(find.text('Option 1'), findsNWidgets(2));
expect(find.text('Option 2'), findsOneWidget);
expect(find.text('Option 3'), findsOneWidget);

// Selecting 'Option 2' should close the popup menu,
// and 'Option 2' should be displayed in the dropdown button.
await tester.tap(find.text('Option 2'));
await tester.pumpAndSettle();
expect(find.text('Option 1'), findsNothing);
expect(find.text('Option 2'), findsOneWidget);
expect(find.text('Option 3'), findsNothing);
},
);
}

0 comments on commit fc28743

Please sign in to comment.