From 0448122298b898b07a43b95cdf9d17601cf38310 Mon Sep 17 00:00:00 2001 From: holybiber Date: Wed, 24 Jul 2024 09:18:14 +0200 Subject: [PATCH] Refactor: Start using feature-first architecture It's a goal to refactor the source to have a clean feature-first architecture. Start by putting the new sharing feature into a separate folder features/share/ Increase test coverage for ShareButton to 100% --- .../share}/share_button.dart | 40 +------------------ lib/features/share/share_service.dart | 40 +++++++++++++++++++ lib/routes/view_page.dart | 2 +- test/share_button_test.dart | 10 ++++- test/view_page_test.dart | 2 +- 5 files changed, 51 insertions(+), 43 deletions(-) rename lib/{widgets => features/share}/share_button.dart (82%) create mode 100644 lib/features/share/share_service.dart diff --git a/lib/widgets/share_button.dart b/lib/features/share/share_button.dart similarity index 82% rename from lib/widgets/share_button.dart rename to lib/features/share/share_button.dart index d747552..5cb2735 100644 --- a/lib/widgets/share_button.dart +++ b/lib/features/share/share_button.dart @@ -2,55 +2,17 @@ import 'dart:async'; import 'package:app4training/data/languages.dart'; import 'package:app4training/design/theme.dart'; +import 'package:app4training/features/share/share_service.dart'; import 'package:app4training/l10n/l10n.dart'; import 'package:app4training/routes/view_page.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:open_filex/open_filex.dart'; -import 'package:share_plus/share_plus.dart'; // TODO: Use build_runner with flutter_gen instead const String openPdfImage = 'assets/file-document-outline.png'; const String sharePdfImage = 'assets/file-document-arrow-right-outline.png'; const String shareLinkImage = 'assets/link.png'; -/// A class around all sharing functionality to enable testing -/// (Packages: share_plus, url_launcher, open_filex) -/// -/// Other options for testing (dependency injection etc.) are hard to use here -/// because of the usage of context.findAncestorWidgetOfExactType -/// so this is probably the best way to do it -class ShareService { - /// Wraps Share.share() (package share_plus) - Future share(String text) { - return Share.share(text); - } - - /// Wraps Share.shareXFiles() (package share_plus) - /// - /// Not using the same argument List because that would make it - /// harder to verify that the function gets called with correct arguments - /// with mocktail - Future shareFile(String path) { - return Share.shareXFiles([XFile(path)]); - } - - /// Wraps launchUrl (package url_launcher) - Future launchUrl(Uri url) { - return launchUrl(url); - } - - /// Wraps OpenFilex.open (package open_filex) - Future open(String? filePath) { - return OpenFilex.open(filePath); - } -} - -/// A provider for all sharing functionality to enable testing -final shareProvider = Provider((ref) { - return ShareService(); -}); - /// Share button in the top right corner of the main view. /// Opens a dropdown with several sharing options. /// Implemented with MenuAnchor+MenuItemButton diff --git a/lib/features/share/share_service.dart b/lib/features/share/share_service.dart new file mode 100644 index 0000000..7e8e429 --- /dev/null +++ b/lib/features/share/share_service.dart @@ -0,0 +1,40 @@ +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:open_filex/open_filex.dart'; +import 'package:share_plus/share_plus.dart'; + +/// A class around all sharing functionality to enable testing +/// (Packages: share_plus, url_launcher, open_filex) +/// +/// Other options for testing (dependency injection etc.) are hard to use here +/// because of the usage of context.findAncestorWidgetOfExactType +/// so this is probably the best way to do it +class ShareService { + /// Wraps Share.share() (package share_plus) + Future share(String text) { + return Share.share(text); + } + + /// Wraps Share.shareXFiles() (package share_plus) + /// + /// Not using the same argument List because that would make it + /// harder to verify that the function gets called with correct arguments + /// with mocktail + Future shareFile(String path) { + return Share.shareXFiles([XFile(path)]); + } + + /// Wraps launchUrl (package url_launcher) + Future launchUrl(Uri url) { + return launchUrl(url); + } + + /// Wraps OpenFilex.open (package open_filex) + Future open(String? filePath) { + return OpenFilex.open(filePath); + } +} + +/// A provider for all sharing functionality to enable testing +final shareProvider = Provider((ref) { + return ShareService(); +}); diff --git a/lib/routes/view_page.dart b/lib/routes/view_page.dart index 2896c3c..09185eb 100644 --- a/lib/routes/view_page.dart +++ b/lib/routes/view_page.dart @@ -4,7 +4,7 @@ import 'package:app4training/data/globals.dart'; import 'package:app4training/l10n/l10n.dart'; import 'package:app4training/widgets/error_message.dart'; import 'package:app4training/widgets/html_view.dart'; -import 'package:app4training/widgets/share_button.dart'; +import 'package:app4training/features/share/share_button.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:app4training/data/languages.dart'; diff --git a/test/share_button_test.dart b/test/share_button_test.dart index 9f509bf..32b7386 100644 --- a/test/share_button_test.dart +++ b/test/share_button_test.dart @@ -1,8 +1,9 @@ import 'package:app4training/data/app_language.dart'; import 'package:app4training/data/languages.dart'; +import 'package:app4training/features/share/share_service.dart'; +import 'package:app4training/features/share/share_button.dart'; import 'package:app4training/l10n/l10n.dart'; import 'package:app4training/routes/view_page.dart'; -import 'package:app4training/widgets/share_button.dart'; import 'package:flutter/material.dart' hide Page; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -42,7 +43,7 @@ Finder findAssetImageIcon(String assetName, [Color? color]) { class MockShareService extends Mock implements ShareService {} void main() { - testWidgets('Smoke test: open the share menu (English locale)', + testWidgets('Smoke test: open and close the share menu (English locale)', (WidgetTester tester) async { await tester.pumpWidget(ProviderScope(overrides: [ appLanguageProvider.overrideWith(() => TestAppLanguage('en')), @@ -64,6 +65,11 @@ void main() { expect(findAssetImageIcon(openPdfImage), findsOneWidget); expect(findAssetImageIcon(sharePdfImage), findsOneWidget); expect(findAssetImageIcon(shareLinkImage), findsOneWidget); + + await tester.tap(find.byType(ShareButton)); + await tester.pump(); + expect(find.byIcon(Icons.share), findsOneWidget); + expect(find.text('Open PDF'), findsNothing); }); testWidgets('Test when PDFs are not available', (WidgetTester tester) async { diff --git a/test/view_page_test.dart b/test/view_page_test.dart index 9c9e1ae..f82990e 100644 --- a/test/view_page_test.dart +++ b/test/view_page_test.dart @@ -2,10 +2,10 @@ import 'package:app4training/data/app_language.dart'; import 'package:app4training/data/exceptions.dart'; import 'package:app4training/data/globals.dart'; import 'package:app4training/data/languages.dart'; +import 'package:app4training/features/share/share_button.dart'; import 'package:app4training/l10n/l10n.dart'; import 'package:app4training/routes/view_page.dart'; import 'package:app4training/widgets/language_selection_button.dart'; -import 'package:app4training/widgets/share_button.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart';