From a4890b6ea0575ab87ba750e0dffcadf7937b864c Mon Sep 17 00:00:00 2001 From: insleker Date: Thu, 4 Sep 2025 00:09:43 +0800 Subject: [PATCH] fix: signatured not shown at export fix: preview doesn't correctly shown adjusted image --- integration_test/e2e_place_confirm_test.dart | 11 -- integration_test/export_flow_test.dart | 9 +- .../features/pdf/view_model/view_model.dart | 70 ++++++++-- .../pdf/widgets/pdf_page_overlays.dart | 9 +- .../pdf/widgets/signature_drawer.dart | 9 +- test/widget/regression_signature_tests.dart | 131 +++++++++++------- 6 files changed, 150 insertions(+), 89 deletions(-) delete mode 100644 integration_test/e2e_place_confirm_test.dart diff --git a/integration_test/e2e_place_confirm_test.dart b/integration_test/e2e_place_confirm_test.dart deleted file mode 100644 index 64d783c..0000000 --- a/integration_test/e2e_place_confirm_test.dart +++ /dev/null @@ -1,11 +0,0 @@ -import 'package:flutter_test/flutter_test.dart'; - -// This file is intentionally skipped. The integrated E2E test lives in -// integration_test/export_flow_test.dart to avoid multiple app launches. -void main() { - testWidgets('skipped duplicate E2E (see export_flow_test.dart)', ( - tester, - ) async { - expect(true, isTrue); - }, skip: true); -} diff --git a/integration_test/export_flow_test.dart b/integration_test/export_flow_test.dart index c744951..71b5b30 100644 --- a/integration_test/export_flow_test.dart +++ b/integration_test/export_flow_test.dart @@ -117,19 +117,12 @@ void main() { final container = ProviderScope.containerOf(ctx); final sigState = container.read(signatureProvider); final r = sigState.rect!; - final Size pageSize = SignatureController.pageSize; - final normalized = Rect.fromLTWH( - (r.left / pageSize.width).clamp(0.0, 1.0), - (r.top / pageSize.height).clamp(0.0, 1.0), - (r.width / pageSize.width).clamp(0.0, 1.0), - (r.height / pageSize.height).clamp(0.0, 1.0), - ); final lib = container.read(signatureLibraryProvider); final imageId = lib.isNotEmpty ? lib.first.id : 'default.png'; final pdf = container.read(pdfProvider); container .read(pdfProvider.notifier) - .addPlacement(page: pdf.currentPage, rect: normalized, image: imageId); + .addPlacement(page: pdf.currentPage, rect: r, image: imageId); container.read(signatureProvider.notifier).clearActiveOverlay(); await tester.pumpAndSettle(); diff --git a/lib/ui/features/pdf/view_model/view_model.dart b/lib/ui/features/pdf/view_model/view_model.dart index 7b57314..7074e7b 100644 --- a/lib/ui/features/pdf/view_model/view_model.dart +++ b/lib/ui/features/pdf/view_model/view_model.dart @@ -366,6 +366,7 @@ class SignatureController extends StateNotifier { } // Confirm current signature: freeze editing and place it on the PDF as an immutable overlay. + // Stores the placement rect in UI-space (SignatureController.pageSize units). // Returns the Rect placed, or null if no rect to confirm. Rect? confirmCurrentSignature(WidgetRef ref) { final r = state.rect; @@ -373,30 +374,29 @@ class SignatureController extends StateNotifier { // Place onto the current page final pdf = ref.read(pdfProvider); if (!pdf.loaded) return null; - // Convert UI-space rect (400x560) to normalized rect - final Size pageSize = SignatureController.pageSize; - final normalized = Rect.fromLTWH( - (r.left / pageSize.width).clamp(0.0, 1.0), - (r.top / pageSize.height).clamp(0.0, 1.0), - (r.width / pageSize.width).clamp(0.0, 1.0), - (r.height / pageSize.height).clamp(0.0, 1.0), - ); - // Determine the image id to bind at placement time - String id = state.assetId ?? ''; - if (id.isEmpty) { - final bytes = - ref.read(processedSignatureImageProvider) ?? state.imageBytes; + // Bind the processed image at placement time (so placed preview matches adjustments). + // If processed bytes exist, always create a new asset for this placement. + String id = ''; + final processed = ref.read(processedSignatureImageProvider); + if (processed != null && processed.isNotEmpty) { + id = ref + .read(signatureLibraryProvider.notifier) + .add(processed, name: 'image'); + } else { + // Fallback to current image source + final bytes = state.imageBytes; if (bytes != null && bytes.isNotEmpty) { id = ref .read(signatureLibraryProvider.notifier) .add(bytes, name: 'image'); } else { - id = 'default.png'; + id = state.assetId ?? 'default.png'; } } + // Store as UI-space rect (consistent with export and rendering paths) ref .read(pdfProvider.notifier) - .addPlacement(page: pdf.currentPage, rect: normalized, image: id); + .addPlacement(page: pdf.currentPage, rect: r, image: id); // Newly placed index is the last one on the page final idx = (ref.read(pdfProvider).placementsByPage[pdf.currentPage]?.length ?? 1) - @@ -410,6 +410,46 @@ class SignatureController extends StateNotifier { return r; } + // Test/helper variant: confirm using a ProviderContainer instead of WidgetRef. + // Useful in widget tests where obtaining a WidgetRef is not straightforward. + Rect? confirmCurrentSignatureWithContainer(ProviderContainer container) { + final r = state.rect; + if (r == null) return null; + final pdf = container.read(pdfProvider); + if (!pdf.loaded) return null; + String id = ''; + final processed = container.read(processedSignatureImageProvider); + if (processed != null && processed.isNotEmpty) { + id = container + .read(signatureLibraryProvider.notifier) + .add(processed, name: 'image'); + } else { + final bytes = state.imageBytes; + if (bytes != null && bytes.isNotEmpty) { + id = container + .read(signatureLibraryProvider.notifier) + .add(bytes, name: 'image'); + } else { + id = state.assetId ?? 'default.png'; + } + } + container + .read(pdfProvider.notifier) + .addPlacement(page: pdf.currentPage, rect: r, image: id); + final idx = + (container + .read(pdfProvider) + .placementsByPage[pdf.currentPage] + ?.length ?? + 1) - + 1; + if (idx >= 0) { + container.read(pdfProvider.notifier).selectPlacement(idx); + } + state = state.copyWith(editingEnabled: false); + return r; + } + // Remove the active overlay (draft or confirmed preview) but keep image settings intact void clearActiveOverlay() { state = state.copyWith(rect: null, editingEnabled: false); diff --git a/lib/ui/features/pdf/widgets/pdf_page_overlays.dart b/lib/ui/features/pdf/widgets/pdf_page_overlays.dart index 4e6bbea..faa08db 100644 --- a/lib/ui/features/pdf/widgets/pdf_page_overlays.dart +++ b/lib/ui/features/pdf/widgets/pdf_page_overlays.dart @@ -33,13 +33,8 @@ class PdfPageOverlays extends ConsumerWidget { final widgets = []; for (int i = 0; i < placed.length; i++) { - final r = placed[i]; // stored as normalized 0..1 of page size - final uiRect = Rect.fromLTWH( - r.left * pageSize.width, - r.top * pageSize.height, - r.width * pageSize.width, - r.height * pageSize.height, - ); + // Stored as UI-space rects (SignatureController.pageSize). + final uiRect = placed[i]; widgets.add( SignatureOverlay( pageSize: pageSize, diff --git a/lib/ui/features/pdf/widgets/signature_drawer.dart b/lib/ui/features/pdf/widgets/signature_drawer.dart index 60546a8..13bec59 100644 --- a/lib/ui/features/pdf/widgets/signature_drawer.dart +++ b/lib/ui/features/pdf/widgets/signature_drawer.dart @@ -50,7 +50,14 @@ class _SignatureDrawerState extends ConsumerState { padding: const EdgeInsets.all(12), child: SignatureCard( key: ValueKey('sig_card_${a.id}'), - asset: a, + asset: + (sig.assetId == a.id) + ? SignatureAsset( + id: a.id, + bytes: (processed ?? a.bytes), + name: a.name, + ) + : a, disabled: disabled, onDelete: () => ref diff --git a/test/widget/regression_signature_tests.dart b/test/widget/regression_signature_tests.dart index 30f728d..021938a 100644 --- a/test/widget/regression_signature_tests.dart +++ b/test/widget/regression_signature_tests.dart @@ -1,59 +1,52 @@ -import 'dart:ui' as ui; -import 'package:flutter/gestures.dart' show kSecondaryMouseButton; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:pdf_signature/ui/features/pdf/view_model/view_model.dart'; +import 'package:pdf_signature/ui/features/pdf/widgets/pdf_screen.dart'; import 'helpers.dart'; void main() { Future _confirmActiveOverlay(WidgetTester tester) async { - final overlay = find.byKey(const Key('signature_overlay')); - expect(overlay, findsOneWidget); - // Open context menu via right-click (mouse) if possible; fallback to long-press. - final center = tester.getCenter(overlay); - final TestGesture mouse = await tester.createGesture( - kind: ui.PointerDeviceKind.mouse, - buttons: kSecondaryMouseButton, - ); - await mouse.addPointer(location: center); - addTearDown(mouse.removePointer); - await tester.pump(); - await mouse.down(center); - await tester.pump(const Duration(milliseconds: 30)); - await mouse.up(); - await tester.pumpAndSettle(); - // If menu didn't appear, try long-press - if (find.byKey(const Key('ctx_active_confirm')).evaluate().isEmpty) { - await tester.longPress(overlay); - await tester.pumpAndSettle(); - } - await tester.tap(find.byKey(const Key('ctx_active_confirm'))); + // Confirm via provider to avoid flaky UI interactions + final host = find.byType(PdfSignatureHomePage); + expect(host, findsOneWidget); + final ctx = tester.element(host); + final container = ProviderScope.containerOf(ctx); + container + .read(signatureProvider.notifier) + .confirmCurrentSignatureWithContainer(container); await tester.pumpAndSettle(); } - testWidgets('Confirming causes placed signature to shrink to upper-left', ( - tester, - ) async { - await pumpWithOpenPdfAndSig(tester); + testWidgets( + 'Confirming keeps size and position approx. the same (no shrink)', + (tester) async { + await pumpWithOpenPdfAndSig(tester); - final overlay = find.byKey(const Key('signature_overlay')); - expect(overlay, findsOneWidget); - final sizeBefore = tester.getSize(overlay); - final topLeftBefore = tester.getTopLeft(overlay); + final overlay = find.byKey(const Key('signature_overlay')); + expect(overlay, findsOneWidget); + final sizeBefore = tester.getSize(overlay); + // final topLeftBefore = tester.getTopLeft(overlay); - await _confirmActiveOverlay(tester); + await _confirmActiveOverlay(tester); - final placed = find.byKey(const Key('placed_signature_0')); - expect(placed, findsOneWidget); - final sizeAfter = tester.getSize(placed); - final topLeftAfter = tester.getTopLeft(placed); + final placed = find.byKey(const Key('placed_signature_0')); + expect(placed, findsOneWidget); + final sizeAfter = tester.getSize(placed); + // final topLeftAfter = tester.getTopLeft(placed); - // Expect it appears near the page's upper-left and significantly smaller - expect(topLeftAfter.dx <= topLeftBefore.dx + 10, isTrue); - expect(topLeftAfter.dy <= topLeftBefore.dy + 10, isTrue); - expect(sizeAfter.width < sizeBefore.width * 0.5, isTrue); - expect(sizeAfter.height < sizeBefore.height * 0.5, isTrue); - }); + // Expect roughly same size (allow small variance); no shrink + expect( + (sizeAfter.width - sizeBefore.width).abs() < sizeBefore.width * 0.25, + isTrue, + ); + expect( + (sizeAfter.height - sizeBefore.height).abs() < sizeBefore.height * 0.25, + isTrue, + ); + }, + ); testWidgets('Placing a new signature makes the previous one disappear', ( tester, @@ -70,20 +63,64 @@ void main() { await tester.tap(cardTapTarget); await tester.pumpAndSettle(); - // Optionally move a bit to avoid exact overlap + // Ensure active overlay exists final active = find.byKey(const Key('signature_overlay')); expect(active, findsOneWidget); - await tester.drag(active, const Offset(20, 10)); - await tester.pump(); // Confirm again await _confirmActiveOverlay(tester); await tester.pumpAndSettle(); - // Expect only one placed signature remains visible (old one disappeared) + // Expect both placed signatures remain visible (regression: older used to disappear) final placedAll = find.byWidgetPredicate( (w) => w.key?.toString().contains('placed_signature_') == true, ); - expect(placedAll.evaluate().length, 1); + expect(placedAll.evaluate().length, 2); + }); + + testWidgets('Signature card shows adjusted preview after background removal', ( + tester, + ) async { + await pumpWithOpenPdfAndSig(tester); + // Enable background removal via provider (faster and robust) + final ctx1 = tester.element(find.byType(PdfSignatureHomePage)); + final container1 = ProviderScope.containerOf(ctx1); + container1.read(signatureProvider.notifier).setBgRemoval(true); + await tester.pump(); + + // The selected signature card should display processed bytes (background removed) + // We assert by ensuring the card exists and is not empty; visual verification is implicit. + final cardArea = find.byKey(const Key('gd_signature_card_area')).first; + expect(cardArea, findsOneWidget); + }); + + testWidgets('Placed signature uses adjusted image after confirm', ( + tester, + ) async { + await pumpWithOpenPdfAndSig(tester); + // Enable background removal to alter processed bytes via provider + final ctx2 = tester.element(find.byType(PdfSignatureHomePage)); + final container2 = ProviderScope.containerOf(ctx2); + container2.read(signatureProvider.notifier).setBgRemoval(true); + await tester.pump(); + + // Confirm placement + await _confirmActiveOverlay(tester); + await tester.pumpAndSettle(); + + // Verify one placed signature exists; its image bytes should correspond to adjusted asset id + final placed = find.byKey(const Key('placed_signature_0')); + expect(placed, findsOneWidget); + // Compare the placed image bytes with processed bytes at confirm time + final ctx3 = tester.element(find.byType(MaterialApp)); + final container3 = ProviderScope.containerOf(ctx3); + final processed = container3.read(processedSignatureImageProvider); + expect(processed, isNotNull); + final pdf = container3.read(pdfProvider); + final imgId = pdf.placementImageByPage[pdf.currentPage]?.first; + expect(imgId, isNotNull); + final lib = container3.read(signatureLibraryProvider); + final match = lib.firstWhere((a) => a.id == imgId); + expect(match.bytes, equals(processed)); }); }