diff --git a/ios/Podfile.lock b/ios/Podfile.lock index 1f3f75d8b8..158d87b032 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -1,4 +1,6 @@ PODS: + - "app_settings (3.0.0+1)": + - Flutter - device_info_plus (0.0.1): - Flutter - DKImagePickerController/Core (4.3.4): @@ -36,6 +38,8 @@ PODS: - DKImagePickerController/PhotoGallery - Flutter - Flutter (1.0.0) + - image_picker_ios (0.0.1): + - Flutter - path_provider_foundation (0.0.1): - Flutter - FlutterMacOS @@ -62,9 +66,11 @@ PODS: - SwiftyGif (5.4.4) DEPENDENCIES: + - app_settings (from `.symlinks/plugins/app_settings/ios`) - device_info_plus (from `.symlinks/plugins/device_info_plus/ios`) - file_picker (from `.symlinks/plugins/file_picker/ios`) - Flutter (from `Flutter`) + - image_picker_ios (from `.symlinks/plugins/image_picker_ios/ios`) - path_provider_foundation (from `.symlinks/plugins/path_provider_foundation/darwin`) - share_plus (from `.symlinks/plugins/share_plus/ios`) - sqlite3_flutter_libs (from `.symlinks/plugins/sqlite3_flutter_libs/ios`) @@ -78,12 +84,16 @@ SPEC REPOS: - SwiftyGif EXTERNAL SOURCES: + app_settings: + :path: ".symlinks/plugins/app_settings/ios" device_info_plus: :path: ".symlinks/plugins/device_info_plus/ios" file_picker: :path: ".symlinks/plugins/file_picker/ios" Flutter: :path: Flutter + image_picker_ios: + :path: ".symlinks/plugins/image_picker_ios/ios" path_provider_foundation: :path: ".symlinks/plugins/path_provider_foundation/darwin" share_plus: @@ -92,11 +102,13 @@ EXTERNAL SOURCES: :path: ".symlinks/plugins/sqlite3_flutter_libs/ios" SPEC CHECKSUMS: + app_settings: d103828c9f5d515c4df9ee754dabd443f7cedcf3 device_info_plus: e5c5da33f982a436e103237c0c85f9031142abed DKImagePickerController: b512c28220a2b8ac7419f21c491fc8534b7601ac DKPhotoGallery: fdfad5125a9fdda9cc57df834d49df790dbb4179 file_picker: ce3938a0df3cc1ef404671531facef740d03f920 Flutter: f04841e97a9d0b0a8025694d0796dd46242b2854 + image_picker_ios: 4a8aadfbb6dc30ad5141a2ce3832af9214a705b5 path_provider_foundation: c68054786f1b4f3343858c1e1d0caaded73f0be9 SDWebImage: fd7e1a22f00303e058058278639bf6196ee431fe share_plus: 056a1e8ac890df3e33cb503afffaf1e9b4fbae68 diff --git a/ios/Runner/Info.plist b/ios/Runner/Info.plist index 6ddd93282d..28d1c53236 100644 --- a/ios/Runner/Info.plist +++ b/ios/Runner/Info.plist @@ -53,5 +53,9 @@ fetch + NSCameraUsageDescription + By allowing camera access, you can take photos and send them in Zulip messages. + NSPhotoLibraryUsageDescription + Choose photos from your library and send them in Zulip messages. diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 76da7eae2e..db03721cd9 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -1,5 +1,8 @@ +import 'package:app_settings/app_settings.dart'; import 'package:file_picker/file_picker.dart'; import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:image_picker/image_picker.dart'; import 'dialog.dart'; import '../api/route/messages.dart'; @@ -210,23 +213,97 @@ class _StreamContentInputState extends State<_StreamContentInput> { } } -class _AttachFileButton extends StatelessWidget { - const _AttachFileButton({required this.contentController, required this.contentFocusNode}); +/// Data on a file to be uploaded, from any source. +/// +/// A convenience class to represent data from the generic file picker, +/// the media library, and the camera, in a single form. +class _File { + _File({required this.content, required this.length, required this.filename}); - final ContentTextEditingController contentController; - final FocusNode contentFocusNode; + final Stream> content; + final int length; + final String filename; +} + +Future _uploadFiles({ + required BuildContext context, + required ContentTextEditingController contentController, + required FocusNode contentFocusNode, + required Iterable<_File> files, +}) async { + assert(context.mounted); + final store = PerAccountStoreWidget.of(context); + + final List<_File> tooLargeFiles = []; + final List<_File> rightSizeFiles = []; + for (final file in files) { + if ((file.length / (1 << 20)) > store.maxFileUploadSizeMib) { + tooLargeFiles.add(file); + } else { + rightSizeFiles.add(file); + } + } + + if (tooLargeFiles.isNotEmpty) { + final listMessage = tooLargeFiles + .map((file) => '${file.filename}: ${(file.length / (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, _File)> uploadsInProgress = []; + for (final file in rightSizeFiles) { + final tag = contentController.registerUploadStart(file.filename); + uploadsInProgress.add((tag, file)); + } + if (!contentFocusNode.hasFocus) { + contentFocusNode.requestFocus(); + } - _handlePress(BuildContext context) async { - FilePickerResult? result; + for (final (tag, file) in uploadsInProgress) { + final _File(:content, :length, :filename) = file; + Uri? url; try { - result = await FilePicker.platform.pickFiles(allowMultiple: true, withReadStream: true); + final result = await uploadFile(store.connection, + content: content, length: length, filename: filename); + url = Uri.parse(result.uri); } catch (e) { - // TODO(i18n) - showErrorDialog(context: context, title: 'Error', message: e.toString()); - return; + 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: $filename', message: e.toString()); + } finally { + contentController.registerUploadEnd(tag, url); } - if (result == null) { - return; // User cancelled; do nothing + } +} + +abstract class _AttachUploadsButton extends StatelessWidget { + const _AttachUploadsButton({required this.contentController, required this.contentFocusNode}); + + final ContentTextEditingController contentController; + final FocusNode contentFocusNode; + + IconData get icon; + + /// Request files from the user, in the way specific to this upload type. + /// + /// Subclasses should manage the interaction completely, e.g., by catching and + /// handling any permissions-related exceptions. + /// + /// To signal exiting the interaction with no files chosen, + /// return an empty [Iterable] after showing user feedback as appropriate. + Future> getFiles(BuildContext context); + + void _handlePress(BuildContext context) async { + final files = await getFiles(context); + if (files.isEmpty) { + return; // Nothing to do (getFiles handles user feedback) } if (context.mounted) {} // https://github.com/dart-lang/linter/issues/4007 @@ -234,61 +311,123 @@ class _AttachFileButton extends StatelessWidget { return; } - final store = PerAccountStoreWidget.of(context); + await _uploadFiles( + context: context, + contentController: contentController, + contentFocusNode: contentFocusNode, + files: files); + } - 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); - } - } + @override + Widget build(BuildContext context) { + return IconButton(icon: Icon(icon), onPressed: () => _handlePress(context)); + } +} - 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'); +Future> _getFilePickerFiles(BuildContext context, FileType type) async { + FilePickerResult? result; + try { + result = await FilePicker.platform + .pickFiles(allowMultiple: true, withReadStream: true, type: type); + } catch (e) { + if (e is PlatformException && e.code == 'read_external_storage_denied') { + // Observed on Android. If Android's error message tells us whether the + // user has checked "Don't ask again", it seems the library doesn't pass + // that on to us. So just always prompt to check permissions in settings. + // If the user hasn't checked "Don't ask again", they can always dismiss + // our prompt and retry, and the permissions request will reappear, + // letting them grant permissions and complete the upload. + showSuggestedActionDialog(context: context, // TODO(i18n) + title: 'Permissions needed', + message: 'To upload files, please grant Zulip additional permissions in Settings.', + actionButtonText: 'Open settings', + onActionButtonPress: () { + AppSettings.openAppSettings(); + }); + } else { + // TODO(i18n) + showErrorDialog(context: context, title: 'Error', message: e.toString()); } + return []; + } + if (result == null) { + return []; // User cancelled; do nothing + } - 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(); - } + return result.files.map((f) { + assert(f.readStream != null); // We passed `withReadStream: true` to pickFiles. + return _File(content: f.readStream!, length: f.size, filename: f.name); + }); +} - 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); - } - } +class _AttachFileButton extends _AttachUploadsButton { + const _AttachFileButton({required super.contentController, required super.contentFocusNode}); + + @override + IconData get icon => Icons.attach_file; + + @override + Future> getFiles(BuildContext context) async { + return _getFilePickerFiles(context, FileType.any); } +} + +class _AttachMediaButton extends _AttachUploadsButton { + const _AttachMediaButton({required super.contentController, required super.contentFocusNode}); @override - Widget build(BuildContext context) { - return IconButton(icon: const Icon(Icons.attach_file), onPressed: () => _handlePress(context)); + IconData get icon => Icons.image; + + @override + Future> getFiles(BuildContext context) async { + // TODO: This doesn't give quite the right UI on Android. + // Perhaps try `image_picker`: https://github.com/zulip/zulip-flutter/issues/56#issuecomment-1514001281 + return _getFilePickerFiles(context, FileType.media); + } +} + +class _AttachFromCameraButton extends _AttachUploadsButton { + const _AttachFromCameraButton({required super.contentController, required super.contentFocusNode}); + + @override + IconData get icon => Icons.camera_alt; + + @override + Future> getFiles(BuildContext context) async { + final picker = ImagePicker(); + final XFile? result; + try { + // Ideally we'd open a platform interface that lets you choose between + // taking a photo and a video. `image_picker` doesn't yet have that + // option: https://github.com/flutter/flutter/issues/89159 + // so just stick with images for now. We could add another button for + // videos, but we don't want too many buttons. + result = await picker.pickImage(source: ImageSource.camera, requestFullMetadata: false); + } catch (e) { + if (e is PlatformException && e.code == 'camera_access_denied') { + // iOS has a quirk where it will only request the native + // permission-request alert once, the first time the app wants to + // use a protected resource. After that, the only way the user can + // grant it is in Settings. + showSuggestedActionDialog(context: context, // TODO(i18n) + title: 'Permissions needed', + message: 'To upload an image, please grant Zulip additional permissions in Settings.', + actionButtonText: 'Open settings', + onActionButtonPress: () { + AppSettings.openAppSettings(); + }); + } else { + // TODO(i18n) + showErrorDialog(context: context, title: 'Error', message: e.toString()); + } + return []; + } + if (result == null) { + return []; // User cancelled; do nothing + } + final length = await result.length(); + + return [_File(content: result.openRead(), length: length, filename: result.name)]; } } @@ -490,6 +629,8 @@ class _StreamComposeBoxState extends State { child: Row( children: [ _AttachFileButton(contentController: _contentController, contentFocusNode: _contentFocusNode), + _AttachMediaButton(contentController: _contentController, contentFocusNode: _contentFocusNode), + _AttachFromCameraButton(contentController: _contentController, contentFocusNode: _contentFocusNode), ])), ])))); } diff --git a/lib/widgets/dialog.dart b/lib/widgets/dialog.dart index bc42106b52..d0c363b2bb 100644 --- a/lib/widgets/dialog.dart +++ b/lib/widgets/dialog.dart @@ -27,3 +27,23 @@ void showErrorDialog({required BuildContext context, required String title, Stri child: _dialogActionText('OK')), ])); } + +void showSuggestedActionDialog({ + required BuildContext context, + required String title, + required String message, + required String? actionButtonText, + required VoidCallback onActionButtonPress, +}) { + showDialog(context: context, builder: (BuildContext context) => AlertDialog( + title: Text(title), + content: SingleChildScrollView(child: Text(message)), + actions: [ + TextButton( + onPressed: () => Navigator.pop(context), + child: _dialogActionText('Cancel')), + TextButton( + onPressed: onActionButtonPress, + child: _dialogActionText(actionButtonText ?? 'Continue')), + ])); +} diff --git a/pubspec.lock b/pubspec.lock index a291389aa5..749d0ba897 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -25,6 +25,14 @@ packages: url: "https://pub.dev" source: hosted version: "0.11.2" + app_settings: + dependency: "direct main" + description: + name: app_settings + sha256: "66715a323ac36d6c8201035ba678777c0d2ea869e4d7064300d95af10c3bb8cb" + url: "https://pub.dev" + source: hosted + version: "4.2.0" args: dependency: transitive description: @@ -393,6 +401,46 @@ packages: url: "https://pub.dev" source: hosted version: "4.0.2" + image_picker: + dependency: "direct main" + description: + name: image_picker + sha256: f202f5d730eb8219e35e80c4461fb3a779940ad30ce8fde1586df756e3af25e6 + url: "https://pub.dev" + source: hosted + version: "0.8.7+3" + image_picker_android: + dependency: transitive + description: + name: image_picker_android + sha256: "1ea6870350f56af8dab716459bd9d5dc76947e29e07a2ba1d0c172eaaf4f269c" + url: "https://pub.dev" + source: hosted + version: "0.8.6+7" + image_picker_for_web: + dependency: transitive + description: + name: image_picker_for_web + sha256: "98f50d6b9f294c8ba35e25cc0d13b04bfddd25dbc8d32fa9d566a6572f2c081c" + url: "https://pub.dev" + source: hosted + version: "2.1.12" + image_picker_ios: + dependency: transitive + description: + name: image_picker_ios + sha256: a1546ff5861fc15812953d4733b520c3d371cec3d2859a001ff04c46c4d81883 + url: "https://pub.dev" + source: hosted + version: "0.8.7+3" + image_picker_platform_interface: + dependency: transitive + description: + name: image_picker_platform_interface + sha256: "1991219d9dbc42a99aff77e663af8ca51ced592cd6685c9485e3458302d3d4f8" + url: "https://pub.dev" + source: hosted + version: "2.6.3" intl: dependency: "direct main" description: diff --git a/pubspec.yaml b/pubspec.yaml index 8b24617df4..ac0bd840af 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -52,6 +52,8 @@ dependencies: path_provider: ^2.0.13 path: ^1.8.3 sqlite3_flutter_libs: ^0.5.13 + app_settings: ^4.2.0 + image_picker: ^0.8.7+3 dev_dependencies: flutter_test: