mentions : make @-mentions tappable to navigate to user profiles #1899
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| import 'package:flutter/material.dart'; | ||
loveucifer marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| import 'package:flutter_test/flutter_test.dart'; | ||
| import 'package:zulip/model/content.dart'; | ||
| import 'package:zulip/widgets/content.dart'; | ||
|
|
||
| import '../example_data.dart' as eg; | ||
| import '../model/binding.dart'; | ||
| import '../test_navigation.dart'; | ||
| import 'test_app.dart'; | ||
|
|
||
| Widget plainContent(String html) { | ||
| return Builder(builder: (context) => | ||
| DefaultTextStyle( | ||
| style: ContentTheme.of(context).textStylePlainParagraph, | ||
| child: BlockContentList(nodes: parseContent(html).nodes))); | ||
| } | ||
|
|
||
| Future<void> prepareContent(WidgetTester tester, Widget child, { | ||
| bool wrapWithPerAccountStoreWidget = false, | ||
| }) async { | ||
| if (wrapWithPerAccountStoreWidget) { | ||
| await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); | ||
| } | ||
| addTearDown(testBinding.reset); | ||
|
|
||
| await tester.pumpWidget(TestZulipApp( | ||
| accountId: wrapWithPerAccountStoreWidget ? eg.selfAccount.id : null, | ||
| child: child)); | ||
| await tester.pump(); // global store | ||
| if (wrapWithPerAccountStoreWidget) { | ||
| await tester.pump(); | ||
| } | ||
| } | ||
|
|
||
| void main() { | ||
| TestZulipBinding.ensureInitialized(); | ||
|
|
||
| group('UserMention tappable functionality', () { | ||
| testWidgets('mention with valid user ID has gesture detector', (tester) async { | ||
| await prepareContent(tester, plainContent('<p><span class="user-mention" data-user-id="123">@Test User</span></p>')); | ||
| expect(find.byType(GestureDetector), findsOneWidget); | ||
| }); | ||
|
|
||
| testWidgets('mention with user ID navigates to ProfilePage when tapped', (tester) async { | ||
| final pushedRoutes = <Route<dynamic>>[]; | ||
| final testNavObserver = TestNavigatorObserver() | ||
| ..onPushed = (route, prevRoute) => pushedRoutes.add(route); | ||
|
|
||
| await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); | ||
| addTearDown(testBinding.reset); | ||
| await tester.pumpWidget(TestZulipApp( | ||
| accountId: eg.selfAccount.id, | ||
| navigatorObservers: [testNavObserver], | ||
| child: plainContent('<p><span class="user-mention" data-user-id="123">@Test User</span></p>'), | ||
| )); | ||
| await tester.pump(); // global store | ||
|
|
||
| await tester.pump(); // Allow any deferred work to complete | ||
|
|
||
| expect(find.byType(GestureDetector), findsOneWidget); | ||
|
|
||
| await tester.tap(find.byType(GestureDetector)); | ||
| await tester.pump(); | ||
|
|
||
| // Verify that navigation occurred (at least one route was pushed) | ||
| expect(pushedRoutes.length, greaterThanOrEqualTo(1)); | ||
| }); | ||
|
|
||
| testWidgets('mention without user ID does not have gesture detector', (tester) async { | ||
| await prepareContent(tester, plainContent('<p><span class="user-mention">@Test User</span></p>')); | ||
| expect(find.byType(GestureDetector), findsNothing); | ||
| }); | ||
|
|
||
| testWidgets('mention with invalid user ID does not have gesture detector', (tester) async { | ||
| await prepareContent(tester, plainContent('<p><span class="user-mention" data-user-id="invalid">@Test User</span></p>')); | ||
| expect(find.byType(GestureDetector), findsNothing); | ||
| }); | ||
|
|
||
| testWidgets('mention with wildcard user ID does not have gesture detector', (tester) async { | ||
| await prepareContent(tester, plainContent('<p><span class="user-mention" data-user-id="*">@all</span></p>')); | ||
| expect(find.byType(GestureDetector), findsNothing); | ||
| }); | ||
|
|
||
| testWidgets('mention with zero user ID does not have gesture detector', (tester) async { | ||
| await prepareContent(tester, plainContent('<p><span class="user-mention" data-user-id="0">@Test User</span></p>')); | ||
| expect(find.byType(GestureDetector), findsNothing); | ||
| }); | ||
|
|
||
| testWidgets('mention with negative user ID does not have gesture detector', (tester) async { | ||
| await prepareContent(tester, plainContent('<p><span class="user-mention" data-user-id="-1">@Test User</span></p>')); | ||
| expect(find.byType(GestureDetector), findsNothing); | ||
| }); | ||
| }); | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be required so that the parameters are explicit, especially in the tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW the reason I generally want the fields on content nodes (and other types from the API) to be required in the constructors is for the sake of the code other than the tests. It's because these generally don't have reasonable defaults outside the context of making test data — instead, the only reasonable value to to use is whatever comes from the actual data from the server, and there's usually not any call site where that would have a constant value like null.
In test data, by contrast, there's very commonly some reasonable "boring" value that would work for most tests without the reader of the test needing to care about what specific value it is. For those, defaults are good. The contrast where a lot of fields have good defaults for test data but no good defaults for the app's code is why
example_data.darthas lots of functions that basically just wrap constructors for various API types but provide defaults.