Skip to content

Commit 571df4b

Browse files
committed
wip fake_api [nfc]: Add delay option to connection.prepare; TODO test
This allows simulating a request that takes some time to return and consequently races with something else. This is NFC because `Future.delayed(Duration.zero, f)` is exactly equivalent to `Future(f)`. (The docs aren't real clear on this; but reading the implementations confirms they boil down to the very same `Timer` constructor call, and then the docs do seem to be trying to say that when read in light of that information.)
1 parent 88f4042 commit 571df4b

File tree

2 files changed

+80
-100
lines changed

2 files changed

+80
-100
lines changed

lib/widgets/action_sheet.dart

Lines changed: 59 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import 'package:share_plus/share_plus.dart';
66
import '../api/exception.dart';
77
import '../api/model/model.dart';
88
import '../api/route/messages.dart';
9-
import '../model/internal_link.dart';
10-
import '../model/narrow.dart';
119
import 'clipboard.dart';
1210
import 'compose_box.dart';
1311
import 'dialog.dart';
@@ -41,13 +39,12 @@ void showMessageActionSheet({required BuildContext context, required Message mes
4139
return Column(children: [
4240
if (!hasThumbsUpReactionVote) AddThumbsUpButton(message: message, messageListContext: context),
4341
StarButton(message: message, messageListContext: context),
42+
ShareButton(message: message, messageListContext: context),
4443
if (isComposeBoxOffered) QuoteAndReplyButton(
4544
message: message,
4645
messageListContext: context,
4746
),
4847
CopyMessageTextButton(message: message, messageListContext: context),
49-
CopyMessageLinkButton(message: message, messageListContext: context),
50-
ShareButton(message: message, messageListContext: context),
5148
]);
5249
});
5350
}
@@ -167,6 +164,63 @@ class StarButton extends MessageActionSheetMenuItemButton {
167164
}
168165
}
169166

