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.

Sign up for GitHub

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

lightbox: Add "share" button in bottom app bar #1145

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

lightbox: Add "share" button in bottom app bar #1145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,10 @@
"@errorDialogTitle": {
"description": "Generic title for error dialog."
},
"errorShareFailed": "Failed to share the image",
"@errorShareFailed": {
"description": "Title for sharing image error dialog."
},
"snackBarDetails": "Details",
"@snackBarDetails": {
"description": "Button label for snack bar button that opens a dialog with more details."
Expand All @@ -395,6 +399,10 @@
"@lightboxCopyLinkTooltip": {
"description": "Tooltip in lightbox for the copy link action."
},
"lightboxShareImageTooltip": "Share Image",
"@lightboxShareImageTooltip": {
"description": "Tooltip in lightbox for the Share Image action."
},
"loginPageTitle": "Log in",
"@loginPageTitle": {
"description": "Title for login page."
Expand Down
12 changes: 12 additions & 0 deletions lib/generated/l10n/zulip_localizations.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,12 @@ abstract class ZulipLocalizations {
/// **'Error'**
String get errorDialogTitle;

/// Title for sharing image error dialog.
///
/// In en, this message translates to:
/// **'Failed to share the image'**
String get errorShareFailed;

/// Button label for snack bar button that opens a dialog with more details.
///
/// In en, this message translates to:
Expand All @@ -631,6 +637,12 @@ abstract class ZulipLocalizations {
/// **'Copy link'**
String get lightboxCopyLinkTooltip;

/// Tooltip in lightbox for the Share Image action.
///
/// In en, this message translates to:
/// **'Share Image'**
String get lightboxShareImageTooltip;

/// Title for login page.
///
/// In en, this message translates to:
Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_ar.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,12 +304,18 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
@override
String get errorDialogTitle => 'Error';

@override
String get errorShareFailed => 'Failed to share the image';

@override
String get snackBarDetails => 'Details';

@override
String get lightboxCopyLinkTooltip => 'Copy link';

@override
String get lightboxShareImageTooltip => 'Share Image';

@override
String get loginPageTitle => 'Log in';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_en.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,12 +304,18 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
@override
String get errorDialogTitle => 'Error';

@override
String get errorShareFailed => 'Failed to share the image';

@override
String get snackBarDetails => 'Details';

@override
String get lightboxCopyLinkTooltip => 'Copy link';

@override
String get lightboxShareImageTooltip => 'Share Image';

@override
String get loginPageTitle => 'Log in';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_fr.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,12 +304,18 @@ class ZulipLocalizationsFr extends ZulipLocalizations {
@override
String get errorDialogTitle => 'Error';

@override
String get errorShareFailed => 'Failed to share the image';

@override
String get snackBarDetails => 'Details';

@override
String get lightboxCopyLinkTooltip => 'Copy link';

@override
String get lightboxShareImageTooltip => 'Share Image';

@override
String get loginPageTitle => 'Log in';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_ja.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,12 +304,18 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
@override
String get errorDialogTitle => 'Error';

@override
String get errorShareFailed => 'Failed to share the image';

@override
String get snackBarDetails => 'Details';

@override
String get lightboxCopyLinkTooltip => 'Copy link';

@override
String get lightboxShareImageTooltip => 'Share Image';

@override
String get loginPageTitle => 'Log in';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_pl.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,12 +304,18 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
@override
String get errorDialogTitle => 'Błąd';

@override
String get errorShareFailed => 'Failed to share the image';

@override
String get snackBarDetails => 'Szczegóły';

@override
String get lightboxCopyLinkTooltip => 'Skopiuj odnośnik';

@override
String get lightboxShareImageTooltip => 'Share Image';

@override
String get loginPageTitle => 'Zaloguj';

Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations_ru.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,12 +304,18 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
@override
String get errorDialogTitle => 'Error';

@override
String get errorShareFailed => 'Failed to share the image';

@override
String get snackBarDetails => 'Details';

@override
String get lightboxCopyLinkTooltip => 'Copy link';

@override
String get lightboxShareImageTooltip => 'Share Image';

@override
String get loginPageTitle => 'Log in';

Expand Down
45 changes: 43 additions & 2 deletions lib/widgets/lightbox.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import 'package:flutter/material.dart';
import 'package:flutter/scheduler.dart';
import 'package:flutter/services.dart';
import 'package:intl/intl.dart';
import 'package:share_plus/share_plus.dart';
import 'package:video_player/video_player.dart';

import 'package:http/http.dart' as http;
import '../api/core.dart';
import '../api/model/model.dart';
import '../generated/l10n/zulip_localizations.dart';
Expand Down Expand Up @@ -89,6 +90,46 @@ class _CopyLinkButton extends StatelessWidget {
}
}

Future<XFile> _downloadImage(Uri url, Map<String, String> headers) async {
final response = await http.get(url, headers: headers);
final bytes = response.bodyBytes;
return XFile.fromData(bytes,
name: url.pathSegments.last,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed this in last review — trying to get the filename this way seems fragile.

To do this correctly we will need changes in the content parser (lib/model/content.dart) to read the filename in ImageNode, but it may be out of scope for this PR. So, for now how about not populating the name field here, and leaving a TODO comment.

mimeType: response.headers['content-type']);
}

class _ShareButton extends StatelessWidget {
const _ShareButton({required this.url});

final Uri url;

@override
Widget build(BuildContext context) {
final zulipLocalizations = ZulipLocalizations.of(context);
return IconButton(
tooltip: zulipLocalizations.lightboxShareImageTooltip,
icon: const Icon(Icons.share),
onPressed: () async {
try {
final store = PerAccountStoreWidget.of(context);
final headers = {
if (url.origin == store.account.realmUrl.origin)
...authHeader(email: store.account.email, apiKey: store.account.apiKey),
...userAgentHeader()
};
final xFile = await _downloadImage(url, headers);
await Share.shareXFiles([xFile]);
} catch (error) {
if (!context.mounted) return;
showErrorDialog(
context: context,
title: zulipLocalizations.errorDialogTitle,
message: zulipLocalizations.errorShareFailed);
}
});
}
}

class _LightboxPageLayout extends StatefulWidget {
const _LightboxPageLayout({
required this.routeEntranceAnimation,
Expand Down Expand Up @@ -258,7 +299,7 @@ class _ImageLightboxPageState extends State<_ImageLightboxPage> {
elevation: elevation,
child: Row(children: [
_CopyLinkButton(url: widget.src),
// TODO(#43): Share image
_ShareButton(url: widget.src),
// TODO(#42): Download image
]),
);
Expand Down
23 changes: 23 additions & 0 deletions test/widgets/lightbox_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,29 @@ void main() {
debugNetworkImageHttpClientProvider = null;
});

testWidgets('share button shows correct icon and downloads image', (tester) async {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This'll need more tests, namely:

  • Tapping on share button opens the share sheet (check if shareXFiles function was called).
  • Share failed and the error dialog was shown.

prepareBoringImageHttpClient();
final message = eg.streamMessage();
await setupPage(tester, message: message, thumbnailUrl: null);

// Verify share icon exists
final shareIcon = find.descendant(
of: find.byType(BottomAppBar),
matching: find.byIcon(Icons.share),
skipOffstage: false);
check(tester.widget<Icon>(shareIcon).icon).equals(Icons.share);

// Verify tooltip
final button = tester.widget<IconButton>(find.ancestor(
of: shareIcon,
matching: find.byType(IconButton)));
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
check(button.tooltip).equals(zulipLocalizations.lightboxShareImageTooltip);
check(button.tooltip).equals(zulipLocalizations.lightboxShareImageTooltip);

debugNetworkImageHttpClientProvider = null;
});

// TODO test _CopyLinkButton
// TODO test thumbnail gets shown, then gets replaced when main image loads
// TODO test image is scaled down to fit, but not up
Expand Down