Skip to content

Commit af6fe45

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 bb7c69d commit af6fe45

10 files changed

+194
-0
lines changed

assets/l10n/app_en.arb

+8
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,14 @@
458458
"@topicValidationErrorMandatoryButEmpty": {
459459
"description": "Topic validation error when topic is required but was empty."
460460
},
461+
"errorContentNotInsertedTitle": "Content not inserted",
462+
"@errorContentNotInsertedTitle": {
463+
"description": "Title for error dialog when an attempt to insert rich content failed."
464+
},
465+
"errorContentToInsertIsEmpty": "The file to be inserted is empty or cannot be accessed.",
466+
"@errorContentToInsertIsEmpty": {
467+
"description": "Error message when the rich content to be inserted is empty or cannot be accessed."
468+
},
461469
"errorInvalidResponse": "The server sent an invalid response",
462470
"@errorInvalidResponse": {
463471
"description": "Error message when an API call returned an invalid response."

lib/generated/l10n/zulip_localizations.dart

+12
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,18 @@ abstract class ZulipLocalizations {
721721
/// **'Topics are required in this organization.'**
722722
String get topicValidationErrorMandatoryButEmpty;
723723

724+
/// Title for error dialog when an attempt to insert rich content failed.
725+
///
726+
/// In en, this message translates to:
727+
/// **'Content not inserted'**
728+
String get errorContentNotInsertedTitle;
729+
730+
/// Error message when the rich content to be inserted is empty or cannot be accessed.
731+
///
732+
/// In en, this message translates to:
733+
/// **'The file to be inserted is empty or cannot be accessed.'**
734+
String get errorContentToInsertIsEmpty;
735+
724736
/// Error message when an API call returned an invalid response.
725737
///
726738
/// In en, this message translates to:

lib/generated/l10n/zulip_localizations_ar.dart

+6
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,12 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
357357
@override
358358
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
359359

360+
@override
361+
String get errorContentNotInsertedTitle => 'Content not inserted';
362+
363+
@override
364+
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';
365+
360366
@override
361367
String get errorInvalidResponse => 'The server sent an invalid response';
362368

lib/generated/l10n/zulip_localizations_en.dart

+6
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,12 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
357357
@override
358358
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
359359

360+
@override
361+
String get errorContentNotInsertedTitle => 'Content not inserted';
362+
363+
@override
364+
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';
365+
360366
@override
361367
String get errorInvalidResponse => 'The server sent an invalid response';
362368

lib/generated/l10n/zulip_localizations_fr.dart

+6
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,12 @@ class ZulipLocalizationsFr extends ZulipLocalizations {
357357
@override
358358
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
359359

360+
@override
361+
String get errorContentNotInsertedTitle => 'Content not inserted';
362+
363+
@override
364+
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';
365+
360366
@override
361367
String get errorInvalidResponse => 'The server sent an invalid response';
362368

lib/generated/l10n/zulip_localizations_ja.dart

+6
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,12 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
357357
@override
358358
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
359359

360+
@override
361+
String get errorContentNotInsertedTitle => 'Content not inserted';
362+
363+
@override
364+
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';
365+
360366
@override
361367
String get errorInvalidResponse => 'The server sent an invalid response';
362368

lib/generated/l10n/zulip_localizations_pl.dart

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

360+
@override
361+
String get errorContentNotInsertedTitle => 'Content not inserted';
362+
363+
@override
364+
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';
365+
360366
@override
361367
String get errorInvalidResponse => 'Nieprawidłowa odpowiedź serwera';
362368

lib/generated/l10n/zulip_localizations_ru.dart

+6
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,12 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
357357
@override
358358
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
359359

360+
@override
361+
String get errorContentNotInsertedTitle => 'Content not inserted';
362+
363+
@override
364+
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';
365+
360366
@override
361367
String get errorInvalidResponse => 'The server sent an invalid response';
362368

lib/widgets/compose_box.dart

+37
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';
@@ -362,6 +363,40 @@ class _ContentInputState extends State<_ContentInput> with WidgetsBindingObserve
362363
}
363364
}
364365

366+
void _handleContentInserted(KeyboardInsertedContent content) async {
367+
if (content.data == null || content.data!.isEmpty) {
368+
// As of writing, the engine implementation never leaves `content.data` as
369+
// `null`, but ideally it should be when the data cannot be read for
370+
// errors.
371+
//
372+
// When `content.data` is empty, the data is not literally empty — this
373+
// can also happen when the data can't be read from the input stream
374+
// provided by the Android SDK because of an IO exception.
375+
//
376+
// See Flutter engine implementation that prepares this data:
377+
// https://github.com/flutter/flutter/blob/0ffc4ce00ea7bb912e379adf39354644eab2c17e/engine/src/flutter/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java#L497-L548
378+
// TODO(upstream): improve the API for this
379+
final zulipLocalizations = ZulipLocalizations.of(context);
380+
showErrorDialog(
381+
context: context,
382+
title: zulipLocalizations.errorContentNotInsertedTitle,
383+
message: zulipLocalizations.errorContentToInsertIsEmpty);
384+
return;
385+
}
386+
387+
final file = _File(
388+
content: Stream.fromIterable([content.data!]),
389+
length: content.data!.length,
390+
filename: path.basename(content.uri),
391+
mimeType: content.mimeType);
392+
393+
await _uploadFiles(
394+
context: context,
395+
contentController: widget.controller.content,
396+
contentFocusNode: widget.controller.contentFocusNode,
397+
files: [file]);
398+
}
399+
365400
static double maxHeight(BuildContext context) {
366401
final clampingTextScaler = MediaQuery.textScalerOf(context)
367402
.clamp(maxScaleFactor: 1.5);
@@ -405,6 +440,8 @@ class _ContentInputState extends State<_ContentInput> with WidgetsBindingObserve
405440
child: TextField(
406441
controller: widget.controller.content,
407442
focusNode: widget.controller.contentFocusNode,
443+
contentInsertionConfiguration: ContentInsertionConfiguration(
444+
onContentInserted: _handleContentInserted),
408445
// Let the content show through the `contentPadding` so that
409446
// our [InsetShadowBox] can fade it smoothly there.
410447
clipBehavior: Clip.none,

test/widgets/compose_box_test.dart

+101
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import 'dart:convert';
33

44
import 'package:checks/checks.dart';
55
import 'package:file_picker/file_picker.dart';
6+
import 'package:flutter/services.dart';
67
import 'package:flutter_checks/flutter_checks.dart';
78
import 'package:http/http.dart' as http;
89
import 'package:flutter/material.dart';
@@ -575,6 +576,106 @@ void main() {
575576

576577
// TODO test what happens when capturing/uploading fails
577578
});
579+
580+
group('attach from keyboard', () {
581+
// This is adapted from:
582+
// https://github.com/flutter/flutter/blob/0ffc4ce00ea7bb912e379adf39354644eab2c17e/packages/flutter/test/widgets/editable_text_test.dart#L724-L740
583+
Future<void> insertContentFromKeyboard(WidgetTester tester, {
584+
required List<int>? data,
585+
required String attachedFileUrl,
586+
required String mimeType,
587+
}) async {
588+
await tester.showKeyboard(contentInputFinder);
589+
// This invokes [EditableText.performAction] on the content [TextField],
590+
// which did not expose an API for testing.
591+
// TODO(upstream): support a better API for testing this
592+
await tester.binding.defaultBinaryMessenger.handlePlatformMessage(
593+
SystemChannels.textInput.name,
594+
SystemChannels.textInput.codec.encodeMethodCall(
595+
MethodCall('TextInputClient.performAction', <dynamic>[
596+
-1,
597+
'TextInputAction.commitContent',
598+
// This fakes data originally provided by the Flutter engine:
599+
// https://github.com/flutter/flutter/blob/0ffc4ce00ea7bb912e379adf39354644eab2c17e/engine/src/flutter/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java#L497-L548
600+
{
601+
"mimeType": mimeType,
602+
"data": data,
603+
"uri": attachedFileUrl,
604+
},
605+
])),
606+
(ByteData? data) {});
607+
}
608+
609+
testWidgets('success', (tester) async {
610+
const fileContent = [1, 0, 1, 0, 0];
611+
await prepare(tester);
612+
const uploadUrl = '/user_uploads/1/4e/m2A3MSqFnWRLUf9SaPzQ0Up_/test.gif';
613+
connection.prepare(json: UploadFileResult(uri: uploadUrl).toJson());
614+
await insertContentFromKeyboard(tester,
615+
data: fileContent,
616+
attachedFileUrl:
617+
'content://com.zulip.android.zulipboard.provider'
618+
'/root/com.zulip.android.zulipboard/candidate_temp/test.gif',
619+
mimeType: 'image/gif');
620+
621+
await tester.pump();
622+
check(controller!.content.text)
623+
.equals('see image: [Uploading test.gif…]()\n\n');
624+
// (the request is checked more thoroughly in API tests)
625+
check(connection.lastRequest!).isA<http.MultipartRequest>()
626+
..method.equals('POST')
627+
..files.single.which((it) => it
628+
..field.equals('file')
629+
..length.equals(fileContent.length)
630+
..filename.equals('test.gif')
631+
..contentType.asString.equals('image/gif')
632+
..has<Future<List<int>>>((f) => f.finalize().toBytes(), 'contents')
633+
.completes((it) => it.deepEquals(fileContent))
634+
);
635+
checkAppearsLoading(tester, true);
636+
637+
await tester.pump(Duration.zero);
638+
check(controller!.content.text)
639+
.equals('see image: [test.gif]($uploadUrl)\n\n');
640+
checkAppearsLoading(tester, false);
641+
});
642+
643+
testWidgets('data is null', (tester) async {
644+
await prepare(tester);
645+
await insertContentFromKeyboard(tester,
646+
data: null,
647+
attachedFileUrl:
648+
'content://com.zulip.android.zulipboard.provider'
649+
'/root/com.zulip.android.zulipboard/candidate_temp/test.gif',
650+
mimeType: 'image/jpeg');
651+
652+
await tester.pump();
653+
check(controller!.content.text).equals('see image: ');
654+
check(connection.takeRequests()).isEmpty();
655+
checkErrorDialog(tester,
656+
expectedTitle: 'Content not inserted',
657+
expectedMessage: 'The file to be inserted is empty or cannot be accessed.');
658+
checkAppearsLoading(tester, false);
659+
});
660+
661+
testWidgets('data is empty', (tester) async {
662+
await prepare(tester);
663+
await insertContentFromKeyboard(tester,
664+
data: [],
665+
attachedFileUrl:
666+
'content://com.zulip.android.zulipboard.provider'
667+
'/root/com.zulip.android.zulipboard/candidate_temp/test.gif',
668+
mimeType: 'image/jpeg');
669+
670+
await tester.pump();
671+
check(controller!.content.text).equals('see image: ');
672+
check(connection.takeRequests()).isEmpty();
673+
checkErrorDialog(tester,
674+
expectedTitle: 'Content not inserted',
675+
expectedMessage: 'The file to be inserted is empty or cannot be accessed.');
676+
checkAppearsLoading(tester, false);
677+
});
678+
});
578679
});
579680

580681
group('error banner', () {

0 commit comments

Comments
 (0)