Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compose: Support images from keyboard for Android #1203

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions assets/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,14 @@
"@topicValidationErrorMandatoryButEmpty": {
"description": "Topic validation error when topic is required but was empty."
},
"errorContentNotInsertedTitle": "Content not inserted",
"@errorContentNotInsertedTitle": {
"description": "Title for error dialog when an attempt to insert rich content failed."
},
"errorContentToInsertIsEmpty": "The file to be inserted is empty or cannot be accessed.",
"@errorContentToInsertIsEmpty": {
"description": "Error message when the rich content to be inserted is empty or cannot be accessed."
},
"errorInvalidResponse": "The server sent an invalid response",
"@errorInvalidResponse": {
"description": "Error message when an API call returned an invalid response."
Expand Down
12 changes: 12 additions & 0 deletions lib/generated/l10n/zulip_localizations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,18 @@ abstract class ZulipLocalizations {
/// **'Topics are required in this organization.'**
String get topicValidationErrorMandatoryButEmpty;

/// Title for error dialog when an attempt to insert rich content failed.
///
/// In en, this message translates to:
/// **'Content not inserted'**
String get errorContentNotInsertedTitle;

/// Error message when the rich content to be inserted is empty or cannot be accessed.
///
/// In en, this message translates to:
/// **'The file to be inserted is empty or cannot be accessed.'**
String get errorContentToInsertIsEmpty;

/// Error message when an API call returned an invalid response.
///
/// In en, this message translates to:
Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_ar.dart
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,12 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
@override
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';

@override
String get errorContentNotInsertedTitle => 'Content not inserted';

@override
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';

@override
String get errorInvalidResponse => 'The server sent an invalid response';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_en.dart
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,12 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
@override
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';

@override
String get errorContentNotInsertedTitle => 'Content not inserted';

@override
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';

@override
String get errorInvalidResponse => 'The server sent an invalid response';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_fr.dart
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,12 @@ class ZulipLocalizationsFr extends ZulipLocalizations {
@override
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';

@override
String get errorContentNotInsertedTitle => 'Content not inserted';

@override
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';

@override
String get errorInvalidResponse => 'The server sent an invalid response';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_ja.dart
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,12 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
@override
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';

@override
String get errorContentNotInsertedTitle => 'Content not inserted';

@override
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';

@override
String get errorInvalidResponse => 'The server sent an invalid response';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_pl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,12 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
@override
String get topicValidationErrorMandatoryButEmpty => 'Wątki są wymagane przez tę organizację.';

@override
String get errorContentNotInsertedTitle => 'Content not inserted';

@override
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';

@override
String get errorInvalidResponse => 'Nieprawidłowa odpowiedź serwera';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_ru.dart
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,12 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
@override
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';

@override
String get errorContentNotInsertedTitle => 'Content not inserted';

@override
String get errorContentToInsertIsEmpty => 'The file to be inserted is empty or cannot be accessed.';

@override
String get errorInvalidResponse => 'The server sent an invalid response';

Expand Down
37 changes: 37 additions & 0 deletions lib/widgets/compose_box.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import 'package:app_settings/app_settings.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:mime/mime.dart';
import 'package:path/path.dart' as path;

import '../api/exception.dart';
import '../api/model/model.dart';
Expand Down Expand Up @@ -362,6 +363,40 @@ class _ContentInputState extends State<_ContentInput> with WidgetsBindingObserve
}
}

void _handleContentInserted(KeyboardInsertedContent content) async {
if (content.data == null || content.data!.isEmpty) {
// As of writing, the engine implementation never leaves `content.data` as
// `null`, but ideally it should be when the data cannot be read for
// errors.
//
// When `content.data` is empty, the data is not literally empty — this
// can also happen when the data can't be read from the input stream
// provided by the Android SDK because of an IO exception.
//
// See Flutter engine implementation that prepares this data:
// https://github.com/flutter/flutter/blob/0ffc4ce00ea7bb912e379adf39354644eab2c17e/engine/src/flutter/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java#L497-L548
// TODO(upstream): improve the API for this
final zulipLocalizations = ZulipLocalizations.of(context);
showErrorDialog(
context: context,
title: zulipLocalizations.errorContentNotInsertedTitle,
message: zulipLocalizations.errorContentToInsertIsEmpty);
return;
}

final file = _File(
content: Stream.fromIterable([content.data!]),
length: content.data!.length,
filename: path.basename(content.uri),
mimeType: content.mimeType);

await _uploadFiles(
context: context,
contentController: widget.controller.content,
contentFocusNode: widget.controller.contentFocusNode,
files: [file]);
}

static double maxHeight(BuildContext context) {
final clampingTextScaler = MediaQuery.textScalerOf(context)
.clamp(maxScaleFactor: 1.5);
Expand Down Expand Up @@ -405,6 +440,8 @@ class _ContentInputState extends State<_ContentInput> with WidgetsBindingObserve
child: TextField(
controller: widget.controller.content,
focusNode: widget.controller.contentFocusNode,
contentInsertionConfiguration: ContentInsertionConfiguration(
onContentInserted: _handleContentInserted),
// Let the content show through the `contentPadding` so that
// our [InsetShadowBox] can fade it smoothly there.
clipBehavior: Clip.none,
Expand Down
142 changes: 118 additions & 24 deletions test/widgets/compose_box_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import 'dart:io';

import 'package:checks/checks.dart';
import 'package:file_picker/file_picker.dart';
import 'package:flutter/services.dart';
import 'package:flutter_checks/flutter_checks.dart';
import 'package:http/http.dart' as http;
import 'package:flutter/material.dart';
Expand Down Expand Up @@ -464,20 +465,24 @@ void main() {
.isA<Icon>().color.isNotNull().isSameColorAs(expectedIconColor);
}

group('attach from media library', () {
testWidgets('success', (tester) async {
TypingNotifier.debugEnable = false;
addTearDown(TypingNotifier.debugReset);
Future<void> prepare(WidgetTester tester) async {
TypingNotifier.debugEnable = false;
addTearDown(TypingNotifier.debugReset);

final channel = eg.stream();
final narrow = ChannelNarrow(channel.streamId);
await prepareComposeBox(tester, narrow: narrow, streams: [channel]);

final channel = eg.stream();
final narrow = ChannelNarrow(channel.streamId);
await prepareComposeBox(tester, narrow: narrow, streams: [channel]);
// (When we check that the send button looks disabled, it should be because
// the file is uploading, not a pre-existing reason.)
await enterTopic(tester, narrow: narrow, topic: 'some topic');
await tester.enterText(contentInputFinder, 'see image: ');
await tester.pump();
}

// (When we check that the send button looks disabled, it should be because
// the file is uploading, not a pre-existing reason.)
await enterTopic(tester, narrow: narrow, topic: 'some topic');
controller!.content.value = const TextEditingValue(text: 'see image: ');
await tester.pump();
group('attach from media library', () {
testWidgets('success', (tester) async {
await prepare(tester);
checkAppearsLoading(tester, false);

testBinding.pickFilesResult = FilePickerResult([PlatformFile(
Expand Down Expand Up @@ -526,18 +531,7 @@ void main() {

group('attach from camera', () {
testWidgets('success', (tester) async {
TypingNotifier.debugEnable = false;
addTearDown(TypingNotifier.debugReset);

final channel = eg.stream();
final narrow = ChannelNarrow(channel.streamId);
await prepareComposeBox(tester, narrow: narrow, streams: [channel]);

// (When we check that the send button looks disabled, it should be because
// the file is uploading, not a pre-existing reason.)
await enterTopic(tester, narrow: narrow, topic: 'some topic');
controller!.content.value = const TextEditingValue(text: 'see image: ');
await tester.pump();
await prepare(tester);
checkAppearsLoading(tester, false);

testBinding.pickImageResult = XFile.fromData(
Expand Down Expand Up @@ -589,6 +583,106 @@ void main() {
// target platform the test is simulating.
// TODO(upstream): unskip after fix to https://github.com/flutter/flutter/issues/161073
skip: Platform.isWindows);

group('attach from keyboard', () {
// This is adapted from:
// https://github.com/flutter/flutter/blob/0ffc4ce00ea7bb912e379adf39354644eab2c17e/packages/flutter/test/widgets/editable_text_test.dart#L724-L740
Future<void> insertContentFromKeyboard(WidgetTester tester, {
required List<int>? data,
required String attachedFileUrl,
required String mimeType,
}) async {
await tester.showKeyboard(contentInputFinder);
// This invokes [EditableText.performAction] on the content [TextField],
// which did not expose an API for testing.
// TODO(upstream): support a better API for testing this
await tester.binding.defaultBinaryMessenger.handlePlatformMessage(
SystemChannels.textInput.name,
SystemChannels.textInput.codec.encodeMethodCall(
MethodCall('TextInputClient.performAction', <dynamic>[
-1,
'TextInputAction.commitContent',
// This fakes data originally provided by the Flutter engine:
// https://github.com/flutter/flutter/blob/0ffc4ce00ea7bb912e379adf39354644eab2c17e/engine/src/flutter/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java#L497-L548
{
"mimeType": mimeType,
"data": data,
"uri": attachedFileUrl,
},
])),
(ByteData? data) {});
}

testWidgets('success', (tester) async {
const fileContent = [1, 0, 1, 0, 0];
await prepare(tester);
const uploadUrl = '/user_uploads/1/4e/m2A3MSqFnWRLUf9SaPzQ0Up_/test.gif';
connection.prepare(json: UploadFileResult(uri: uploadUrl).toJson());
await insertContentFromKeyboard(tester,
data: fileContent,
attachedFileUrl:
'content://com.zulip.android.zulipboard.provider'
'/root/com.zulip.android.zulipboard/candidate_temp/test.gif',
mimeType: 'image/gif');

await tester.pump();
check(controller!.content.text)
.equals('see image: [Uploading test.gif…]()\n\n');
// (the request is checked more thoroughly in API tests)
check(connection.lastRequest!).isA<http.MultipartRequest>()
..method.equals('POST')
..files.single.which((it) => it
..field.equals('file')
..length.equals(fileContent.length)
..filename.equals('test.gif')
..contentType.asString.equals('image/gif')
..has<Future<List<int>>>((f) => f.finalize().toBytes(), 'contents')
.completes((it) => it.deepEquals(fileContent))
);
checkAppearsLoading(tester, true);

await tester.pump(Duration.zero);
check(controller!.content.text)
.equals('see image: [test.gif]($uploadUrl)\n\n');
checkAppearsLoading(tester, false);
});

testWidgets('data is null', (tester) async {
await prepare(tester);
await insertContentFromKeyboard(tester,
data: null,
attachedFileUrl:
'content://com.zulip.android.zulipboard.provider'
'/root/com.zulip.android.zulipboard/candidate_temp/test.gif',
mimeType: 'image/jpeg');

await tester.pump();
check(controller!.content.text).equals('see image: ');
check(connection.takeRequests()).isEmpty();
checkErrorDialog(tester,
expectedTitle: 'Content not inserted',
expectedMessage: 'The file to be inserted is empty or cannot be accessed.');
checkAppearsLoading(tester, false);
});

testWidgets('data is empty', (tester) async {
await prepare(tester);
await insertContentFromKeyboard(tester,
data: [],
attachedFileUrl:
'content://com.zulip.android.zulipboard.provider'
'/root/com.zulip.android.zulipboard/candidate_temp/test.gif',
mimeType: 'image/jpeg');

await tester.pump();
check(controller!.content.text).equals('see image: ');
check(connection.takeRequests()).isEmpty();
checkErrorDialog(tester,
expectedTitle: 'Content not inserted',
expectedMessage: 'The file to be inserted is empty or cannot be accessed.');
checkAppearsLoading(tester, false);
});
});
});

group('error banner', () {
Expand Down