From e6c531a76301c13e0a784dc71ce07ab5d08ee3fb Mon Sep 17 00:00:00 2001 From: Sayed Mahmood Sayedi Date: Tue, 26 Mar 2024 14:32:53 +0430 Subject: [PATCH] accounts-ui: Make accounts list scrollable Before this fix, the list of accounts did not scroll when they were more than a screenful and would show an overflow error on the screen. After this fix, the list of accounts scrolls properly with no overflow error and without shifting other content offscreen. Fixes: #100 --- lib/widgets/app.dart | 15 +++-- test/flutter_checks.dart | 7 +++ test/widgets/app_test.dart | 110 ++++++++++++++++++++++++++++++++++++- 3 files changed, 125 insertions(+), 7 deletions(-) diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index 75c7fd90e2..e32d39dd10 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -194,12 +194,15 @@ class ChooseAccountPage extends StatelessWidget { child: Center( child: ConstrainedBox( constraints: const BoxConstraints(maxWidth: 400), - child: Column(mainAxisAlignment: MainAxisAlignment.center, children: [ - for (final (:accountId, :account) in globalStore.accountEntries) - _buildAccountItem(context, - accountId: accountId, - title: Text(account.realmUrl.toString()), - subtitle: Text(account.email)), + child: Column(mainAxisSize: MainAxisSize.min, children: [ + Flexible(child: SingleChildScrollView( + child: Column(mainAxisSize: MainAxisSize.min, children: [ + for (final (:accountId, :account) in globalStore.accountEntries) + _buildAccountItem(context, + accountId: accountId, + title: Text(account.realmUrl.toString()), + subtitle: Text(account.email)), + ]))), const SizedBox(height: 12), ElevatedButton( onPressed: () => Navigator.push(context, diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index eed202ad68..bc7e9d83a7 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -5,6 +5,13 @@ import 'package:checks/checks.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; +extension RectChecks on Subject { + Subject get top => has((d) => d.top, 'top'); + Subject get bottom => has((d) => d.bottom, 'bottom'); + + // TODO others +} + extension AnimationChecks on Subject> { Subject get status => has((d) => d.status, 'status'); Subject get value => has((d) => d.value, 'value'); diff --git a/test/widgets/app_test.dart b/test/widgets/app_test.dart index 020a95986f..cac58f726b 100644 --- a/test/widgets/app_test.dart +++ b/test/widgets/app_test.dart @@ -1,11 +1,15 @@ import 'package:checks/checks.dart'; -import 'package:flutter/widgets.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_gen/gen_l10n/zulip_localizations.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/model/database.dart'; import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/inbox.dart'; import 'package:zulip/widgets/page.dart'; +import 'package:zulip/widgets/store.dart'; import '../example_data.dart' as eg; +import '../flutter_checks.dart'; import '../model/binding.dart'; import '../test_navigation.dart'; import 'page_checks.dart'; @@ -51,4 +55,108 @@ void main() { ]); }); }); + + group('ChooseAccountPage', () { + Future setupChooseAccountPage(WidgetTester tester, { + required List accounts, + }) async { + addTearDown(testBinding.reset); + + for (final account in accounts) { + await testBinding.globalStore + .insertAccount(account.toCompanion(false)); + } + + await tester.pumpWidget( + const MaterialApp( + localizationsDelegates: ZulipLocalizations.localizationsDelegates, + supportedLocales: ZulipLocalizations.supportedLocales, + home: GlobalStoreWidget( + child: ChooseAccountPage()))); + + // global store gets loaded + await tester.pumpAndSettle(); + } + + List generateAccounts(int count) { + return List.generate(count, (i) => eg.account( + id: i, + user: eg.user(fullName: 'User $i', email: 'user$i@example'), + apiKey: 'user${i}apikey', + )); + } + + Finder findAccount(Account account) => find.text(account.email).hitTestable(); + + Finder findButton({required String withText}) { + return find + .descendant(of: find.bySubtype(), matching: find.text(withText)) + .hitTestable(); + } + + void checkAccountShown(Account account, {required bool expected}) { + check(findAccount(account).evaluate()).length.equals(expected ? 1 : 0); + } + + void checkButtonShown({ + required String withText, + required bool expected, + }) { + check(findButton(withText: withText).evaluate()) + .length.equals(expected ? 1 : 0); + } + + testWidgets('accounts list is scrollable when more than a screenful', (tester) async { + final accounts = generateAccounts(15); + await setupChooseAccountPage(tester, accounts: accounts); + + // Accounts list is more than a screenful + // * First account is shown + // * Last account is out of view + checkAccountShown(accounts.first, expected: true); + checkAccountShown(accounts.last, expected: false); + + // Button to add an account is visible + // and not moved offscreen by the long list of accounts + checkButtonShown(withText: 'Add an account', expected: true); + + // Accounts list is scrollable to the bottom + await tester.scrollUntilVisible(findAccount(accounts.last), 50); + checkAccountShown(accounts.last, expected: true); + }); + + testWidgets('with just one account, the layout is centered', (tester) async { + final account = eg.selfAccount; + await setupChooseAccountPage(tester, accounts: [account]); + + const buttonText = 'Add an account'; + checkAccountShown(account, expected: true); + checkButtonShown(withText: buttonText, expected: true); + + final screenHeight = + (tester.view.physicalSize / tester.view.devicePixelRatio).height; + + check(tester.getRect(findAccount(account))) + ..top.isGreaterThan(1 / 3 * screenHeight) + ..bottom.isLessThan(2 / 3 * screenHeight); + + check(tester.getRect(findButton(withText: buttonText))) + ..top.isGreaterThan(1 / 3 * screenHeight) + ..bottom.isLessThan(2 / 3 * screenHeight); + }); + + testWidgets('with no accounts, the Add an Account button is centered', (tester) async { + await setupChooseAccountPage(tester, accounts: []); + + const buttonText = 'Add an account'; + checkButtonShown(withText: buttonText, expected: true); + + final screenHeight = + (tester.view.physicalSize / tester.view.devicePixelRatio).height; + + check(tester.getRect(findButton(withText: buttonText))) + ..top.isGreaterThan(1 / 3 * screenHeight) + ..bottom.isLessThan(2 / 3 * screenHeight); + }); + }); }