diff --git a/lib/providers/collab/active_page_live_sync_provider.dart b/lib/providers/collab/active_page_live_sync_provider.dart index 60551c02..aa6cde90 100644 --- a/lib/providers/collab/active_page_live_sync_provider.dart +++ b/lib/providers/collab/active_page_live_sync_provider.dart @@ -588,7 +588,8 @@ class ActivePageLiveSyncNotifier extends Notifier { ); } - for (final text in ref.read(textProvider)) { + for (final text + in ref.read(textProvider.notifier).snapshotForPersistence()) { final payload = Map.from(text.toJson()) ..putIfAbsent('elementType', () => _CollabElementKind.text.name); envelopes.add( diff --git a/lib/providers/strategy_provider.dart b/lib/providers/strategy_provider.dart index a670d1eb..a5a226e7 100644 --- a/lib/providers/strategy_provider.dart +++ b/lib/providers/strategy_provider.dart @@ -23,6 +23,7 @@ import 'package:icarus/providers/user_preferences_provider.dart'; import 'package:icarus/providers/strategy_page.dart'; import 'package:icarus/providers/strategy_settings_provider.dart'; import 'package:icarus/providers/text_provider.dart'; +import 'package:icarus/providers/text_draft_provider.dart'; import 'package:icarus/providers/transition_provider.dart'; import 'package:icarus/providers/utility_provider.dart'; import 'package:hive_ce/hive.dart'; @@ -184,8 +185,9 @@ class StrategyProvider extends Notifier { return true; } + final hasTextDrafts = ref.read(textDraftProvider).isNotEmpty; final saveState = ref.read(strategySaveStateProvider); - if (!saveState.isDirty) { + if (!saveState.isDirty && !hasTextDrafts) { return true; } diff --git a/lib/services/unsaved_strategy_guard.dart b/lib/services/unsaved_strategy_guard.dart index c6e82c40..eb4c7ee4 100644 --- a/lib/services/unsaved_strategy_guard.dart +++ b/lib/services/unsaved_strategy_guard.dart @@ -6,6 +6,7 @@ import 'package:icarus/providers/collab/cloud_media_upload_queue_provider.dart'; import 'package:icarus/providers/collab/strategy_op_queue_provider.dart'; import 'package:icarus/providers/strategy_save_state_provider.dart'; import 'package:icarus/providers/strategy_provider.dart'; +import 'package:icarus/providers/text_draft_provider.dart'; import 'package:icarus/services/app_error_reporter.dart'; import 'package:icarus/strategy/strategy_page_models.dart'; import 'package:shadcn_ui/shadcn_ui.dart'; @@ -141,6 +142,24 @@ Future _guardCloudStrategyExit({ required WidgetRef ref, required Future Function() onContinue, }) async { + final openingStrategy = ref.read(strategyProvider); + if (ref.read(textDraftProvider).isNotEmpty && + openingStrategy.strategyId != null) { + try { + await ref + .read(strategyProvider.notifier) + .forceSaveNow(openingStrategy.strategyId!); + } catch (error, stackTrace) { + AppErrorReporter.reportError( + 'Failed to sync the active text edit before leaving.', + error: error, + stackTrace: stackTrace, + source: 'cloud_media.exit_guard', + ); + return false; + } + } + while (true) { final strategyState = ref.read(strategyProvider); final saveState = ref.read(strategySaveStateProvider); @@ -279,7 +298,9 @@ Future guardUnsavedStrategyExit({ ); } - if (strategyState.strategyName == null || !saveState.isDirty) { + final hasTextDrafts = ref.read(textDraftProvider).isNotEmpty; + if (strategyState.strategyName == null || + (!saveState.isDirty && !hasTextDrafts)) { await onContinue(); return true; } diff --git a/lib/widgets/cloud_sync_status_chip.dart b/lib/widgets/cloud_sync_status_chip.dart index 365db773..66376b5d 100644 --- a/lib/widgets/cloud_sync_status_chip.dart +++ b/lib/widgets/cloud_sync_status_chip.dart @@ -9,13 +9,14 @@ import 'package:icarus/providers/collab/strategy_conflict_provider.dart'; import 'package:icarus/providers/collab/strategy_op_queue_provider.dart'; import 'package:icarus/providers/strategy_provider.dart'; import 'package:icarus/providers/strategy_save_state_provider.dart'; +import 'package:icarus/providers/text_draft_provider.dart'; import 'package:icarus/strategy/strategy_page_models.dart'; import 'package:shadcn_ui/shadcn_ui.dart'; const _chipSwitchDuration = Duration(milliseconds: 150); const _conflictToastGap = Duration(seconds: 5); -enum _SyncStatus { synced, syncing, offline, attention } +enum _SyncStatus { synced, editing, syncing, offline, attention } /// Persistent cloud sync indicator for the strategy editor top strip. /// @@ -103,6 +104,9 @@ class _CloudSyncStatusChipState extends ConsumerState { final saveState = ref.watch(strategySaveStateProvider); final opQueueState = ref.watch(strategyOpQueueProvider); + final hasTextDrafts = ref.watch( + textDraftProvider.select((drafts) => drafts.isNotEmpty), + ); final isConnected = ref.watch(convexConnectionProvider).valueOrNull ?? true; final _SyncStatus status; @@ -112,6 +116,8 @@ class _CloudSyncStatusChipState extends ConsumerState { status = _SyncStatus.attention; } else if (!isConnected) { status = _SyncStatus.offline; + } else if (hasTextDrafts) { + status = _SyncStatus.editing; } else if (saveState.isSaving || saveState.hasPendingCloudSync || saveState.hasPendingMediaSync || @@ -190,6 +196,7 @@ class _CloudSyncStatusChipState extends ConsumerState { case _SyncStatus.attention: return Settings.tacticalVioletTheme.destructive.withValues(alpha: 0.14); case _SyncStatus.offline: + case _SyncStatus.editing: case _SyncStatus.syncing: case _SyncStatus.synced: return Settings.tacticalVioletTheme.muted.withValues(alpha: 0.4); @@ -201,6 +208,7 @@ class _CloudSyncStatusChipState extends ConsumerState { case _SyncStatus.attention: return Settings.tacticalVioletTheme.destructive; case _SyncStatus.offline: + case _SyncStatus.editing: case _SyncStatus.syncing: case _SyncStatus.synced: return Settings.tacticalVioletTheme.mutedForeground; @@ -217,6 +225,13 @@ class _CloudSyncStatusChipState extends ConsumerState { size: 13, color: color, ); + case _SyncStatus.editing: + return Icon( + Icons.edit_outlined, + key: const ValueKey('editing'), + size: 13, + color: color, + ); case _SyncStatus.syncing: return SizedBox( key: const ValueKey('syncing'), @@ -248,6 +263,8 @@ class _CloudSyncStatusChipState extends ConsumerState { switch (status) { case _SyncStatus.synced: return 'Synced'; + case _SyncStatus.editing: + return 'Editing…'; case _SyncStatus.syncing: return 'Syncing…'; case _SyncStatus.offline: @@ -324,6 +341,8 @@ class _SyncStatusPopover extends StatelessWidget { switch (status) { case _SyncStatus.synced: return 'All changes synced'; + case _SyncStatus.editing: + return 'Edit not synced yet'; case _SyncStatus.syncing: return 'Syncing changes'; case _SyncStatus.offline: @@ -337,6 +356,9 @@ class _SyncStatusPopover extends StatelessWidget { switch (status) { case _SyncStatus.synced: return 'Your strategy is safely stored in the cloud.'; + case _SyncStatus.editing: + return 'Finish editing or switch pages to send this change to the ' + 'cloud.'; case _SyncStatus.syncing: return 'Your edits are being sent to the cloud. You can keep ' 'working — this happens in the background.'; diff --git a/lib/widgets/draggable_widgets/text/text_widget.dart b/lib/widgets/draggable_widgets/text/text_widget.dart index f451885d..676c597b 100644 --- a/lib/widgets/draggable_widgets/text/text_widget.dart +++ b/lib/widgets/draggable_widgets/text/text_widget.dart @@ -79,12 +79,14 @@ class _EditableTextWidgetState extends ConsumerState<_EditableTextWidget> { late final FocusNode _focusNode; late final TextDraftProvider _draftNotifier; late final ProviderSubscription> _draftSubscription; + bool _syncingController = false; @override void initState() { super.initState(); _draftNotifier = ref.read(textDraftProvider.notifier); _controller = TextEditingController(text: _effectiveText()); + _controller.addListener(_onControllerChanged); _focusNode = FocusNode()..addListener(_onFocusChange); _draftSubscription = ref.listenManual>( textDraftProvider, @@ -115,7 +117,9 @@ class _EditableTextWidgetState extends ConsumerState<_EditableTextWidget> { _focusNode ..removeListener(_onFocusChange) ..dispose(); - _controller.dispose(); + _controller + ..removeListener(_onControllerChanged) + ..dispose(); super.dispose(); } @@ -128,6 +132,17 @@ class _EditableTextWidgetState extends ConsumerState<_EditableTextWidget> { _draftNotifier.commitDraft(widget.id); } + void _onControllerChanged() { + if (_syncingController) return; + + final nextText = _controller.text; + final currentText = _draftNotifier.draftFor(widget.id) ?? widget.text; + if (nextText == currentText) return; + + _draftNotifier.setDraft(widget.id, nextText); + if (mounted) setState(() {}); + } + void _syncControllerWithExternalState() { if (!_controller.value.isComposingRangeValid) { _controller.clearComposing(); @@ -141,12 +156,17 @@ class _EditableTextWidgetState extends ConsumerState<_EditableTextWidget> { final extentOffset = selection.extentOffset.clamp(0, nextText.length).toInt(); - _controller.value = TextEditingValue( - text: nextText, - selection: selection.isValid - ? TextSelection(baseOffset: baseOffset, extentOffset: extentOffset) - : TextSelection.collapsed(offset: nextText.length), - ); + _syncingController = true; + try { + _controller.value = TextEditingValue( + text: nextText, + selection: selection.isValid + ? TextSelection(baseOffset: baseOffset, extentOffset: extentOffset) + : TextSelection.collapsed(offset: nextText.length), + ); + } finally { + _syncingController = false; + } } void _updateMeasuredSize() { @@ -183,10 +203,6 @@ class _EditableTextWidgetState extends ConsumerState<_EditableTextWidget> { controller: _controller, focusNode: _focusNode, fontSize: widget.fontSize, - onChanged: (value) { - _draftNotifier.setDraft(widget.id, value); - setState(() {}); - }, onTapOutside: (_) { _focusNode.unfocus(); }, @@ -272,7 +288,6 @@ class _SharedTextField extends StatelessWidget { this.readOnly = false, this.enableInteractiveSelection = true, this.showCursor = true, - this.onChanged, this.onTapOutside, }); @@ -282,13 +297,12 @@ class _SharedTextField extends StatelessWidget { final bool readOnly; final bool enableInteractiveSelection; final bool showCursor; - final ValueChanged? onChanged; final TapRegionCallback? onTapOutside; @override Widget build(BuildContext context) { final coordinateSystem = CoordinateSystem.instance; - return MediaQuery( + final textField = MediaQuery( data: MediaQuery.of(context).copyWith(textScaler: TextScaler.noScaling), child: TextField( focusNode: focusNode, @@ -308,10 +322,16 @@ class _SharedTextField extends StatelessWidget { scrollPadding: EdgeInsets.zero, textAlignVertical: TextAlignVertical.top, keyboardType: TextInputType.multiline, - onChanged: onChanged, onTapOutside: onTapOutside, ), ); + + return MergeSemantics( + child: Semantics( + label: 'Placed text', + child: textField, + ), + ); } } diff --git a/test/strategy_page_session_provider_test.dart b/test/strategy_page_session_provider_test.dart index 1e8fc5f4..fef0accb 100644 --- a/test/strategy_page_session_provider_test.dart +++ b/test/strategy_page_session_provider_test.dart @@ -954,19 +954,22 @@ void main() { selectFirstPageIfNeeded: true, ); container.read(textProvider.notifier).fromHive([ - PlacedText(id: 'local-edit', position: const Offset(5, 5)) - ..text = 'unsent', + PlacedText(id: 'text-page-1', position: const Offset(5, 5))..text = 'one', ]); + container + .read(textDraftProvider.notifier) + .setDraft('text-page-1', 'unsent draft'); await session.setActivePage('page-2').timeout(const Duration(seconds: 2)); expect(session.activePageId, 'page-2'); expect(remote.selectedPageIds, contains('page-2')); expect(container.read(textProvider).single.text, 'two'); + expect(container.read(textDraftProvider), isEmpty); expect(queue.flushNowCount, 1); expect( container.read(strategyOpQueueProvider).pending.any((pending) => - pending.op.pagePublicId == 'page-1' || - pending.op.entityPublicId == 'page-1'), + pending.op.entityPublicId == 'text-page-1' && + pending.op.payload.toString().contains('unsent draft')), isTrue, ); }); diff --git a/test/text_widget_resilience_test.dart b/test/text_widget_resilience_test.dart index cbcbb488..e59150a0 100644 --- a/test/text_widget_resilience_test.dart +++ b/test/text_widget_resilience_test.dart @@ -1,6 +1,7 @@ import 'dart:io'; import 'package:flutter/material.dart'; +import 'package:flutter/semantics.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:hive_ce/hive.dart'; @@ -174,6 +175,68 @@ void main() { expect(container.read(textProvider).single.text, 'edited'); }); + testWidgets('semantics text changes enter the draft and commit pipeline', + (tester) async { + final semanticsHandle = tester.ensureSemantics(); + final container = createContainer(); + container.read(textProvider.notifier).fromHive([ + PlacedText(id: 'text-1', position: const Offset(10, 20))..text = 'before', + ]); + + await tester.pumpWidget(buildTextHarness(container)); + + await tester.tap(find.byType(TextField)); + await tester.pump(); + final placedText = find.semantics.byLabel('Placed text'); + expect(placedText, findsOneWidget); + tester.semantics.performAction( + placedText, + SemanticsAction.setText, + args: 'edited through semantics', + ); + await tester.pump(); + + expect( + container.read(textDraftProvider), + {'text-1': 'edited through semantics'}, + ); + expect(container.read(textProvider).single.text, 'before'); + + final field = tester.widget(find.byType(TextField)); + field.focusNode!.unfocus(); + await tester.pump(); + + expect(container.read(textDraftProvider), isEmpty); + expect( + container.read(textProvider).single.text, + 'edited through semantics', + ); + semanticsHandle.dispose(); + }); + + testWidgets('controller text changes enter the draft pipeline', + (tester) async { + final container = createContainer(); + container.read(textProvider.notifier).fromHive([ + PlacedText(id: 'text-1', position: const Offset(10, 20))..text = 'before', + ]); + + await tester.pumpWidget(buildTextHarness(container)); + + final field = tester.widget(find.byType(TextField)); + field.controller!.value = const TextEditingValue( + text: 'edited through controller', + selection: TextSelection.collapsed(offset: 25), + ); + await tester.pump(); + + expect( + container.read(textDraftProvider), + {'text-1': 'edited through controller'}, + ); + expect(container.read(textProvider).single.text, 'before'); + }); + testWidgets( 'drag start commits the draft before the drag lifecycle swaps children', (tester) async { diff --git a/test/unsaved_strategy_guard_test.dart b/test/unsaved_strategy_guard_test.dart index 86bd3f43..10331e44 100644 --- a/test/unsaved_strategy_guard_test.dart +++ b/test/unsaved_strategy_guard_test.dart @@ -194,7 +194,8 @@ void main() { expect(saved!.pages.single.textData.single.text, 'before'); }); - test('already saved returns true without another save', () async { + test('an active draft is saved even when committed state was clean', + () async { await _setAutosaveEnabled(true); final strategy = await _storeStrategyWithText( id: 'strategy-3', @@ -207,7 +208,7 @@ void main() { .fromHive(strategy.pages.single.textData); container .read(textDraftProvider.notifier) - .setDraft('text-1', 'draft should not save'); + .setDraft('text-1', 'active draft'); final notifier = container.read(strategyProvider.notifier); notifier.setFromState( @@ -226,7 +227,7 @@ void main() { final saved = Hive.box(HiveBoxNames.strategiesBox).get(strategy.id); expect(saved, isNotNull); - expect(saved!.pages.single.textData.single.text, 'before'); + expect(saved!.pages.single.textData.single.text, 'active draft'); }); test('no loaded strategy returns true', () async { @@ -336,6 +337,46 @@ void main() { expect(find.text('Save changes?'), findsNothing); }); + testWidgets('clean local state still flushes an active text draft', + (tester) async { + notifier = _FakeGuardStrategyProvider( + initialState: const StrategyState( + strategyId: 'strategy-draft', + strategyName: 'Draft Strategy', + source: StrategySource.local, + storageDirectory: null, + isOpen: true, + ), + flushResult: true, + ); + container = ProviderContainer( + overrides: [ + strategyProvider.overrideWith(() => notifier), + ], + ); + addTearDown(container.dispose); + container + .read(textDraftProvider.notifier) + .setDraft('text-1', 'active local draft'); + await pumpHarness(tester); + + var continueCalls = 0; + final result = await guardUnsavedStrategyExit( + context: context, + ref: ref, + source: 'guard-test-local-draft', + onContinue: () async { + continueCalls++; + }, + ); + await tester.pumpAndSettle(); + + expect(result, isTrue); + expect(continueCalls, 1); + expect(notifier.flushCalls, 1); + expect(find.text('Save changes?'), findsNothing); + }); + testWidgets( 'dirty autosave-disabled exit shows dialog and save still works', (tester) async { diff --git a/test/widgets/cloud_sync_status_chip_test.dart b/test/widgets/cloud_sync_status_chip_test.dart new file mode 100644 index 00000000..fa2e6b23 --- /dev/null +++ b/test/widgets/cloud_sync_status_chip_test.dart @@ -0,0 +1,114 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:icarus/providers/collab/cloud_media_upload_queue_provider.dart'; +import 'package:icarus/providers/collab/convex_connection_provider.dart'; +import 'package:icarus/providers/collab/strategy_op_queue_provider.dart'; +import 'package:icarus/providers/strategy_provider.dart'; +import 'package:icarus/providers/text_draft_provider.dart'; +import 'package:icarus/strategy/strategy_models.dart'; +import 'package:icarus/strategy/strategy_page_models.dart'; +import 'package:icarus/widgets/cloud_sync_status_chip.dart'; +import 'package:shadcn_ui/shadcn_ui.dart'; + +class _CloudStrategyProvider extends StrategyProvider { + @override + StrategyState build() => const StrategyState( + strategyId: 'cloud-strategy', + strategyName: 'Cloud Strategy', + source: StrategySource.cloud, + storageDirectory: null, + isOpen: true, + ); +} + +class _SettledOpQueue extends StrategyOpQueueNotifier { + @override + StrategyOpQueueState build() => const StrategyOpQueueState( + accountId: 'account-a', + strategyPublicId: 'cloud-strategy', + clientId: 'client-a', + durableLoaded: true, + ); +} + +class _EmptyMediaQueue extends CloudMediaUploadQueueNotifier { + @override + CloudMediaUploadQueueState build() => const CloudMediaUploadQueueState( + jobs: [], + isProcessing: false, + ); +} + +ProviderContainer _createContainer({bool connected = true}) { + return ProviderContainer( + overrides: [ + strategyProvider.overrideWith(_CloudStrategyProvider.new), + strategyOpQueueProvider.overrideWith(_SettledOpQueue.new), + cloudMediaUploadQueueProvider.overrideWith(_EmptyMediaQueue.new), + convexConnectionProvider.overrideWith((ref) => Stream.value(connected)), + ], + ); +} + +void main() { + testWidgets('an active text draft can never appear synced', (tester) async { + final container = _createContainer(); + addTearDown(container.dispose); + + await tester.pumpWidget( + UncontrolledProviderScope( + container: container, + child: const ShadApp( + home: Scaffold(body: CloudSyncStatusChip()), + ), + ), + ); + await tester.pump(); + + expect(find.text('Synced'), findsOneWidget); + + container + .read(textDraftProvider.notifier) + .setDraft('text-1', 'visible local edit'); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 200)); + + expect(find.text('Editing…'), findsOneWidget); + expect(find.text('Synced'), findsNothing); + + await tester.tap(find.text('Editing…')); + await tester.pumpAndSettle(); + expect(find.text('Edit not synced yet'), findsOneWidget); + expect( + find.text( + 'Finish editing or switch pages to send this change to the cloud.', + ), + findsOneWidget, + ); + }); + + testWidgets('offline remains visible while a text draft is active', + (tester) async { + final container = _createContainer(connected: false); + addTearDown(container.dispose); + container + .read(textDraftProvider.notifier) + .setDraft('text-1', 'offline edit'); + + await tester.pumpWidget( + UncontrolledProviderScope( + container: container, + child: const ShadApp( + home: Scaffold(body: CloudSyncStatusChip()), + ), + ), + ); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 200)); + + expect(find.text('Offline'), findsOneWidget); + expect(find.text('Editing…'), findsNothing); + expect(find.text('Synced'), findsNothing); + }); +}