From 2b3e7819cac210a67bc96873e95e04cf0535dffb Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 12 Apr 2023 14:49:46 -0700 Subject: [PATCH 01/11] deps: Upgrade material_color_utilities, source_span via `flutter pub get` Much like the `meta` package a few weeks ago in 0b89ce988, I don't understand why this causes an update, but it does. --- pubspec.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pubspec.lock b/pubspec.lock index c88d7b12b1..655a9bd6b3 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -405,10 +405,10 @@ packages: dependency: transitive description: name: material_color_utilities - sha256: d92141dc6fe1dad30722f9aa826c7fbc896d021d792f80678280601aff8cf724 + sha256: "586678f20e112219ed0f73215f01bcdf1d769824ba2ebae45ad918a9bfde9bdb" url: "https://pub.dev" source: hosted - version: "0.2.0" + version: "0.3.0" meta: dependency: transitive description: @@ -634,10 +634,10 @@ packages: dependency: transitive description: name: source_span - sha256: dd904f795d4b4f3b870833847c461801f6750a9fa8e61ea5ac53f9422b31f250 + sha256: "53e943d4206a5e30df338fd4c6e7a077e02254531b138a15aec3bd143c1a8b3c" url: "https://pub.dev" source: hosted - version: "1.9.1" + version: "1.10.0" stack_trace: dependency: transitive description: From ef3db6899ea5e24129fcacf6ca9d8b042149fa1b Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 11 Apr 2023 15:16:57 -0700 Subject: [PATCH 02/11] api: Add `close` method to ApiConnection Soon we'll give LiveApiConnection a nontrivial implementation of this method. --- lib/api/core.dart | 12 ++++++++++++ lib/model/store.dart | 2 +- test/api/fake_api.dart | 5 +++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/api/core.dart b/lib/api/core.dart index 6da65a95a6..d9542efdd7 100644 --- a/lib/api/core.dart +++ b/lib/api/core.dart @@ -32,6 +32,8 @@ abstract class ApiConnection { // that ensures nothing assumes base class has a real API key final Auth auth; + void close(); + Future get(String route, Map? params); Future post(String route, Map? params); @@ -49,10 +51,19 @@ Map authHeader(Auth auth) { class LiveApiConnection extends ApiConnection { LiveApiConnection({required super.auth}); + bool _isOpen = true; + + @override + void close() { + assert(_isOpen); + _isOpen = false; + } + Map _headers() => authHeader(auth); @override Future get(String route, Map? params) async { + assert(_isOpen); final baseUrl = Uri.parse(auth.realmUrl); final url = Uri( scheme: baseUrl.scheme, @@ -71,6 +82,7 @@ class LiveApiConnection extends ApiConnection { @override Future post(String route, Map? params) async { + assert(_isOpen); final response = await http.post( Uri.parse("${auth.realmUrl}/api/v1/$route"), headers: _headers(), diff --git a/lib/model/store.dart b/lib/model/store.dart index 3feec9f1ca..da08f9fae0 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -294,7 +294,7 @@ class LivePerAccountStore extends PerAccountStore { final result = await getEvents(connection, queueId: queueId, lastEventId: lastEventId); // TODO handle errors on get-events; retry with backoff - // TODO abort long-poll on [dispose] + // TODO abort long-poll and close LiveApiConnection on [dispose] final events = result.events; for (final event in events) { handleEvent(event); diff --git a/test/api/fake_api.dart b/test/api/fake_api.dart index 4fbcd7394a..a0533c4ea0 100644 --- a/test/api/fake_api.dart +++ b/test/api/fake_api.dart @@ -20,6 +20,11 @@ class FakeApiConnection extends ApiConnection { _nextResponse = response; } + @override + void close() { + // TODO: record connection closed; assert open in methods + } + @override Future get(String route, Map? params) async { final response = _nextResponse; From ab60489a584e020ac669980bc50ca106c5959e9f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 11 Apr 2023 14:56:07 -0700 Subject: [PATCH 03/11] api: Have LiveApiConnection instance use a persistent http.Client As Alex points out in discussion: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20http.20.60Client.60/near/1545511 > Looks like doing so will allow connection reuse, which is a great > performance boost, particularly with HTTP/2. So that seems good. This will also help with file uploads (#56, #57, and #61) because http.Client has a `send` method -- https://pub.dev/documentation/http/latest/http/Client/send.html -- that we can use for requests we want more control over (in particular, a file-upload request), and a counterpart toplevel convenience function like `http.send` isn't offered. See doc: https://pub.dev/documentation/http/latest/http/Client-class.html --- lib/api/core.dart | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/api/core.dart b/lib/api/core.dart index d9542efdd7..a59b40b93a 100644 --- a/lib/api/core.dart +++ b/lib/api/core.dart @@ -51,11 +51,14 @@ Map authHeader(Auth auth) { class LiveApiConnection extends ApiConnection { LiveApiConnection({required super.auth}); + final http.Client _client = http.Client(); + bool _isOpen = true; @override void close() { assert(_isOpen); + _client.close(); _isOpen = false; } @@ -73,7 +76,7 @@ class LiveApiConnection extends ApiConnection { path: "/api/v1/$route", queryParameters: encodeParameters(params)); if (kDebugMode) print("GET $url"); - final response = await http.get(url, headers: _headers()); + final response = await _client.get(url, headers: _headers()); if (response.statusCode != 200) { throw Exception("error on GET $route: status ${response.statusCode}"); } @@ -83,7 +86,7 @@ class LiveApiConnection extends ApiConnection { @override Future post(String route, Map? params) async { assert(_isOpen); - final response = await http.post( + final response = await _client.post( Uri.parse("${auth.realmUrl}/api/v1/$route"), headers: _headers(), body: encodeParameters(params)); From ca20d0e9a5b2aeecfbfbe218d136175d8e5e05de Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 12 Apr 2023 13:32:38 -0700 Subject: [PATCH 04/11] deps: Add file_picker at latest Toward #57, "Attach files from compose box, with file picker". --- ios/Podfile.lock | 53 +++++++++++++++++++++++++++++++++++++++++++ ios/Runner/Info.plist | 4 ++++ pubspec.lock | 16 +++++++++++++ pubspec.yaml | 1 + 4 files changed, 74 insertions(+) diff --git a/ios/Podfile.lock b/ios/Podfile.lock index 834f5dce9f..d1ddbbbd44 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -1,22 +1,70 @@ PODS: - device_info_plus (0.0.1): - Flutter + - DKImagePickerController/Core (4.3.4): + - DKImagePickerController/ImageDataManager + - DKImagePickerController/Resource + - DKImagePickerController/ImageDataManager (4.3.4) + - DKImagePickerController/PhotoGallery (4.3.4): + - DKImagePickerController/Core + - DKPhotoGallery + - DKImagePickerController/Resource (4.3.4) + - DKPhotoGallery (0.0.17): + - DKPhotoGallery/Core (= 0.0.17) + - DKPhotoGallery/Model (= 0.0.17) + - DKPhotoGallery/Preview (= 0.0.17) + - DKPhotoGallery/Resource (= 0.0.17) + - SDWebImage + - SwiftyGif + - DKPhotoGallery/Core (0.0.17): + - DKPhotoGallery/Model + - DKPhotoGallery/Preview + - SDWebImage + - SwiftyGif + - DKPhotoGallery/Model (0.0.17): + - SDWebImage + - SwiftyGif + - DKPhotoGallery/Preview (0.0.17): + - DKPhotoGallery/Model + - DKPhotoGallery/Resource + - SDWebImage + - SwiftyGif + - DKPhotoGallery/Resource (0.0.17): + - SDWebImage + - SwiftyGif + - file_picker (0.0.1): + - DKImagePickerController/PhotoGallery + - Flutter - Flutter (1.0.0) - path_provider_foundation (0.0.1): - Flutter - FlutterMacOS + - SDWebImage (5.15.5): + - SDWebImage/Core (= 5.15.5) + - SDWebImage/Core (5.15.5) - share_plus (0.0.1): - Flutter + - SwiftyGif (5.4.4) DEPENDENCIES: - device_info_plus (from `.symlinks/plugins/device_info_plus/ios`) + - file_picker (from `.symlinks/plugins/file_picker/ios`) - Flutter (from `Flutter`) - path_provider_foundation (from `.symlinks/plugins/path_provider_foundation/darwin`) - share_plus (from `.symlinks/plugins/share_plus/ios`) +SPEC REPOS: + trunk: + - DKImagePickerController + - DKPhotoGallery + - SDWebImage + - SwiftyGif + EXTERNAL SOURCES: device_info_plus: :path: ".symlinks/plugins/device_info_plus/ios" + file_picker: + :path: ".symlinks/plugins/file_picker/ios" Flutter: :path: Flutter path_provider_foundation: @@ -26,9 +74,14 @@ EXTERNAL SOURCES: SPEC CHECKSUMS: device_info_plus: e5c5da33f982a436e103237c0c85f9031142abed + DKImagePickerController: b512c28220a2b8ac7419f21c491fc8534b7601ac + DKPhotoGallery: fdfad5125a9fdda9cc57df834d49df790dbb4179 + file_picker: ce3938a0df3cc1ef404671531facef740d03f920 Flutter: f04841e97a9d0b0a8025694d0796dd46242b2854 path_provider_foundation: c68054786f1b4f3343858c1e1d0caaded73f0be9 + SDWebImage: fd7e1a22f00303e058058278639bf6196ee431fe share_plus: 056a1e8ac890df3e33cb503afffaf1e9b4fbae68 + SwiftyGif: 93a1cc87bf3a51916001cf8f3d63835fb64c819f PODFILE CHECKSUM: 985e5b058f26709dc81f9ae74ea2b2775bdbcefe diff --git a/ios/Runner/Info.plist b/ios/Runner/Info.plist index a3c26d885f..6ddd93282d 100644 --- a/ios/Runner/Info.plist +++ b/ios/Runner/Info.plist @@ -49,5 +49,9 @@ UIApplicationSupportsIndirectInputEvents + UIBackgroundModes + + fetch + diff --git a/pubspec.lock b/pubspec.lock index 655a9bd6b3..b5fe0a93c7 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -250,6 +250,14 @@ packages: url: "https://pub.dev" source: hosted version: "6.1.4" + file_picker: + dependency: "direct main" + description: + name: file_picker + sha256: "0d923fb610d0abf67f2149c3a50ef85f78bebecfc4d645719ca70bcf4abc788f" + url: "https://pub.dev" + source: hosted + version: "5.2.7" fixnum: dependency: transitive description: @@ -271,6 +279,14 @@ packages: url: "https://pub.dev" source: hosted version: "2.0.1" + flutter_plugin_android_lifecycle: + dependency: transitive + description: + name: flutter_plugin_android_lifecycle + sha256: c224ac897bed083dabf11f238dd11a239809b446740be0c2044608c50029ffdf + url: "https://pub.dev" + source: hosted + version: "2.0.9" flutter_test: dependency: "direct dev" description: flutter diff --git a/pubspec.yaml b/pubspec.yaml index d8616fd694..0deef45732 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -47,6 +47,7 @@ dependencies: intl: ^0.18.0 share_plus: ^6.3.1 device_info_plus: ^8.1.0 + file_picker: ^5.2.7 dev_dependencies: flutter_test: From 723b399f918c582445a0b5465758bd9ad16d3b07 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 12 Apr 2023 13:34:35 -0700 Subject: [PATCH 05/11] model: Add maxFileUploadSizeMib to InitialSnapshot And, from there, to PerAccountStore. --- lib/api/model/initial_snapshot.dart | 3 +++ lib/api/model/initial_snapshot.g.dart | 2 ++ lib/model/store.dart | 4 +++- test/example_data.dart | 1 + 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/api/model/initial_snapshot.dart b/lib/api/model/initial_snapshot.dart index 9a92038200..4feaff093e 100644 --- a/lib/api/model/initial_snapshot.dart +++ b/lib/api/model/initial_snapshot.dart @@ -21,6 +21,8 @@ class InitialSnapshot { final List subscriptions; + final int maxFileUploadSizeMib; + // TODO etc., etc. InitialSnapshot({ @@ -32,6 +34,7 @@ class InitialSnapshot { required this.alertWords, required this.customProfileFields, required this.subscriptions, + required this.maxFileUploadSizeMib, }); factory InitialSnapshot.fromJson(Map json) => diff --git a/lib/api/model/initial_snapshot.g.dart b/lib/api/model/initial_snapshot.g.dart index d70fbdb3b1..55e2d5555e 100644 --- a/lib/api/model/initial_snapshot.g.dart +++ b/lib/api/model/initial_snapshot.g.dart @@ -22,6 +22,7 @@ InitialSnapshot _$InitialSnapshotFromJson(Map json) => subscriptions: (json['subscriptions'] as List) .map((e) => Subscription.fromJson(e as Map)) .toList(), + maxFileUploadSizeMib: json['max_file_upload_size_mib'] as int, ); Map _$InitialSnapshotToJson(InitialSnapshot instance) => @@ -34,4 +35,5 @@ Map _$InitialSnapshotToJson(InitialSnapshot instance) => 'alert_words': instance.alertWords, 'custom_profile_fields': instance.customProfileFields, 'subscriptions': instance.subscriptions, + 'max_file_upload_size_mib': instance.maxFileUploadSizeMib, }; diff --git a/lib/model/store.dart b/lib/model/store.dart index da08f9fae0..67a1a815dd 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -142,13 +142,15 @@ class PerAccountStore extends ChangeNotifier { required InitialSnapshot initialSnapshot, }) : zulipVersion = initialSnapshot.zulipVersion, subscriptions = Map.fromEntries(initialSnapshot.subscriptions.map( - (subscription) => MapEntry(subscription.streamId, subscription))); + (subscription) => MapEntry(subscription.streamId, subscription))), + maxFileUploadSizeMib = initialSnapshot.maxFileUploadSizeMib; final Account account; final ApiConnection connection; final String zulipVersion; final Map subscriptions; + final int maxFileUploadSizeMib; // No event for this. // TODO lots more data. When adding, be sure to update handleEvent too. diff --git a/test/example_data.dart b/test/example_data.dart index 2c0376aade..15581891f5 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -80,4 +80,5 @@ final InitialSnapshot initialSnapshot = InitialSnapshot( alertWords: ['klaxon'], customProfileFields: [], subscriptions: [], // TODO add subscriptions to example initial snapshot + maxFileUploadSizeMib: 25, ); From 1650f0defe316db131f1020f85ae26348dbe9429 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 12 Apr 2023 13:36:47 -0700 Subject: [PATCH 06/11] api: Add ApiConnection.postFileFromStream --- lib/api/core.dart | 15 +++++++++++++++ test/api/fake_api.dart | 7 +++++++ 2 files changed, 22 insertions(+) diff --git a/lib/api/core.dart b/lib/api/core.dart index a59b40b93a..a3464810c7 100644 --- a/lib/api/core.dart +++ b/lib/api/core.dart @@ -37,6 +37,8 @@ abstract class ApiConnection { Future get(String route, Map? params); Future post(String route, Map? params); + + Future postFileFromStream(String route, Stream> content, int length, { String? filename }); } // TODO memoize @@ -95,6 +97,19 @@ class LiveApiConnection extends ApiConnection { } return utf8.decode(response.bodyBytes); } + + @override + Future postFileFromStream(String route, Stream> content, int length, { String? filename }) async { + assert(_isOpen); + http.MultipartRequest request = http.MultipartRequest('POST', Uri.parse("${auth.realmUrl}/api/v1/$route")) + ..files.add(http.MultipartFile('file', content, length, filename: filename)) + ..headers.addAll(_headers()); + final response = await http.Response.fromStream(await _client.send(request)); + if (response.statusCode != 200) { + throw Exception("error on file-upload POST $route: status ${response.statusCode}"); + } + return utf8.decode(response.bodyBytes); + } } Map? encodeParameters(Map? params) { diff --git a/test/api/fake_api.dart b/test/api/fake_api.dart index a0533c4ea0..63d6bd1ccf 100644 --- a/test/api/fake_api.dart +++ b/test/api/fake_api.dart @@ -38,6 +38,13 @@ class FakeApiConnection extends ApiConnection { _nextResponse = null; return response!; } + + @override + Future postFileFromStream(String route, Stream> content, int length, { String? filename }) async { + final response = _nextResponse; + _nextResponse = null; + return response!; + } } const String _fakeApiKey = 'fake-api-key'; From f9139e4c22552c1043fb2db568d496ca3fb7011f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 12 Apr 2023 14:45:11 -0700 Subject: [PATCH 07/11] api: Add upload-file route --- lib/api/route/messages.dart | 25 +++++++++++++++++++++++++ lib/api/route/messages.g.dart | 10 ++++++++++ 2 files changed, 35 insertions(+) diff --git a/lib/api/route/messages.dart b/lib/api/route/messages.dart index 62f4d0b711..0fc4db27fe 100644 --- a/lib/api/route/messages.dart +++ b/lib/api/route/messages.dart @@ -97,3 +97,28 @@ class SendMessageResult { Map toJson() => _$SendMessageResultToJson(this); } + +/// https://zulip.com/api/upload-file +Future uploadFile( + ApiConnection connection, { + required Stream> content, + required int length, + required String filename, +}) async { + final data = await connection.postFileFromStream('user_uploads', content, length, filename: filename); + return UploadFileResult.fromJson(jsonDecode(data)); +} + +@JsonSerializable(fieldRename: FieldRename.snake) +class UploadFileResult { + final String uri; + + UploadFileResult({ + required this.uri, + }); + + factory UploadFileResult.fromJson(Map json) => + _$UploadFileResultFromJson(json); + + Map toJson() => _$UploadFileResultToJson(this); +} diff --git a/lib/api/route/messages.g.dart b/lib/api/route/messages.g.dart index fe929c9278..dae949c63b 100644 --- a/lib/api/route/messages.g.dart +++ b/lib/api/route/messages.g.dart @@ -39,3 +39,13 @@ Map _$SendMessageResultToJson(SendMessageResult instance) => 'id': instance.id, 'deliver_at': instance.deliverAt, }; + +UploadFileResult _$UploadFileResultFromJson(Map json) => + UploadFileResult( + uri: json['uri'] as String, + ); + +Map _$UploadFileResultToJson(UploadFileResult instance) => + { + 'uri': instance.uri, + }; From 5dca3359b97437ef6aceb80b8b044f697c6a4562 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Wed, 12 Apr 2023 13:47:11 -0700 Subject: [PATCH 08/11] compose [nfc]: Rename ContentTextEditingController's listener TopicTextEditingController and ContentTextEditingController inherit from ValueNotifier, so they have a `value` property, and their listeners are notified when that property changes. They don't currently notify their listeners in any other cases, so the listeners are reasonable to have "value" in their names. But for ContentTextEditingController, soon we'll want to call notifyListeners for something other than a change in `value`: in particular, it'll have a stateful property to track in-progress file uploads, and that property will affect `validationErrors`, so we'll call `notifyListeners` when a file upload starts or ends. So, we give its listener this more general name. --- lib/widgets/compose_box.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 69ba6ded39..a445533af7 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -165,7 +165,7 @@ class _StreamSendButtonState extends State<_StreamSendButton> { } } - _contentValueChanged() { + _contentChanged() { final oldIsEmpty = _contentValidationErrors.isEmpty; final newErrors = widget.contentController.validationErrors(); final newIsEmpty = newErrors.isEmpty; @@ -183,13 +183,13 @@ class _StreamSendButtonState extends State<_StreamSendButton> { _topicValidationErrors = widget.topicController.validationErrors(); _contentValidationErrors = widget.contentController.validationErrors(); widget.topicController.addListener(_topicValueChanged); - widget.contentController.addListener(_contentValueChanged); + widget.contentController.addListener(_contentChanged); } @override void dispose() { widget.topicController.removeListener(_topicValueChanged); - widget.contentController.removeListener(_contentValueChanged); + widget.contentController.removeListener(_contentChanged); super.dispose(); } From e221d4645953e6f5bc3390faa3b56f85bf0057c4 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 14 Apr 2023 13:30:33 -0700 Subject: [PATCH 09/11] compose [nfc]: Rename TopicTextEditingController's listeners As we did for ContentTextEditingController's listener in the previous commit. As Greg pointed out in reviewing the previous commit: https://github.com/zulip/zulip-flutter/pull/63#discussion_r1166047338 > Perhaps apply the same rename to the topic controller too. We > won't currently be using that extra generality, but in principle > there's no reason we wouldn't. --- lib/widgets/compose_box.dart | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index a445533af7..0bbaaf20cf 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -92,7 +92,7 @@ class _StreamContentInput extends StatefulWidget { class _StreamContentInputState extends State<_StreamContentInput> { late String _topicTextNormalized; - _topicValueChanged() { + _topicChanged() { setState(() { _topicTextNormalized = widget.topicController.textNormalized(); }); @@ -102,12 +102,12 @@ class _StreamContentInputState extends State<_StreamContentInput> { void initState() { super.initState(); _topicTextNormalized = widget.topicController.textNormalized(); - widget.topicController.addListener(_topicValueChanged); + widget.topicController.addListener(_topicChanged); } @override void dispose() { - widget.topicController.removeListener(_topicValueChanged); + widget.topicController.removeListener(_topicChanged); super.dispose(); } @@ -153,7 +153,7 @@ class _StreamSendButtonState extends State<_StreamSendButton> { late List _topicValidationErrors; late List _contentValidationErrors; - _topicValueChanged() { + _topicChanged() { final oldIsEmpty = _topicValidationErrors.isEmpty; final newErrors = widget.topicController.validationErrors(); final newIsEmpty = newErrors.isEmpty; @@ -182,13 +182,13 @@ class _StreamSendButtonState extends State<_StreamSendButton> { super.initState(); _topicValidationErrors = widget.topicController.validationErrors(); _contentValidationErrors = widget.contentController.validationErrors(); - widget.topicController.addListener(_topicValueChanged); + widget.topicController.addListener(_topicChanged); widget.contentController.addListener(_contentChanged); } @override void dispose() { - widget.topicController.removeListener(_topicValueChanged); + widget.topicController.removeListener(_topicChanged); widget.contentController.removeListener(_contentChanged); super.dispose(); } From 0c029b5cbfc75548715694e2c2f91ec042588b11 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 17 Apr 2023 13:10:06 -0700 Subject: [PATCH 10/11] compose [nfc]: Add _contentFocusNode and pass to TextField This will let us query and request focus on the content input, which we'd like to do soon. (When inserting upload-file markdown, we'd like to focus the content input if it wasn't already focused.) --- lib/widgets/compose_box.dart | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 0bbaaf20cf..44fd893ea0 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -80,10 +80,15 @@ class ContentTextEditingController extends TextEditingController { /// The content input for StreamComposeBox. class _StreamContentInput extends StatefulWidget { - const _StreamContentInput({required this.controller, required this.topicController}); + const _StreamContentInput({ + required this.controller, + required this.topicController, + required this.focusNode, + }); final ContentTextEditingController controller; final TopicTextEditingController topicController; + final FocusNode focusNode; @override State<_StreamContentInput> createState() => _StreamContentInputState(); @@ -126,6 +131,7 @@ class _StreamContentInputState extends State<_StreamContentInput> { ), child: TextField( controller: widget.controller, + focusNode: widget.focusNode, style: TextStyle(color: colorScheme.onSurface), decoration: InputDecoration.collapsed( hintText: "Message #test here > $_topicTextNormalized", @@ -268,11 +274,13 @@ class StreamComposeBox extends StatefulWidget { class _StreamComposeBoxState extends State { final _topicController = TopicTextEditingController(); final _contentController = ContentTextEditingController(); + final _contentFocusNode = FocusNode(); @override void dispose() { _topicController.dispose(); _contentController.dispose(); + _contentFocusNode.dispose(); super.dispose(); } @@ -318,8 +326,11 @@ class _StreamComposeBoxState extends State { children: [ topicInput, const SizedBox(height: 8), - _StreamContentInput(topicController: _topicController, controller: _contentController), - ]))), + _StreamContentInput( + topicController: _topicController, + controller: _contentController, + focusNode: _contentFocusNode), + ]))), const SizedBox(width: 8), _StreamSendButton(topicController: _topicController, contentController: _contentController), ])))); From cb256434c6d87691c293dcb520df394c9db305e2 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 11 Apr 2023 13:28:03 -0700 Subject: [PATCH 11/11] compose: Prototype upload-file UI Fixes: #57 --- lib/widgets/compose_box.dart | 194 +++++++++++++++++++++++++++++++---- 1 file changed, 176 insertions(+), 18 deletions(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 44fd893ea0..76da7eae2e 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -1,3 +1,4 @@ +import 'package:file_picker/file_picker.dart'; import 'package:flutter/material.dart'; import 'dialog.dart'; @@ -44,9 +45,10 @@ class TopicTextEditingController extends TextEditingController { enum ContentValidationError { empty, - tooLong; + tooLong, + uploadInProgress; - // Later: upload in progress; quote-and-reply in progress + // Later: quote-and-reply in progress String message() { switch (this) { @@ -54,11 +56,73 @@ enum ContentValidationError { return "Message length shouldn't be greater than 10000 characters."; case ContentValidationError.empty: return 'You have nothing to send!'; + case ContentValidationError.uploadInProgress: + return 'Please wait for the upload to complete.'; } } } class ContentTextEditingController extends TextEditingController { + int _nextUploadTag = 0; + + final Map _uploads = {}; + + /// A probably-reasonable place to insert Markdown, such as for a file upload. + /// + /// Gives the cursor position, + /// or if text is selected, the end of the selection range. + /// + /// If there isn't a cursor position or a text selection + /// (e.g., when the input has never been focused), + /// gives the end of the whole text. + /// + /// Expressed as a collapsed `TextRange` at the index. + TextRange _insertionIndex() { + final TextRange selection = value.selection; + final String text = value.text; + return selection.isValid + ? (selection.isCollapsed + ? selection + : TextRange.collapsed(selection.end)) + : TextRange.collapsed(text.length); + } + + /// Tells the controller that a file upload has started. + /// + /// Returns an int "tag" that should be passed to registerUploadEnd on the + /// upload's success or failure. + int registerUploadStart(String filename) { + final tag = _nextUploadTag; + _nextUploadTag += 1; + final placeholder = '[Uploading $filename...]()'; // TODO(i18n) + _uploads[tag] = (filename: filename, placeholder: placeholder); + notifyListeners(); // _uploads change could affect validationErrors + value = value.replaced(_insertionIndex(), '$placeholder\n\n'); + return tag; + } + + /// Tells the controller that a file upload has ended, with success or error. + /// + /// To indicate success, pass the URL to be used for the Markdown link. + /// If `url` is null, failure is assumed. + void registerUploadEnd(int tag, Uri? url) { + final val = _uploads[tag]; + assert(val != null, 'registerUploadEnd called twice for same tag'); + final (:filename, :placeholder) = val!; + final int startIndex = text.indexOf(placeholder); + final replacementRange = startIndex >= 0 + ? TextRange(start: startIndex, end: startIndex + placeholder.length) + : _insertionIndex(); + + value = value.replaced( + replacementRange, + url == null + ? '[Failed to upload file: $filename]()' // TODO(i18n) + : '[$filename](${url.toString()})'); + _uploads.remove(tag); + notifyListeners(); // _uploads change could affect validationErrors + } + String textNormalized() { return text.trim(); } @@ -74,6 +138,9 @@ class ContentTextEditingController extends TextEditingController { // be conservative and may cut the user off shorter than necessary. if (normalized.length > kMaxMessageLengthCodePoints) ContentValidationError.tooLong, + + if (_uploads.isNotEmpty) + ContentValidationError.uploadInProgress, ]; } } @@ -143,6 +210,87 @@ class _StreamContentInputState extends State<_StreamContentInput> { } } +class _AttachFileButton extends StatelessWidget { + const _AttachFileButton({required this.contentController, required this.contentFocusNode}); + + final ContentTextEditingController contentController; + final FocusNode contentFocusNode; + + _handlePress(BuildContext context) async { + FilePickerResult? result; + try { + result = await FilePicker.platform.pickFiles(allowMultiple: true, withReadStream: true); + } catch (e) { + // TODO(i18n) + showErrorDialog(context: context, title: 'Error', message: e.toString()); + return; + } + if (result == null) { + return; // User cancelled; do nothing + } + + if (context.mounted) {} // https://github.com/dart-lang/linter/issues/4007 + else { + return; + } + + final store = PerAccountStoreWidget.of(context); + + final List tooLargeFiles = []; + final List rightSizeFiles = []; + for (PlatformFile file in result.files) { + if ((file.size / (1 << 20)) > store.maxFileUploadSizeMib) { + tooLargeFiles.add(file); + } else { + rightSizeFiles.add(file); + } + } + + if (tooLargeFiles.isNotEmpty) { + final listMessage = tooLargeFiles + .map((file) => '${file.name}: ${(file.size / (1 << 20)).toStringAsFixed(1)} MiB') + .join('\n'); + showErrorDialog( // TODO(i18n) + context: context, + title: 'File(s) too large', + message: + '${tooLargeFiles.length} file(s) are larger than the server\'s limit of ${store.maxFileUploadSizeMib} MiB and will not be uploaded:\n\n$listMessage'); + } + + final List<(int, PlatformFile)> uploadsInProgress = []; + for (final file in rightSizeFiles) { + final tag = contentController.registerUploadStart(file.name); + uploadsInProgress.add((tag, file)); + } + if (!contentFocusNode.hasFocus) { + contentFocusNode.requestFocus(); + } + + for (final (tag, file) in uploadsInProgress) { + final PlatformFile(:readStream, :size, :name) = file; + assert(readStream != null); // We passed `withReadStream: true` to pickFiles. + Uri? url; + try { + final result = await uploadFile(store.connection, + content: readStream!, length: size, filename: name); + url = Uri.parse(result.uri); + } catch (e) { + if (!context.mounted) return; + // TODO(#37): Specifically handle `413 Payload Too Large` + // TODO(#37): On API errors, quote `msg` from server, with "The server said:" + showErrorDialog(context: context, + title: 'Failed to upload file: $name', message: e.toString()); + } finally { + contentController.registerUploadEnd(tag, url); + } + } + } + + @override + Widget build(BuildContext context) { + return IconButton(icon: const Icon(Icons.attach_file), onPressed: () => _handlePress(context)); + } +} /// The send button for StreamComposeBox. class _StreamSendButton extends StatefulWidget { @@ -318,21 +466,31 @@ class _StreamComposeBoxState extends State { minimum: const EdgeInsets.fromLTRB(8, 0, 8, 8), child: Padding( padding: const EdgeInsets.only(top: 8.0), - child: Row(crossAxisAlignment: CrossAxisAlignment.end, children: [ - Expanded( - child: Theme( - data: inputThemeData, - child: Column( - children: [ - topicInput, - const SizedBox(height: 8), - _StreamContentInput( - topicController: _topicController, - controller: _contentController, - focusNode: _contentFocusNode), - ]))), - const SizedBox(width: 8), - _StreamSendButton(topicController: _topicController, contentController: _contentController), - ])))); + child: Column( + children: [ + Row(crossAxisAlignment: CrossAxisAlignment.end, children: [ + Expanded( + child: Theme( + data: inputThemeData, + child: Column( + children: [ + topicInput, + const SizedBox(height: 8), + _StreamContentInput( + topicController: _topicController, + controller: _contentController, + focusNode: _contentFocusNode), + ]))), + const SizedBox(width: 8), + _StreamSendButton(topicController: _topicController, contentController: _contentController), + ]), + Theme( + data: themeData.copyWith( + iconTheme: themeData.iconTheme.copyWith(color: colorScheme.onSurfaceVariant)), + child: Row( + children: [ + _AttachFileButton(contentController: _contentController, contentFocusNode: _contentFocusNode), + ])), + ])))); } }