167+
class ShareButton extends MessageActionSheetMenuItemButton {
168+
ShareButton({
169+
super.key,
170+
required super.message,
171+
required super.messageListContext,
172+
});
173+
174+
@override IconData get icon => Icons.adaptive.share;
175+
176+
@override
177+
String label(ZulipLocalizations zulipLocalizations) {
178+
return zulipLocalizations.actionSheetOptionShare;
179+
}
180+
181+
@override void onPressed(BuildContext context) async {
182+
// Close the message action sheet; we're about to show the share
183+
// sheet. (We could do this after the sharing Future settles
184+
// with [ShareResultStatus.success], but on iOS I get impatient with
185+
// how slowly our action sheet dismisses in that case.)
186+
// TODO(#24): Fix iOS bug where this call causes the keyboard to
187+
// reopen (if it was open at the time of this
188+
// `showMessageActionSheet` call) and cover a large part of the
189+
// share sheet.
190+
Navigator.of(context).pop();
191+
final zulipLocalizations = ZulipLocalizations.of(messageListContext);
192+
193+
final rawContent = await fetchRawContentWithFeedback(
194+
context: messageListContext,
195+
messageId: message.id,
196+
errorDialogTitle: zulipLocalizations.errorSharingFailed,
197+
);
198+
199+
if (rawContent == null) return;
200+
201+
if (!messageListContext.mounted) return;
202+
203+
// TODO: to support iPads, we're asked to give a
204+
// `sharePositionOrigin` param, or risk crashing / hanging:
205+
// https://pub.dev/packages/share_plus#ipad
206+
// Perhaps a wart in the API; discussion:
207+
// https://github.com/zulip/zulip-flutter/pull/12#discussion_r1130146231
208+
final result = await Share.share(rawContent);
209+
210+
switch (result.status) {
211+
// The plugin isn't very helpful: "The status can not be determined".
212+
// Until we learn otherwise, assume something wrong happened.
213+
case ShareResultStatus.unavailable:
214+
if (!messageListContext.mounted) return;
215+
await showErrorDialog(context: messageListContext,
216+
title: zulipLocalizations.errorSharingFailed);
217+
case ShareResultStatus.success:
218+
case ShareResultStatus.dismissed:
219+
// nothing to do
220+
}
221+
}
222+
}
223+
170224
/// Fetch and return the raw Markdown content for [messageId],
171225
/// showing an error dialog on failure.
172226
Future<String?> fetchRawContentWithFeedback({
@@ -307,96 +361,8 @@ class CopyMessageTextButton extends MessageActionSheetMenuItemButton {
307361

308362
if (!messageListContext.mounted) return;
309363

310-
copyWithPopup(context: messageListContext,
364+
copyWithPopup(context: context,
311365
successContent: Text(zulipLocalizations.successMessageTextCopied),
312366
data: ClipboardData(text: rawContent));
313367
}
314368
}
315-
316-
class CopyMessageLinkButton extends MessageActionSheetMenuItemButton {
317-
CopyMessageLinkButton({
318-
super.key,
319-
required super.message,
320-
required super.messageListContext,
321-
});
322-
323-
@override IconData get icon => Icons.link;
324-
325-
@override
326-
String label(ZulipLocalizations zulipLocalizations) {
327-
return zulipLocalizations.actionSheetOptionCopyMessageLink;
328-
}
329-
330-
@override void onPressed(BuildContext context) {
331-
Navigator.of(context).pop();
332-
final zulipLocalizations = ZulipLocalizations.of(messageListContext);
333-
334-
final store = PerAccountStoreWidget.of(messageListContext);
335-
final messageLink = narrowLink(
336-
store,
337-
SendableNarrow.ofMessage(message, selfUserId: store.selfUserId),
338-
nearMessageId: message.id,
339-
);
340-
341-
copyWithPopup(context: messageListContext,
342-
successContent: Text(zulipLocalizations.successMessageLinkCopied),
343-
data: ClipboardData(text: messageLink.toString()));
344-
}
345-
}
346-
347-
class ShareButton extends MessageActionSheetMenuItemButton {
348-
ShareButton({
349-
super.key,
350-
required super.message,
351-
required super.messageListContext,
352-
});
353-
354-
@override IconData get icon => Icons.adaptive.share;
355-
356-
@override
357-
String label(ZulipLocalizations zulipLocalizations) {
358-
return zulipLocalizations.actionSheetOptionShare;
359-
}
360-
361-
@override void onPressed(BuildContext context) async {
362-
// Close the message action sheet; we're about to show the share
363-
// sheet. (We could do this after the sharing Future settles
364-
// with [ShareResultStatus.success], but on iOS I get impatient with
365-
// how slowly our action sheet dismisses in that case.)
366-
// TODO(#24): Fix iOS bug where this call causes the keyboard to
367-
// reopen (if it was open at the time of this
368-
// `showMessageActionSheet` call) and cover a large part of the
369-
// share sheet.
370-
Navigator.of(context).pop();
371-
final zulipLocalizations = ZulipLocalizations.of(messageListContext);
372-
373-
final rawContent = await fetchRawContentWithFeedback(
374-
context: messageListContext,
375-
messageId: message.id,
376-
errorDialogTitle: zulipLocalizations.errorSharingFailed,
377-
);
378-
379-
if (rawContent == null) return;
380-
381-
if (!messageListContext.mounted) return;
382-
383-
// TODO: to support iPads, we're asked to give a
384-
// `sharePositionOrigin` param, or risk crashing / hanging:
385-
// https://pub.dev/packages/share_plus#ipad
386-
// Perhaps a wart in the API; discussion:
387-
// https://github.com/zulip/zulip-flutter/pull/12#discussion_r1130146231
388-
final result = await Share.share(rawContent);
389-
390-
switch (result.status) {
391-
// The plugin isn't very helpful: "The status can not be determined".
392-
// Until we learn otherwise, assume something wrong happened.
393-
case ShareResultStatus.unavailable:
394-
if (!messageListContext.mounted) return;
395-
await showErrorDialog(context: messageListContext,
396-
title: zulipLocalizations.errorSharingFailed);
397-
case ShareResultStatus.success:
398-
case ShareResultStatus.dismissed:
399-
// nothing to do
400-
}
401-
}
402-
}

test/api/fake_api.dart

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,22 @@ import 'package:zulip/model/store.dart';
88
import '../example_data.dart' as eg;
99

1010
sealed class _PreparedResponse {
11+
final Duration delay;
12+
13+
_PreparedResponse({this.delay = Duration.zero});
1114
}
1215

1316
class _PreparedException extends _PreparedResponse {
1417
final Object exception;
1518

16-
_PreparedException({required this.exception});
19+
_PreparedException({super.delay, required this.exception});
1720
}
1821

1922
class _PreparedSuccess extends _PreparedResponse {
2023
final int httpStatus;
2124
final List<int> bytes;
2225

23-
_PreparedSuccess({required this.httpStatus, required this.bytes});
26+
_PreparedSuccess({super.delay, required this.httpStatus, required this.bytes});
2427
}
2528

2629
/// An [http.Client] that accepts and replays canned responses, for testing.
@@ -53,12 +56,13 @@ class FakeHttpClient extends http.BaseClient {
5356
int? httpStatus,
5457
Map<String, dynamic>? json,
5558
String? body,
59+
Duration delay = Duration.zero,
5660
}) {
5761
assert(_nextResponse == null,
5862
'FakeApiConnection.prepare was called while already expecting a request');
5963
if (exception != null) {
6064
assert(httpStatus == null && json == null && body == null);
61-
_nextResponse = _PreparedException(exception: exception);
65+
_nextResponse = _PreparedException(exception: exception, delay: delay);
6266
} else {
6367
assert((json == null) || (body == null));
6468
final String resolvedBody = switch ((body, json)) {
@@ -69,6 +73,7 @@ class FakeHttpClient extends http.BaseClient {
6973
_nextResponse = _PreparedSuccess(
7074
httpStatus: httpStatus ?? 200,
7175
bytes: utf8.encode(resolvedBody),
76+
delay: delay,
7277
);
7378
}
7479
}
@@ -89,14 +94,16 @@ class FakeHttpClient extends http.BaseClient {
8994
final response = _nextResponse!;
9095
_nextResponse = null;
9196

97+
final http.StreamedResponse Function() computation;
9298
switch (response) {
9399
case _PreparedException(:var exception):
94-
return Future(() => throw exception);
100+
computation = () => throw exception;
95101
case _PreparedSuccess(:var bytes, :var httpStatus):
96102
final byteStream = http.ByteStream.fromBytes(bytes);
97-
return Future(() => http.StreamedResponse(
98-
byteStream, httpStatus, request: request));
103+
computation = () => http.StreamedResponse(
104+
byteStream, httpStatus, request: request);
99105
}
106+
return Future.delayed(response.delay, computation);
100107
}
101108
}
102109

@@ -203,13 +210,20 @@ class FakeApiConnection extends ApiConnection {
203210
///
204211
/// If `exception` is non-null, then `httpStatus`, `body`, and `json` must
205212
/// all be null, and the next request will throw the given exception.
213+
///
214+
/// In either case, the next request will complete a duration of `delay`
215+
/// after being started.
206216
void prepare({
207217
Object? exception,
208218
int? httpStatus,
209219
Map<String, dynamic>? json,
210220
String? body,
221+
Duration delay = Duration.zero,
211222
}) {
212223
client.prepare(
213-
exception: exception, httpStatus: httpStatus, json: json, body: body);
224+
exception: exception,
225+
httpStatus: httpStatus, json: json, body: body,
226+
delay: delay,
227+
);
214228
}
215229
}

0 commit comments

Comments
 (0)