- 
                Notifications
    You must be signed in to change notification settings 
- Fork 347
Get route-transition duration robustly,instead of hard-coding #1920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
2ea25f6    to
    0de6501      
    Compare
  
    | Thanks. Some of these changes look good, but there are other places where a lot more code is getting changed and it's not obvious what the reasons for those changes are. That makes it considerably more work for someone to review, because they have to read through all those changes and reverse-engineer what your intent is. Please take a look at our guide to writing a clean commit history: 
 | 
bfa6bb4    to
    7d9283f      
    Compare
  
    | @gnprice I guess it's fine now. | 
| This is improved, but there are still some places like this: 
 For example: -      testWidgets('can show snackbar on success', (tester) async {
-        // Regression test for: https://github.com/zulip/zulip-flutter/issues/732
-        testBinding.deviceInfoResult = const IosDeviceInfo(systemVersion: '16.0');
-
-        final message = eg.streamMessage();
-        await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(messa
ge));
+    testWidgets('can show snackbar on success', (WidgetTester tester) async {
+      // Regression test for: https://github.com/zulip/zulip-flutter/issues/732
+      final TransitionDurationObserver transitionDurationObserver = TransitionDurationObserver();
+      await tester.pumpWidget(
+        MaterialApp(
+          navigatorObservers: <NavigatorObserver>[transitionDurationObserver],
+          home: Scaffold(
+            body: Builder(
+              builder: (context) {
+                testBinding.deviceInfoResult = const IosDeviceInfo(systemVersion: '16.0');
+                final message = eg.streamMessage();
+                setupToMessageActionSheet(tester,message: message,narrow: TopicNarrow.ofMessage(messa
ge));
+                return const SizedBox.shrink();}
+            ))));
+      final message = eg.streamMessage();
+       // Make the request take a bit of time to complete
+      prepareRawContentResponseSuccess(message: message,rawContent: 'Hello world',delay: const Durati
on(milliseconds: 500),
+      );
+      await tapCopyMessageTextButton(tester);
+      // … and pump a frame to finish the NavigationState.pop animation…
+      await transitionDurationObserver.pumpPastTransition(tester);
+      // … before the request finishes.  This is the repro condition for #732.
+      await transitionDurationObserver.pumpPastTransition(tester);
 
-        // Make the request take a bit of time to complete…
-        prepareRawContentResponseSuccess(message: message, rawContent: 'Hello world',
-          delay: const Duration(milliseconds: 500));
-        await tapCopyMessageTextButton(tester);
-        // … and pump a frame to finish the NavigationState.pop animation…
-        await tester.pump(const Duration(milliseconds: 250));
-        // … before the request finishes.  This is the repro condition for #732.
-        await tester.pump(const Duration(milliseconds: 250));
+      final snackbar = tester.widget<SnackBar>(find.byType(SnackBar));
+      check(snackbar.behavior).equals(SnackBarBehavior.floating);
 
-        final snackbar = tester.widget<SnackBar>(find.byType(SnackBar));
-        check(snackbar.behavior).equals(SnackBarBehavior.floating);
-        final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
-        tester.widget(find.descendant(matchRoot: true,
+      final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
+      tester.widget(find.descendant(matchRoot: true,
           of: find.byWidget(snackbar.content),
-          matching: find.text(zulipLocalizations.successMessageTextCopied)));
-      });
+          matching: find.text(zulipLocalizations.successMessageTextCopied),
+        ));
+    });
+Take a look again at the changes in your PR. Use  
 | 
| @gnprice done. Followed this flutter/flutter#171109 coding convention as suggested here #1668, otherwise the tests kept failing. fixed indentation mismatch related diffs(mostly). | 
7c59555    to
    af69cbf      
    Compare
  
    | This revision is somewhat improved again, but there are still some irrelevant changes, and still some places where more code is getting changed and it's not clear exactly why. If you need help in doing this: 
 then the #git-help channel on chat.zulip.org is a good place to ask. 
 Why mostly and not entirely? 
 I don't understand this comment. #1668 is the issue this PR is meant to fix, so in general these changes should be in the direction called for by that issue. If you're encountering test failures that you don't understand the cause of, then the #mobile-dev-help channel is the best venue to ask about them. | 
3ade4ff    to
    8d848b1      
    Compare
  
    | @gnprice cleaned up the pr. added          await transitionDurationObserver.pumpPastTransition(tester);
        // … before the request finishes.  This is the repro condition for #732.
        await transitionDurationObserver.pumpPastTransition(tester);
        // …pump for snackbar to show
        await tester.pumpAndSettle();though  Also, in  Future<void> showFromInbox(WidgetTester tester) async {
      transitionDurationObserver = TransitionDurationObserver();
      await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id,
        navigatorObservers: [transitionDurationObserver],
        child: const HomePage()));
      await tester.pump();
      check(find.byType(InboxPageBody)).findsOne();
      await tester.longPress(find.text(someChannel.name).hitTestable());
      await transitionDurationObserver.pumpPastTransition(tester);
    }but in  Future<void> showFromTopicListAppBar(WidgetTester tester, {int? streamId}) async {
    streamId ??= someChannel.streamId;
    final transitionDurationObserver = TransitionDurationObserver();
    connection.prepare(json: GetStreamTopicsResult(topics: []).toJson());
    await tester.pumpWidget(TestZulipApp(
      navigatorObservers: [transitionDurationObserver],
      accountId: eg.selfAccount.id,
      child: TopicListPage(streamId: streamId)));
    await tester.pump();
     .......both works but I followed  | 
| @gnprice have a look. | 
closes #1668