Skip to content

Commit 56008b1

Browse files
committed
compose: Support images from keyboard for Android
Fixes: zulip#419 Fixes: zulip#1173 Signed-off-by: Zixuan James Li <[email protected]>
1 parent de2b4f6 commit 56008b1

11 files changed

+199
-0
lines changed

assets/l10n/app_en.arb

+8
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,14 @@
522522
"@topicValidationErrorMandatoryButEmpty": {
523523
"description": "Topic validation error when topic is required but was empty."
524524
},
525+
"errorContentNotInsertedTitle": "Content not inserted",
526+
"@errorContentNotInsertedTitle": {
527+
"description": "Title for error dialog when an attempt to insert rich content failed."
528+
},
529+
"errorContentToInsertIsEmpty": "The file to be inserted is empty or cannot be accessed.",
530+
"@errorContentToInsertIsEmpty": {
531+
"description": "Error message when the rich content to be inserted is empty or cannot be accessed."
532+
},
525533
"errorInvalidApiKeyMessage": "Your account at {url} could not be authenticated. Please try logging in again or use another account.",
526534
"@errorInvalidApiKeyMessage": {
527535
"description": "Error message in the dialog for invalid API key.",

lib/generated/l10n/zulip_localizations.dart

+12
Original file line numberDiff line numberDiff line change
@@ -801,6 +801,18 @@ abstract class ZulipLocalizations {
801801
/// **'Topics are required in this organization.'**
802802
String get topicValidationErrorMandatoryButEmpty;
803803

804+
/// Title for error dialog when an attempt to insert rich content failed.
805+
///
806+
/// In en, this message translates to:
807+
/// **'Content not inserted'**
808+
String get errorContentNotInsertedTitle;
809+
810+
/// Error message when the rich content to be inserted is empty or cannot be accessed.
811+
///
812+
/// In en, this message translates to:
813+
/// **'The file to be inserted is empty or cannot be accessed.'**
814+
String get errorContentToInsertIsEmpty;
815+
804816
/// Error message in the dialog for invalid API key.
805817
///
806818
/// In en, this message translates to:

lib/generated/l10n/zulip_localizations_ar.dart

+6
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,12 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
404404
@override
405405
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
406406

407+
@override
408+
String get errorContentNotInsertedTitle => 'Content not inserted';
409+
410+
@override
411+
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';
412+
407413
@override
408414
String errorInvalidApiKeyMessage(String url) {
409415
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';

lib/generated/l10n/zulip_localizations_en.dart

+6
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,12 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
404404
@override
405405
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
406406

407+
@override
408+
String get errorContentNotInsertedTitle => 'Content not inserted';
409+
410+
@override
411+
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';
412+
407413
@override
408414
String errorInvalidApiKeyMessage(String url) {
409415
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';

lib/generated/l10n/zulip_localizations_ja.dart

+6
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,12 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
404404
@override
405405
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
406406

407+
@override
408+
String get errorContentNotInsertedTitle => 'Content not inserted';
409+
410+
@override
411+
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';
412+
407413
@override
408414
String errorInvalidApiKeyMessage(String url) {
409415
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';

lib/generated/l10n/zulip_localizations_nb.dart

+6
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,12 @@ class ZulipLocalizationsNb extends ZulipLocalizations {
404404
@override
405405
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
406406

407+
@override
408+
String get errorContentNotInsertedTitle => 'Content not inserted';
409+
410+
@override
411+
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';
412+
407413
@override
408414
String errorInvalidApiKeyMessage(String url) {
409415
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';

lib/generated/l10n/zulip_localizations_pl.dart

+6
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,12 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
404404
@override
405405
String get topicValidationErrorMandatoryButEmpty => 'Wątki są wymagane przez tę organizację.';
406406

407+
@override
408+
String get errorContentNotInsertedTitle => 'Content not inserted';
409+
410+
@override
411+
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';
412+
407413
@override
408414
String errorInvalidApiKeyMessage(String url) {
409415
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';

lib/generated/l10n/zulip_localizations_ru.dart

+6
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,12 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
404404
@override
405405
String get topicValidationErrorMandatoryButEmpty => 'Темы обязательны в этой организации.';
406406

407+
@override
408+
String get errorContentNotInsertedTitle => 'Content not inserted';
409+
410+
@override
411+
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';
412+
407413
@override
408414
String errorInvalidApiKeyMessage(String url) {
409415
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';

lib/generated/l10n/zulip_localizations_sk.dart

+6
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,12 @@ class ZulipLocalizationsSk extends ZulipLocalizations {
404404
@override
405405
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
406406

407+
@override
408+
String get errorContentNotInsertedTitle => 'Content not inserted';
409+
410+
@override
411+
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';
412+
407413
@override
408414
String errorInvalidApiKeyMessage(String url) {
409415
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';

lib/widgets/compose_box.dart

+36
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import 'package:app_settings/app_settings.dart';
44
import 'package:flutter/material.dart';
55
import 'package:flutter/services.dart';
66
import 'package:mime/mime.dart';
7+
import 'package:path/path.dart' as path;
78

89
import '../api/exception.dart';
910
import '../api/model/model.dart';
@@ -465,6 +466,39 @@ class _ContentInputState extends State<_ContentInput> with WidgetsBindingObserve
465466
}
466467
}
467468

469+
void _handleContentInserted(KeyboardInsertedContent content) async {
470+
if (content.data == null || content.data!.isEmpty) {
471+
// As of writing, the engine implementation never leaves `content.data` as
472+
// `null`, but ideally it should be when the data cannot be read for
473+
// errors.
474+
//
475+
// When `content.data` is empty, the data is not literally empty — this
476+
// can also happen when the data can't be read from the input stream
477+
// provided by the Android SDK because of an IO exception.
478+
//
479+
// See Flutter engine implementation that prepares this data:
480+
// https://github.com/flutter/flutter/blob/0ffc4ce00/engine/src/flutter/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java#L497-L548
481+
// TODO(upstream): improve the API for this
482+
final zulipLocalizations = ZulipLocalizations.of(context);
483+
showErrorDialog(context: context,
484+
title: zulipLocalizations.errorContentNotInsertedTitle,
485+
message: zulipLocalizations.errorContentToInsertIsEmpty);
486+
return;
487+
}
488+
489+
final file = _File(
490+
content: Stream.fromIterable([content.data!]),
491+
length: content.data!.length,
492+
filename: path.basename(content.uri),
493+
mimeType: content.mimeType);
494+
495+
await _uploadFiles(
496+
context: context,
497+
contentController: widget.controller.content,
498+
contentFocusNode: widget.controller.contentFocusNode,
499+
files: [file]);
500+
}
501+
468502
static double maxHeight(BuildContext context) {
469503
final clampingTextScaler = MediaQuery.textScalerOf(context)
470504
.clamp(maxScaleFactor: 1.5);
@@ -508,6 +542,8 @@ class _ContentInputState extends State<_ContentInput> with WidgetsBindingObserve
508542
child: TextField(
509543
controller: widget.controller.content,
510544
focusNode: widget.controller.contentFocusNode,
545+
contentInsertionConfiguration: ContentInsertionConfiguration(
546+
onContentInserted: _handleContentInserted),
511547
// Let the content show through the `contentPadding` so that
512548
// our [InsetShadowBox] can fade it smoothly there.
513549
clipBehavior: Clip.none,

test/widgets/compose_box_test.dart

+101
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import 'dart:io';
44

55
import 'package:checks/checks.dart';
66
import 'package:file_picker/file_picker.dart';
7+
import 'package:flutter/services.dart';
78
import 'package:flutter_checks/flutter_checks.dart';
89
import 'package:http/http.dart' as http;
910
import 'package:flutter/material.dart';
@@ -831,6 +832,106 @@ void main() {
831832
// target platform the test is simulating.
832833
// TODO(upstream): unskip after fix to https://github.com/flutter/flutter/issues/161073
833834
skip: Platform.isWindows);
835+
836+
group('attach from keyboard', () {
837+
// This is adapted from:
838+
// https://github.com/flutter/flutter/blob/0ffc4ce00/packages/flutter/test/widgets/editable_text_test.dart#L724-L740
839+
Future<void> insertContentFromKeyboard(WidgetTester tester, {
840+
required List<int>? data,
841+
required String attachedFileUrl,
842+
required String mimeType,
843+
}) async {
844+
await tester.showKeyboard(contentInputFinder);
845+
// This invokes [EditableText.performAction] on the content [TextField],
846+
// which did not expose an API for testing.
847+
// TODO(upstream): support a better API for testing this
848+
await tester.binding.defaultBinaryMessenger.handlePlatformMessage(
849+
SystemChannels.textInput.name,
850+
SystemChannels.textInput.codec.encodeMethodCall(
851+
MethodCall('TextInputClient.performAction', <dynamic>[
852+
-1,
853+
'TextInputAction.commitContent',
854+
// This fakes data originally provided by the Flutter engine:
855+
// https://github.com/flutter/flutter/blob/0ffc4ce00/engine/src/flutter/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java#L497-L548
856+
{
857+
"mimeType": mimeType,
858+
"data": data,
859+
"uri": attachedFileUrl,
860+
},
861+
])),
862+
(ByteData? data) {});
863+
}
864+
865+
testWidgets('success', (tester) async {
866+
const fileContent = [1, 0, 1, 0, 0];
867+
await prepare(tester);
868+
const uploadUrl = '/user_uploads/1/4e/m2A3MSqFnWRLUf9SaPzQ0Up_/test.gif';
869+
connection.prepare(json: UploadFileResult(uri: uploadUrl).toJson());
870+
await insertContentFromKeyboard(tester,
871+
data: fileContent,
872+
attachedFileUrl:
873+
'content://com.zulip.android.zulipboard.provider'
874+
'/root/com.zulip.android.zulipboard/candidate_temp/test.gif',
875+
mimeType: 'image/gif');
876+
877+
await tester.pump();
878+
check(controller!.content.text)
879+
.equals('see image: [Uploading test.gif…]()\n\n');
880+
// (the request is checked more thoroughly in API tests)
881+
check(connection.lastRequest!).isA<http.MultipartRequest>()
882+
..method.equals('POST')
883+
..files.single.which((it) => it
884+
..field.equals('file')
885+
..length.equals(fileContent.length)
886+
..filename.equals('test.gif')
887+
..contentType.asString.equals('image/gif')
888+
..has<Future<List<int>>>((f) => f.finalize().toBytes(), 'contents')
889+
.completes((it) => it.deepEquals(fileContent))
890+
);
891+
checkAppearsLoading(tester, true);
892+
893+
await tester.pump(Duration.zero);
894+
check(controller!.content.text)
895+
.equals('see image: [test.gif]($uploadUrl)\n\n');
896+
checkAppearsLoading(tester, false);
897+
});
898+
899+
testWidgets('data is null', (tester) async {
900+
await prepare(tester);
901+
await insertContentFromKeyboard(tester,
902+
data: null,
903+
attachedFileUrl:
904+
'content://com.zulip.android.zulipboard.provider'
905+
'/root/com.zulip.android.zulipboard/candidate_temp/test.gif',
906+
mimeType: 'image/jpeg');
907+
908+
await tester.pump();
909+
check(controller!.content.text).equals('see image: ');
910+
check(connection.takeRequests()).isEmpty();
911+
checkErrorDialog(tester,
912+
expectedTitle: 'Content not inserted',
913+
expectedMessage: 'The file to be inserted is empty or cannot be accessed.');
914+
checkAppearsLoading(tester, false);
915+
});
916+
917+
testWidgets('data is empty', (tester) async {
918+
await prepare(tester);
919+
await insertContentFromKeyboard(tester,
920+
data: [],
921+
attachedFileUrl:
922+
'content://com.zulip.android.zulipboard.provider'
923+
'/root/com.zulip.android.zulipboard/candidate_temp/test.gif',
924+
mimeType: 'image/jpeg');
925+
926+
await tester.pump();
927+
check(controller!.content.text).equals('see image: ');
928+
check(connection.takeRequests()).isEmpty();
929+
checkErrorDialog(tester,
930+
expectedTitle: 'Content not inserted',
931+
expectedMessage: 'The file to be inserted is empty or cannot be accessed.');
932+
checkAppearsLoading(tester, false);
933+
});
934+
});
834935
});
835936

836937
group('error banner', () {

0 commit comments

Comments
 (0)