From 580ac521e060371b1a7c7ada2cbb490e3f1dd9c4 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Fri, 9 Oct 2026 14:54:10 -0400 Subject: [PATCH 1/7] Keep a placement edit open when its page reloads Resolving an unrelated cloud conflict (Use cloud, Keep both) reloads the page on screen, which closed Edit placement and threw away the user's drags. A reload of the same page, the one that keeps undo history, now keeps the edit open on whichever of its lineups survive. A drag stays while the reload has its end where the drag started; an end the cloud moved or removed shows the cloud's version, with a toast. Opening another page still ends the edit without writing it. Co-Authored-By: Claude Opus 5.5 --- lib/const/line_provider.dart | 94 +++++++++-- lib/strategy/strategy_page_apply.dart | 8 +- lib/widgets/line_up_placement_editor.dart | 4 +- test/lineup_add_item_interaction_test.dart | 156 ++++++++++++++++++ test/per_object_undo_test.dart | 74 +++++++++ test/strategy_page_session_provider_test.dart | 96 +++++++++++ 6 files changed, 411 insertions(+), 21 deletions(-) diff --git a/lib/const/line_provider.dart b/lib/const/line_provider.dart index 06b4a0fe5..b2ec0f4df 100644 --- a/lib/const/line_provider.dart +++ b/lib/const/line_provider.dart @@ -1048,10 +1048,16 @@ class LineUpProvider extends Notifier { void startEdit(String linkId) { final start = state.linkById(linkId); if (start == null) return; + state = state.copyWith(edit: _editOf([start])); + } + + /// An edit of [links] and every lineup joined to them through a shared + /// origin or landing, with nothing moved yet. + LineUpPlacementEdit _editOf(Iterable links) { final linkIds = {}; final originIds = {}; final landingIds = {}; - final pending = [start]; + final pending = [...links]; while (pending.isNotEmpty) { final link = pending.removeLast(); if (!linkIds.add(link.id)) continue; @@ -1061,12 +1067,10 @@ class LineUpProvider extends Notifier { other.originId == link.originId || other.landingId == link.landingId)); } - state = state.copyWith( - edit: LineUpPlacementEdit( - linkIds: linkIds, - originIds: originIds, - landingIds: landingIds, - ), + return LineUpPlacementEdit( + linkIds: linkIds, + originIds: originIds, + landingIds: landingIds, ); } @@ -1567,19 +1571,73 @@ class LineUpProvider extends Notifier { // --- Lifecycle and serialization ----------------------------------------- - /// Replaces the page's lineups. A placement edit ends with them: its ids - /// may name lineups a copied page shares, and saving it there would move - /// the wrong page's lineups. - void fromHive(LineUpGraph graph) { + /// Replaces the page's lineups. A placement edit ends with them unless + /// [graph] is a newer copy of the same page, say after a cloud conflict + /// was resolved: its ids may name lineups a copied page shares, and saving + /// it there would move the wrong page's lineups. + /// + /// On the same page the edit stays open on whichever of its lineups are + /// still there, joined to any that now share their spots. A drag stays + /// while the newer copy has its end where the drag started; an end the + /// newer copy moved or removed shows the newer copy, and the user is told. + void fromHive(LineUpGraph graph, {bool samePage = false}) { + final before = state; final copy = graph.deepCopy(); - final droppedMoves = state.editMovesAnything; state = state.copyWith( origins: copy.origins, landings: copy.landings, links: copy.links, edit: null, ); - if (droppedMoves) _toastDroppedEdit(); + final edit = before.edit; + if (edit == null) return; + if (!samePage) { + if (before.editMovesAnything) { + _toastDroppedEdit('The page changed, so the placement edit closed ' + 'without saving.'); + } + return; + } + final survivors = edit.linkIds.map(state.linkById).nonNulls; + if (survivors.isEmpty) { + if (before.editMovesAnything) { + _toastDroppedEdit('The lineups you were moving are no longer on this ' + 'page, so the placement edit closed without saving.'); + } + return; + } + final reopened = _editOf(survivors); + var dropped = false; + Map keptDrags( + Map drags, + Set ids, + Offset? Function(LineUpState state, String id) positionOf, + ) { + final kept = {}; + for (final MapEntry(key: id, value: draft) in drags.entries) { + final was = positionOf(before, id); + if (was == null || was == draft) continue; + if (ids.contains(id) && positionOf(state, id) == was) { + kept[id] = draft; + } else { + dropped = true; + } + } + return kept; + } + + state = state.copyWith( + edit: reopened.copyWith( + movedOrigins: keptDrags(edit.movedOrigins, reopened.originIds, + (state, id) => state.originById(id)?.agent.position), + movedLandings: keptDrags(edit.movedLandings, reopened.landingIds, + (state, id) => state.landingById(id)?.ability.position), + ), + ); + if (dropped) { + _toastDroppedEdit("The cloud's version changed a spot you dragged, so " + 'that drag was undone.'); + } } /// Takes [graph] as the server's lineups merged with the ones the user @@ -1629,13 +1687,15 @@ class LineUpProvider extends Notifier { void clearAll() { final droppedMoves = state.editMovesAnything; state = state.copyWith(origins: [], landings: [], links: [], edit: null); - if (droppedMoves) _toastDroppedEdit(); + if (droppedMoves) { + _toastDroppedEdit('The page changed, so the placement edit closed ' + 'without saving.'); + } } - void _toastDroppedEdit() { + void _toastDroppedEdit(String message) { Settings.showToast( - message: 'The page changed, so the placement edit closed without ' - 'saving.', + message: message, backgroundColor: Settings.tacticalVioletTheme.destructive, ); } diff --git a/lib/strategy/strategy_page_apply.dart b/lib/strategy/strategy_page_apply.dart index 5fc118f68..38088a19b 100644 --- a/lib/strategy/strategy_page_apply.dart +++ b/lib/strategy/strategy_page_apply.dart @@ -32,7 +32,6 @@ Future applyStrategyEditorPageData( ref.read(textProvider.notifier).clearAll(); ref.read(placedImageProvider.notifier).clearAll(); ref.read(utilityProvider.notifier).clearAll(); - ref.read(lineUpProvider.notifier).clearAll(); if (!preserveHistory) { ref.read(actionProvider.notifier).clearActionHistory(); } @@ -42,7 +41,12 @@ Future applyStrategyEditorPageData( ref.read(textProvider.notifier).fromHive(data.texts); ref.read(placedImageProvider.notifier).fromHive(data.images); ref.read(utilityProvider.notifier).fromHive(data.utilities); - ref.read(lineUpProvider.notifier).fromHive(data.lineUpGraph); + // Only a reload of the page on screen keeps its undo history, and only + // that one keeps a placement edit open; fromHive replaces the lineups, so + // they are not cleared first. + ref + .read(lineUpProvider.notifier) + .fromHive(data.lineUpGraph, samePage: preserveHistory); ref .read(mapProvider.notifier) .fromHive(mapOverride ?? data.map, data.isAttack); diff --git a/lib/widgets/line_up_placement_editor.dart b/lib/widgets/line_up_placement_editor.dart index c5dec70fc..48988a913 100644 --- a/lib/widgets/line_up_placement_editor.dart +++ b/lib/widgets/line_up_placement_editor.dart @@ -80,8 +80,8 @@ class _LineUpPlacementEditorState extends ConsumerState { final coordinateSystem = CoordinateSystem.instance; final state = ref.watch(lineUpProvider); final edit = state.edit; - // The page's lineups were replaced (another page opened) and the edit - // went with them. + // The page's lineups were replaced (another page opened, or a reload + // took every lineup being moved) and the edit went with them. if (edit == null) { _leave(); return const SizedBox.shrink(); diff --git a/test/lineup_add_item_interaction_test.dart b/test/lineup_add_item_interaction_test.dart index 57bfa1959..0e6217905 100644 --- a/test/lineup_add_item_interaction_test.dart +++ b/test/lineup_add_item_interaction_test.dart @@ -158,6 +158,9 @@ Future _pumpLineupCanvas( ); } +/// How far [editAndDragAbility] drags the ability. +const _editDrag = Offset(-55, 65); + void main() { TestWidgetsFlutterBinding.ensureInitialized(); @@ -610,6 +613,159 @@ void main() { await tester.pumpAndSettle(); }); + /// Opens placement editing on the Breach lineup and drags its ability by + /// [_editDrag]. Returns the lineup as it was placed. + Future editAndDragAbility( + WidgetTester tester, + ProviderContainer container, + ) async { + final group = _breachGroup(); + container + .read(lineUpProvider.notifier) + .fromHive(LineUpGraph.fromLegacyGroups([group])); + container.read(interactionStateProvider.notifier).editLineUpPlacement( + container.read(lineUpProvider).links.single.id, + ); + + CoordinateSystem(playAreaSize: const Size(900, 600)); + await _pumpHarness( + tester, + container: container, + child: const SizedBox( + width: 900, + height: 600, + child: Stack( + children: [ + LineUpOverlay(), + Positioned.fill(child: LineUpPlacementEditor()), + Align( + alignment: Alignment.bottomRight, + child: LineupControlButtons(), + ), + ], + ), + ), + ); + await tester.drag( + find.byKey(const ValueKey('lineup-ability-drag-breach-item')), + _editDrag, + ); + await tester.pump(); + return group; + } + + /// The Breach lineup [group] with its ability at [abilityAt], as a newer + /// copy of the page would bring it. + LineUpGraph reloaded(LineUpGroup group, {required Offset abilityAt}) { + final item = group.items.single; + return LineUpGraph.fromLegacyGroups([ + group.copyWith(items: [ + item.copyWith(ability: item.ability.copyWith(position: abilityAt)), + ]), + ]); + } + + testWidgets('reloading the same page keeps a placement edit and its drag', + (tester) async { + final container = _createContainer(); + final group = await editAndDragAbility(tester, container); + final abilityFinder = + find.byKey(const ValueKey('lineup-ability-drag-breach-item')); + final draggedTopLeft = tester.getTopLeft(abilityFinder); + final draft = + container.read(lineUpProvider).edit!.movedLandings['breach-item']!; + + // Resolving a cloud conflict reloads the page on screen. + container.read(lineUpProvider.notifier).fromHive( + reloaded(group, abilityAt: group.items.single.ability.position), + samePage: true, + ); + await tester.pumpAndSettle(); + + expect( + container.read(interactionStateProvider), + InteractionState.lineUpEditing, + ); + expect(tester.getTopLeft(abilityFinder), draggedTopLeft); + expect(find.textContaining('undone'), findsNothing); + expect(find.textContaining('without saving'), findsNothing); + + await tester.tap(find.byKey(const ValueKey('lineup-edit-save'))); + await tester.pumpAndSettle(); + expect( + container + .read(lineUpProvider) + .landingById('breach-item')! + .ability + .position, + draft, + ); + expect(container.read(actionProvider), hasLength(1)); + }); + + testWidgets('a reload that moved a dragged spot undoes that drag and says so', + (tester) async { + final container = _createContainer(); + final group = await editAndDragAbility(tester, container); + const theirs = Offset(500, 300); + + container.read(lineUpProvider.notifier).fromHive( + reloaded(group, abilityAt: theirs), + samePage: true, + ); + await tester.pumpAndSettle(); + + // Still editing, the ability where the cloud has it. + expect( + container.read(interactionStateProvider), + InteractionState.lineUpEditing, + ); + expect(container.read(lineUpProvider).edit!.movedLandings, isEmpty); + expect( + tester.getTopLeft( + find.byKey(const ValueKey('lineup-ability-drag-breach-item'))), + screenPositionForWidget( + widget: container.read(lineUpProvider).landings.single.ability, + coordinateSystem: CoordinateSystem.instance, + mapScale: Maps.mapScale[MapValue.bind] ?? 1.0, + abilitySize: container.read(strategySettingsProvider).abilitySize, + isAttack: true, + ), + ); + expect( + find.text("The cloud's version changed a spot you dragged, so that " + 'drag was undone.'), + findsOneWidget, + ); + await tester.pump(const Duration(seconds: 4)); + await tester.pumpAndSettle(); + }); + + testWidgets('a reload without the lineups being moved ends the edit', + (tester) async { + final container = _createContainer(); + await editAndDragAbility(tester, container); + + // "Use cloud" took the lineup away. + container + .read(lineUpProvider.notifier) + .fromHive(LineUpGraph.empty, samePage: true); + await tester.pumpAndSettle(); + + expect(container.read(lineUpProvider).edit, isNull); + expect( + container.read(interactionStateProvider), + InteractionState.navigation, + ); + expect( + find.text('The lineups you were moving are no longer on this page, so ' + 'the placement edit closed without saving.'), + findsOneWidget, + ); + await tester.pump(const Duration(seconds: 4)); + await tester.pumpAndSettle(); + }); + testWidgets('lineup origin opens its menu on right-click', (tester) async { final container = _createContainer(); final group = _breachGroup(); diff --git a/test/per_object_undo_test.dart b/test/per_object_undo_test.dart index d8c2c68a1..19cdd9d39 100644 --- a/test/per_object_undo_test.dart +++ b/test/per_object_undo_test.dart @@ -294,6 +294,80 @@ void main() { ); }); + test('a reload of the same page keeps a placement edit and one undo step', + () { + final container = _container(); + final lineUps = container.read(lineUpProvider.notifier) + ..fromHive(oneLineup()); + final history = container.read(actionProvider.notifier); + LineUpState lineUpState() => container.read(lineUpProvider); + + lineUps + ..startEdit('k') + ..moveEditedOrigin('o', const Offset(50, 50)) + ..saveEdit(); + lineUps + ..startEdit('k') + ..moveEditedLanding('l', const Offset(90, 90)); + // The same page comes back from the cloud, the earlier save in it. + lineUps.fromHive(lineUpState().graph, samePage: true); + + expect(lineUpState().edit!.linkIds, {'k'}); + expect(lineUpState().edit!.movedLandings, {'l': const Offset(90, 90)}); + lineUps.saveEdit(); + expect(container.read(actionProvider), hasLength(2)); + expect( + lineUpState().landingById('l')!.ability.position, + const Offset(90, 90), + ); + + // One undo takes back only the save made after the reload. + history.undoAction(); + expect( + lineUpState().landingById('l')!.ability.position, + const Offset(20, 80), + ); + expect(lineUpState().originById('o')!.agent.position, const Offset(50, 50)); + }); + + test('a lineup a reload adds on an edited spot joins the edit', () { + final container = _container(); + final lineUps = container.read(lineUpProvider.notifier) + ..fromHive(oneLineup()); + lineUps + ..startEdit('k') + ..moveEditedOrigin('o', const Offset(50, 50)); + + // A teammate added a lineup thrown from the same agent. + final graph = oneLineup(); + lineUps.fromHive( + LineUpGraph( + origins: graph.origins, + landings: [ + ...graph.landings, + LineUpLanding( + id: 'l2', + ability: PlacedAbility( + id: 'b2', + data: AgentData.agents[AgentType.jett]!.abilities.first, + position: const Offset(70, 80), + ), + ), + ], + links: [ + ...graph.links, + LineUpLink(id: 'k2', originId: 'o', landingId: 'l2'), + ], + ), + samePage: true, + ); + + final edit = container.read(lineUpProvider).edit!; + expect(edit.linkIds, {'k', 'k2'}); + expect(edit.landingIds, {'l', 'l2'}); + expect(edit.movedOrigins, {'o': const Offset(50, 50)}); + }); + test('one undo steps past entries that change nothing to one that does', () { final container = _container(); final agents = container.read(agentProvider.notifier) diff --git a/test/strategy_page_session_provider_test.dart b/test/strategy_page_session_provider_test.dart index 484da0f63..11239a986 100644 --- a/test/strategy_page_session_provider_test.dart +++ b/test/strategy_page_session_provider_test.dart @@ -25,6 +25,7 @@ import 'package:icarus/providers/collab/convex_connection_provider.dart'; import 'package:icarus/providers/drawing_provider.dart'; import 'package:icarus/providers/editor_operation_provider.dart'; import 'package:icarus/providers/image_provider.dart'; +import 'package:icarus/providers/interaction_state_provider.dart'; import 'package:icarus/providers/utility_provider.dart'; import 'package:icarus/collab/cloud_payload_upgrade.dart'; import 'package:icarus/const/agents.dart'; @@ -7371,6 +7372,101 @@ void main() { expect(container.read(lineupConflictsProvider), isNull); }); + for (final keepBoth in [false, true]) { + test( + '${keepBoth ? 'Keep both' : 'Use cloud'} on another group keeps a ' + 'placement edit open, and Save sends its drag as one undo step', + () async { + final page = _page('page-1', 0); + server = _FakeServer(page.publicId, lineups: [ + fanIn(page.publicId), + _lineup(page.publicId, 'z', sortIndex: 1), + ]); + final (container, _) = await openOnRealQueue(page); + final key = keyOf(page, 'link-a'); + StrategyOpQueueState queueState() => + container.read(strategyOpQueueProvider); + Offset serverLanding() => + lineUpGraphFromRemoteLineups([server.row('link-z')]) + .landings + .single + .ability + .position; + // The fan-in group's edit is refused beside a teammate's and waits, + // as in refusedBesideTeammate. + server.teammateEdit('link-a', + (data) => _entry(data, 'links', 'link-b')['notes'] = 'theirs'); + container.read(lineUpProvider.notifier).updateLink(container + .read(lineUpProvider) + .linkById('link-a')! + .copyWith(notes: 'mine')); + await _until(() => queueState().attentionByEntityKey.containsKey(key)); + for (var i = 0; i < 10; i++) { + await _settle(); + } + + // Mid-edit of lineup Z, its landing dragged, the user resolves the + // conflict on the other group. + const dragTo = Offset(400, 250); + container + .read(interactionStateProvider.notifier) + .editLineUpPlacement('link-z'); + container + .read(lineUpProvider.notifier) + .moveEditedLanding('landing-z', dragTo); + final session = container.read(strategyPageSessionProvider.notifier); + if (keepBoth) { + expect(await session.keepBothForRejected(), KeepBothOutcome.kept); + } else { + expect(await session.useCloudVersionsForRejected(), isTrue); + } + await _until(() => queueState().pending.isEmpty); + for (var i = 0; i < 10; i++) { + await _settle(); + } + + // The page reloaded under the edit: the cloud's version of the other + // group is on screen, and the edit is still open with its drag. + expect( + container.read(lineUpProvider).linkById('link-b')!.notes, 'theirs'); + expect(queueState().attentionByEntityKey, isEmpty); + expect(container.read(interactionStateProvider), + InteractionState.lineUpEditing); + final edit = container.read(lineUpProvider).edit!; + expect(edit.linkIds, {'link-z'}); + expect(edit.movedLandings, {'landing-z': dragTo}); + expect(serverLanding(), const Offset(30, 40)); + + final undoSteps = container.read(actionProvider).length; + container.read(lineUpProvider.notifier).saveEdit(); + container + .read(interactionStateProvider.notifier) + .update(InteractionState.navigation); + await _until(() => serverLanding() == dragTo); + await _until(() => queueState().pending.isEmpty); + for (var i = 0; i < 10; i++) { + await _settle(); + } + + expect(queueState().attentionByEntityKey, isEmpty); + expect(container.read(actionProvider), hasLength(undoSteps + 1)); + container.read(actionProvider.notifier).undoAction(); + expect( + container + .read(lineUpProvider) + .landingById('landing-z')! + .ability + .position, + const Offset(30, 40), + ); + await _until(() => serverLanding() == const Offset(30, 40)); + await _until(() => queueState().pending.isEmpty); + for (var i = 0; i < 10; i++) { + await _settle(); + } + }); + } + test( 'Keep both whose cloud version cannot be loaded changes nothing, so ' 'pressing it again cannot copy twice', () async { From eacec3f1552346d1fb1a673bdec106bd1d3219e2 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Fri, 9 Oct 2026 15:04:02 -0400 Subject: [PATCH 2/7] End a drag that a page reload took back A reload in the middle of a drag (pointer still down) left the editor's drag based on the old position: the next pointer move put back the drag the reload had just undone, and when the dragged end was gone, the next drag of another end started from its numbers. The editor now lets a drag carry on only if the reload kept its end where the drag began. fromHive sets the reopened edit in one assignment, so nothing watching sees the edit end on the way. Co-Authored-By: Claude Opus 5.5 --- lib/const/line_provider.dart | 74 +++++++---- lib/widgets/line_up_placement_editor.dart | 76 +++++++++-- test/lineup_add_item_interaction_test.dart | 148 +++++++++++++++++++++ 3 files changed, 261 insertions(+), 37 deletions(-) diff --git a/lib/const/line_provider.dart b/lib/const/line_provider.dart index b2ec0f4df..aa66e5a24 100644 --- a/lib/const/line_provider.dart +++ b/lib/const/line_provider.dart @@ -1048,12 +1048,15 @@ class LineUpProvider extends Notifier { void startEdit(String linkId) { final start = state.linkById(linkId); if (start == null) return; - state = state.copyWith(edit: _editOf([start])); + state = state.copyWith(edit: _editOf([start], within: state)); } - /// An edit of [links] and every lineup joined to them through a shared - /// origin or landing, with nothing moved yet. - LineUpPlacementEdit _editOf(Iterable links) { + /// An edit of [links] and every lineup in [within] joined to them through + /// a shared origin or landing, with nothing moved yet. + static LineUpPlacementEdit _editOf( + Iterable links, { + required LineUpState within, + }) { final linkIds = {}; final originIds = {}; final landingIds = {}; @@ -1063,7 +1066,7 @@ class LineUpProvider extends Notifier { if (!linkIds.add(link.id)) continue; originIds.add(link.originId); landingIds.add(link.landingId); - pending.addAll(state.links.where((other) => + pending.addAll(within.links.where((other) => other.originId == link.originId || other.landingId == link.landingId)); } @@ -1583,30 +1586,45 @@ class LineUpProvider extends Notifier { void fromHive(LineUpGraph graph, {bool samePage = false}) { final before = state; final copy = graph.deepCopy(); - state = state.copyWith( + final loaded = before.copyWith( origins: copy.origins, landings: copy.landings, links: copy.links, edit: null, ); + // Set once, so nothing watching sees the edit end on the way. + final (edit, lost) = samePage + ? _reopenEdit(before, loaded) + : ( + null, + before.editMovesAnything + ? 'The page changed, so the placement edit closed without ' + 'saving.' + : null, + ); + state = loaded.copyWith(edit: edit); + if (lost != null) _toastDroppedEdit(lost); + } + + /// [before]'s placement edit on [loaded], a newer copy of the same page + /// (see [fromHive]), and what to tell the user if drags were lost. + static (LineUpPlacementEdit?, String?) _reopenEdit( + LineUpState before, + LineUpState loaded, + ) { final edit = before.edit; - if (edit == null) return; - if (!samePage) { - if (before.editMovesAnything) { - _toastDroppedEdit('The page changed, so the placement edit closed ' - 'without saving.'); - } - return; - } - final survivors = edit.linkIds.map(state.linkById).nonNulls; + if (edit == null) return (null, null); + final survivors = edit.linkIds.map(loaded.linkById).nonNulls; if (survivors.isEmpty) { - if (before.editMovesAnything) { - _toastDroppedEdit('The lineups you were moving are no longer on this ' - 'page, so the placement edit closed without saving.'); - } - return; + return ( + null, + before.editMovesAnything + ? 'The lineups you were moving are no longer on this page, so ' + 'the placement edit closed without saving.' + : null, + ); } - final reopened = _editOf(survivors); + final reopened = _editOf(survivors, within: loaded); var dropped = false; Map keptDrags( Map drags, @@ -1617,7 +1635,7 @@ class LineUpProvider extends Notifier { for (final MapEntry(key: id, value: draft) in drags.entries) { final was = positionOf(before, id); if (was == null || was == draft) continue; - if (ids.contains(id) && positionOf(state, id) == was) { + if (ids.contains(id) && positionOf(loaded, id) == was) { kept[id] = draft; } else { dropped = true; @@ -1626,18 +1644,18 @@ class LineUpProvider extends Notifier { return kept; } - state = state.copyWith( - edit: reopened.copyWith( + return ( + reopened.copyWith( movedOrigins: keptDrags(edit.movedOrigins, reopened.originIds, (state, id) => state.originById(id)?.agent.position), movedLandings: keptDrags(edit.movedLandings, reopened.landingIds, (state, id) => state.landingById(id)?.ability.position), ), + dropped + ? "The cloud's version changed a spot you dragged, so that drag " + 'was undone.' + : null, ); - if (dropped) { - _toastDroppedEdit("The cloud's version changed a spot you dragged, so " - 'that drag was undone.'); - } } /// Takes [graph] as the server's lineups merged with the ones the user diff --git a/lib/widgets/line_up_placement_editor.dart b/lib/widgets/line_up_placement_editor.dart index 48988a913..e5e2f7ec0 100644 --- a/lib/widgets/line_up_placement_editor.dart +++ b/lib/widgets/line_up_placement_editor.dart @@ -29,6 +29,10 @@ String lineUpEditStatus(LineUpState state) { 'so they move together'; } +/// One end of an edited lineup: its id, and whether it is an origin (else a +/// landing). +typedef _End = (String id, bool origin); + /// Placement editing: the lineups being moved, drawn over the dimmed page at /// their draft positions. Each origin and landing follows the pointer with /// its lines; Save in LineupControlButtons writes the move. @@ -41,21 +45,34 @@ class LineUpPlacementEditor extends ConsumerStatefulWidget { } class _LineUpPlacementEditorState extends ConsumerState { - /// The drag under way: how far the pointer has moved in window pixels, - /// and the end's top-left in this layer's pixels and its stored position - /// where the drag began. - ({Offset moved, Offset topLeft, Offset position})? _drag; + /// The drag under way: which end, how far the pointer has moved in window + /// pixels, and the end's top-left in this layer's pixels and its stored + /// position where the drag began. + ({ + _End end, + Offset moved, + Offset topLeft, + Offset position, + })? _drag; + + /// The end whose drag a reload of the page took back while the pointer + /// was still down. The rest of that gesture moves nothing. + _End? _takenBack; - /// Where the drag puts the end's top-left, keeping the point that was - /// grabbed under the pointer. The movement adds up the updates' deltas: - /// the first update reports where the pointer went down, not where it is. - Offset _draggedTopLeft( + /// Where the drag of [end] puts its top-left, keeping the point that was + /// grabbed under the pointer, or null once a reload took the drag back. + /// The movement adds up the updates' deltas: the first update reports + /// where the pointer went down, not where it is. + Offset? _draggedTopLeft( DragUpdateDetails details, { + required _End end, required Offset topLeft, required Offset position, }) { + if (_takenBack == end) return null; final box = context.findRenderObject() as RenderBox; final drag = _drag = ( + end: end, moved: (_drag?.moved ?? Offset.zero) + details.delta, topLeft: _drag?.topLeft ?? topLeft, position: _drag?.position ?? position, @@ -75,9 +92,36 @@ class _LineUpPlacementEditorState extends ConsumerState { }); } + /// A reload of the page reopens the edit (see LineUpProvider.fromHive). + /// The drag under way carries on only if its end is still there, where + /// the drag began; otherwise the reload took the drag back, and following + /// the pointer would put it back. + void _onEditReopened(LineUpState next) { + final drag = _drag; + if (drag == null) return; + final edit = next.edit; + final (id, origin) = drag.end; + final now = origin + ? (edit?.originIds.contains(id) ?? false) + ? next.originById(id)?.agent.position + : null + : (edit?.landingIds.contains(id) ?? false) + ? next.landingById(id)?.ability.position + : null; + if (now == drag.position) return; + _drag = null; + _takenBack = drag.end; + } + @override Widget build(BuildContext context) { final coordinateSystem = CoordinateSystem.instance; + ref.listen(lineUpProvider, (previous, next) { + // Drags keep the edit's lineups; a new set means it was reopened. + if (!identical(previous?.edit?.linkIds, next.edit?.linkIds)) { + _onEditReopened(next); + } + }); final state = ref.watch(lineUpProvider); final edit = state.edit; // The page's lineups were replaced (another page opened, or a reload @@ -129,7 +173,15 @@ class _LineUpPlacementEditorState extends ConsumerState { return const SizedBox.shrink(); } - void endMove({required bool outOfBounds, required void Function() undo}) { + void endMove( + _End end, { + required bool outOfBounds, + required void Function() undo, + }) { + if (_takenBack == end) { + _takenBack = null; + return; + } // Dropped off the map: the end goes back to where the drag began. if (outOfBounds) undo(); _drag = null; @@ -163,6 +215,7 @@ class _LineUpPlacementEditorState extends ConsumerState { onMove: (details) { final topLeft = _draggedTopLeft( details, + end: (origin.id, true), topLeft: screenPositionForWidget( widget: origin.agent, coordinateSystem: coordinateSystem, @@ -171,6 +224,7 @@ class _LineUpPlacementEditorState extends ConsumerState { ), position: origin.agent.position, ); + if (topLeft == null) return; notifier.moveEditedOrigin( origin.id, storedAgentPositionForRenderedScreenPosition( @@ -186,6 +240,7 @@ class _LineUpPlacementEditorState extends ConsumerState { final position = ref.read(lineUpProvider).edit?.movedOrigins[origin.id]; endMove( + (origin.id, true), outOfBounds: position != null && coordinateSystem .isOutOfBounds(position + storedAgentAnchor), @@ -206,6 +261,7 @@ class _LineUpPlacementEditorState extends ConsumerState { final abilityData = landing.ability.data.abilityData!; final topLeft = _draggedTopLeft( details, + end: (landing.id, false), topLeft: screenPositionForWidget( widget: landing.ability, coordinateSystem: coordinateSystem, @@ -215,6 +271,7 @@ class _LineUpPlacementEditorState extends ConsumerState { ), position: landing.ability.position, ); + if (topLeft == null) return; notifier.moveEditedLanding( landing.id, storedAbilityPositionForRenderedScreenPosition( @@ -236,6 +293,7 @@ class _LineUpPlacementEditorState extends ConsumerState { mapScale: mapScale, ); endMove( + (landing.id, false), outOfBounds: position != null && coordinateSystem.isOutOfBounds(position + anchor), undo: () { diff --git a/test/lineup_add_item_interaction_test.dart b/test/lineup_add_item_interaction_test.dart index 0e6217905..274995786 100644 --- a/test/lineup_add_item_interaction_test.dart +++ b/test/lineup_add_item_interaction_test.dart @@ -766,6 +766,154 @@ void main() { await tester.pumpAndSettle(); }); + /// Opens placement editing on [group]'s lineups, without dragging. + Future openEdit( + WidgetTester tester, + ProviderContainer container, + LineUpGroup group, + ) async { + container + .read(lineUpProvider.notifier) + .fromHive(LineUpGraph.fromLegacyGroups([group])); + container.read(interactionStateProvider.notifier).editLineUpPlacement( + container.read(lineUpProvider).links.first.id, + ); + CoordinateSystem(playAreaSize: const Size(900, 600)); + await _pumpHarness( + tester, + container: container, + child: const SizedBox( + width: 900, + height: 600, + child: Stack( + children: [ + LineUpOverlay(), + Positioned.fill(child: LineUpPlacementEditor()), + ], + ), + ), + ); + } + + Future moveInSteps(TestGesture gesture, Offset by) async { + for (var step = 0; step < 4; step++) { + await gesture.moveBy(by / 4); + } + } + + Finder abilityOf(String landingId) => + find.byKey(ValueKey('lineup-ability-drag-$landingId')); + + testWidgets('a reload mid-drag that kept the spot lets the drag carry on', + (tester) async { + final container = _createContainer(); + final group = _breachGroup(); + await openEdit(tester, container, group); + final startTopLeft = tester.getTopLeft(abilityOf('breach-item')); + + final gesture = await tester.startGesture( + tester.getCenter(abilityOf('breach-item')), + kind: PointerDeviceKind.mouse, + ); + await moveInSteps(gesture, _editDrag / 2); + await tester.pump(); + container.read(lineUpProvider.notifier).fromHive( + reloaded(group, abilityAt: group.items.single.ability.position), + samePage: true, + ); + await tester.pump(); + await moveInSteps(gesture, _editDrag / 2); + await gesture.up(); + await tester.pump(); + + final expected = + CoordinateSystem.instance.screenToCoordinate(startTopLeft + _editDrag); + final draft = + container.read(lineUpProvider).edit!.movedLandings['breach-item']!; + expect(draft.dx, closeTo(expected.dx, 0.001)); + expect(draft.dy, closeTo(expected.dy, 0.001)); + }); + + testWidgets('a reload mid-drag that moved the spot stops that drag', + (tester) async { + final container = _createContainer(); + final group = _breachGroup(); + await openEdit(tester, container, group); + const theirs = Offset(500, 300); + + final gesture = await tester.startGesture( + tester.getCenter(abilityOf('breach-item')), + kind: PointerDeviceKind.mouse, + ); + await moveInSteps(gesture, _editDrag / 2); + await tester.pump(); + container.read(lineUpProvider.notifier).fromHive( + reloaded(group, abilityAt: theirs), + samePage: true, + ); + await tester.pump(); + // The pointer keeps going, but the drag it was making is gone. + await moveInSteps(gesture, _editDrag / 2); + await gesture.up(); + await tester.pumpAndSettle(); + + expect(container.read(lineUpProvider).edit!.movedLandings, isEmpty); + expect( + find.text("The cloud's version changed a spot you dragged, so that " + 'drag was undone.'), + findsOneWidget, + ); + await tester.pump(const Duration(seconds: 4)); + await tester.pumpAndSettle(); + }); + + testWidgets( + 'after a reload removes the lineup being dragged, the next drag starts ' + 'fresh', (tester) async { + final container = _createContainer(); + final breach = _breachGroup(); + final first = breach.items.single; + final second = first.copyWith( + id: 'breach-item-2', + ability: first.ability.copyWith( + id: 'breach-ability-2', + position: const Offset(520, 200), + ), + ); + final group = breach.copyWith(items: [first, second]); + await openEdit(tester, container, group); + + final gesture = await tester.startGesture( + tester.getCenter(abilityOf('breach-item')), + kind: PointerDeviceKind.mouse, + ); + await moveInSteps(gesture, _editDrag); + await tester.pump(); + // The cloud's version no longer has the lineup being dragged. + container.read(lineUpProvider.notifier).fromHive( + LineUpGraph.fromLegacyGroups([ + group.copyWith(items: [second]), + ]), + samePage: true, + ); + await tester.pump(); + await gesture.up(); + await tester.pumpAndSettle(); + expect(container.read(lineUpProvider).edit!.linkIds, hasLength(1)); + + final secondTopLeft = tester.getTopLeft(abilityOf('breach-item-2')); + await tester.drag(abilityOf('breach-item-2'), const Offset(30, 20)); + await tester.pump(); + final expected = CoordinateSystem.instance + .screenToCoordinate(secondTopLeft + const Offset(30, 20)); + final draft = + container.read(lineUpProvider).edit!.movedLandings['breach-item-2']!; + expect(draft.dx, closeTo(expected.dx, 0.001)); + expect(draft.dy, closeTo(expected.dy, 0.001)); + await tester.pump(const Duration(seconds: 4)); + await tester.pumpAndSettle(); + }); + testWidgets('lineup origin opens its menu on right-click', (tester) async { final container = _createContainer(); final group = _breachGroup(); From 8fed3da2ab48dd1ca67ab83a71badcccf38d15aa Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Fri, 9 Oct 2026 15:26:07 -0400 Subject: [PATCH 3/7] Let the reload alone decide which drag carries on The editor compared a reload against where its own gesture began, which for a second drag of an end is the earlier draft, not the saved position fromHive compares against. The two could disagree: a reload that moved the spot to the first draft dropped the drag, and the editor then put it back. The editor now asks the reopened edit whether it kept the draft. A drag taken back on an end the reload removed no longer outlives its gesture, and dragging another end clears it. The screenshot view's lineup fake takes fromHive's new samePage flag. Co-Authored-By: Claude Opus 5.5 --- lib/widgets/line_up_placement_editor.dart | 31 +++--- test/lineup_add_item_interaction_test.dart | 104 +++++++++++++++++++++ test/screenshot_view_test.dart | 2 +- 3 files changed, 119 insertions(+), 18 deletions(-) diff --git a/lib/widgets/line_up_placement_editor.dart b/lib/widgets/line_up_placement_editor.dart index e5e2f7ec0..3aabbba10 100644 --- a/lib/widgets/line_up_placement_editor.dart +++ b/lib/widgets/line_up_placement_editor.dart @@ -56,7 +56,8 @@ class _LineUpPlacementEditorState extends ConsumerState { })? _drag; /// The end whose drag a reload of the page took back while the pointer - /// was still down. The rest of that gesture moves nothing. + /// was still down. The rest of that gesture moves nothing; dragging + /// another end clears it. _End? _takenBack; /// Where the drag of [end] puts its top-left, keeping the point that was @@ -70,6 +71,7 @@ class _LineUpPlacementEditorState extends ConsumerState { required Offset position, }) { if (_takenBack == end) return null; + _takenBack = null; final box = context.findRenderObject() as RenderBox; final drag = _drag = ( end: end, @@ -92,25 +94,20 @@ class _LineUpPlacementEditorState extends ConsumerState { }); } - /// A reload of the page reopens the edit (see LineUpProvider.fromHive). - /// The drag under way carries on only if its end is still there, where - /// the drag began; otherwise the reload took the drag back, and following - /// the pointer would put it back. - void _onEditReopened(LineUpState next) { + /// A reload of the page reopens the edit, deciding which drags it keeps + /// (see LineUpProvider.fromHive). The drag under way carries on only if + /// the reload kept its draft; otherwise following the pointer would put + /// back a drag the reload undid. + void _onEditReopened(LineUpPlacementEdit? edit) { final drag = _drag; if (drag == null) return; - final edit = next.edit; final (id, origin) = drag.end; - final now = origin - ? (edit?.originIds.contains(id) ?? false) - ? next.originById(id)?.agent.position - : null - : (edit?.landingIds.contains(id) ?? false) - ? next.landingById(id)?.ability.position - : null; - if (now == drag.position) return; + final ends = origin ? edit?.originIds : edit?.landingIds; + final drafts = origin ? edit?.movedOrigins : edit?.movedLandings; + if (drafts?.containsKey(id) ?? false) return; _drag = null; - _takenBack = drag.end; + // An end the reload removed has no gesture left to finish. + _takenBack = (ends?.contains(id) ?? false) ? drag.end : null; } @override @@ -119,7 +116,7 @@ class _LineUpPlacementEditorState extends ConsumerState { ref.listen(lineUpProvider, (previous, next) { // Drags keep the edit's lineups; a new set means it was reopened. if (!identical(previous?.edit?.linkIds, next.edit?.linkIds)) { - _onEditReopened(next); + _onEditReopened(next.edit); } }); final state = ref.watch(lineUpProvider); diff --git a/test/lineup_add_item_interaction_test.dart b/test/lineup_add_item_interaction_test.dart index 274995786..4d74c85ae 100644 --- a/test/lineup_add_item_interaction_test.dart +++ b/test/lineup_add_item_interaction_test.dart @@ -914,6 +914,110 @@ void main() { await tester.pumpAndSettle(); }); + for (final cloudTakesFirstDrag in [false, true]) { + testWidgets( + 'a reload during a second drag of a spot ' + '${cloudTakesFirstDrag ? 'the cloud moved stops it' : 'it kept lets it carry on'}', + (tester) async { + final container = _createContainer(); + final group = _breachGroup(); + await openEdit(tester, container, group); + final startTopLeft = tester.getTopLeft(abilityOf('breach-item')); + Offset? draft() => + container.read(lineUpProvider).edit!.movedLandings['breach-item']; + + await tester.drag(abilityOf('breach-item'), _editDrag); + await tester.pump(); + final firstDraft = draft()!; + const second = Offset(40, 30); + final gesture = await tester.startGesture( + tester.getCenter(abilityOf('breach-item')), + kind: PointerDeviceKind.mouse, + ); + await moveInSteps(gesture, second / 2); + await tester.pump(); + // The cloud's version has the spot where the user's first drag put + // it, or where it was saved. + container.read(lineUpProvider.notifier).fromHive( + reloaded( + group, + abilityAt: cloudTakesFirstDrag + ? firstDraft + : group.items.single.ability.position, + ), + samePage: true, + ); + await tester.pump(); + await moveInSteps(gesture, second / 2); + await gesture.up(); + await tester.pumpAndSettle(); + + if (cloudTakesFirstDrag) { + expect(draft(), isNull); + expect( + find.text("The cloud's version changed a spot you dragged, so that " + 'drag was undone.'), + findsOneWidget, + ); + } else { + final expected = CoordinateSystem.instance + .screenToCoordinate(startTopLeft + _editDrag + second); + expect(draft()!.dx, closeTo(expected.dx, 0.001)); + expect(draft()!.dy, closeTo(expected.dy, 0.001)); + } + await tester.pump(const Duration(seconds: 4)); + await tester.pumpAndSettle(); + }); + } + + testWidgets('a lineup a reload removed mid-drag drags again once it is back', + (tester) async { + final container = _createContainer(); + final breach = _breachGroup(); + final first = breach.items.single; + final second = first.copyWith( + id: 'breach-item-2', + ability: first.ability.copyWith( + id: 'breach-ability-2', + position: const Offset(520, 200), + ), + ); + final group = breach.copyWith(items: [first, second]); + await openEdit(tester, container, group); + + final gesture = await tester.startGesture( + tester.getCenter(abilityOf('breach-item')), + kind: PointerDeviceKind.mouse, + ); + await moveInSteps(gesture, _editDrag); + await tester.pump(); + container.read(lineUpProvider.notifier).fromHive( + LineUpGraph.fromLegacyGroups([ + group.copyWith(items: [second]), + ]), + samePage: true, + ); + await tester.pump(); + await gesture.up(); + await tester.pumpAndSettle(); + // A later reload brings it back, joined through the shared agent. + container.read(lineUpProvider.notifier).fromHive( + LineUpGraph.fromLegacyGroups([group]), + samePage: true, + ); + await tester.pumpAndSettle(); + expect(container.read(lineUpProvider).edit!.linkIds, hasLength(2)); + + await tester.drag(abilityOf('breach-item'), const Offset(30, 20)); + await tester.pump(); + expect( + container.read(lineUpProvider).edit!.movedLandings['breach-item'], + isNotNull, + ); + await tester.pump(const Duration(seconds: 4)); + await tester.pumpAndSettle(); + }); + testWidgets('lineup origin opens its menu on right-click', (tester) async { final container = _createContainer(); final group = _breachGroup(); diff --git a/test/screenshot_view_test.dart b/test/screenshot_view_test.dart index c22bf2049..c8b46ff5c 100644 --- a/test/screenshot_view_test.dart +++ b/test/screenshot_view_test.dart @@ -162,7 +162,7 @@ class _NoopLineUpProvider extends LineUpProvider { LineUpState build() => const LineUpState(); @override - void fromHive(LineUpGraph graph) {} + void fromHive(LineUpGraph graph, {bool samePage = false}) {} } void main() { From 2df842b22e7a9c7ca1f483c62f996aa429705d6c Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Fri, 9 Oct 2026 15:35:34 -0400 Subject: [PATCH 4/7] Clear a taken-back drag once its end is gone A reload that took a drag back and a second reload that removed its end, both before the pointer lifted, left the marker set: when a later reload brought the end back, its first drag was ignored. Every reopen now clears a marker whose end is no longer in the edit. Co-Authored-By: Claude Opus 5.5 --- lib/widgets/line_up_placement_editor.dart | 12 ++- test/lineup_add_item_interaction_test.dart | 112 ++++++++++++--------- 2 files changed, 76 insertions(+), 48 deletions(-) diff --git a/lib/widgets/line_up_placement_editor.dart b/lib/widgets/line_up_placement_editor.dart index 3aabbba10..80fafb9d2 100644 --- a/lib/widgets/line_up_placement_editor.dart +++ b/lib/widgets/line_up_placement_editor.dart @@ -99,15 +99,21 @@ class _LineUpPlacementEditorState extends ConsumerState { /// the reload kept its draft; otherwise following the pointer would put /// back a drag the reload undid. void _onEditReopened(LineUpPlacementEdit? edit) { + bool inEdit(_End end) { + final (id, origin) = end; + return (origin ? edit?.originIds : edit?.landingIds)?.contains(id) ?? + false; + } + + // An end the reload removed has no gesture left to finish. + if (_takenBack case final end? when !inEdit(end)) _takenBack = null; final drag = _drag; if (drag == null) return; final (id, origin) = drag.end; - final ends = origin ? edit?.originIds : edit?.landingIds; final drafts = origin ? edit?.movedOrigins : edit?.movedLandings; if (drafts?.containsKey(id) ?? false) return; _drag = null; - // An end the reload removed has no gesture left to finish. - _takenBack = (ends?.contains(id) ?? false) ? drag.end : null; + if (inEdit(drag.end)) _takenBack = drag.end; } @override diff --git a/test/lineup_add_item_interaction_test.dart b/test/lineup_add_item_interaction_test.dart index 4d74c85ae..089e34627 100644 --- a/test/lineup_add_item_interaction_test.dart +++ b/test/lineup_add_item_interaction_test.dart @@ -970,53 +970,75 @@ void main() { }); } - testWidgets('a lineup a reload removed mid-drag drags again once it is back', - (tester) async { - final container = _createContainer(); - final breach = _breachGroup(); - final first = breach.items.single; - final second = first.copyWith( - id: 'breach-item-2', - ability: first.ability.copyWith( - id: 'breach-ability-2', - position: const Offset(520, 200), - ), - ); - final group = breach.copyWith(items: [first, second]); - await openEdit(tester, container, group); + for (final movedFirst in [false, true]) { + testWidgets( + 'a lineup a reload removed mid-drag drags again once it is back' + '${movedFirst ? ', after an earlier reload took its drag back' : ''}', + (tester) async { + final container = _createContainer(); + final breach = _breachGroup(); + final first = breach.items.single; + final second = first.copyWith( + id: 'breach-item-2', + ability: first.ability.copyWith( + id: 'breach-ability-2', + position: const Offset(520, 200), + ), + ); + final group = breach.copyWith(items: [first, second]); + await openEdit(tester, container, group); - final gesture = await tester.startGesture( - tester.getCenter(abilityOf('breach-item')), - kind: PointerDeviceKind.mouse, - ); - await moveInSteps(gesture, _editDrag); - await tester.pump(); - container.read(lineUpProvider.notifier).fromHive( - LineUpGraph.fromLegacyGroups([ - group.copyWith(items: [second]), - ]), - samePage: true, - ); - await tester.pump(); - await gesture.up(); - await tester.pumpAndSettle(); - // A later reload brings it back, joined through the shared agent. - container.read(lineUpProvider.notifier).fromHive( - LineUpGraph.fromLegacyGroups([group]), - samePage: true, - ); - await tester.pumpAndSettle(); - expect(container.read(lineUpProvider).edit!.linkIds, hasLength(2)); + final gesture = await tester.startGesture( + tester.getCenter(abilityOf('breach-item')), + kind: PointerDeviceKind.mouse, + ); + await moveInSteps(gesture, _editDrag); + await tester.pump(); + if (movedFirst) { + // The cloud moves the spot being dragged, then drops its lineup, all + // before the pointer lifts. + container.read(lineUpProvider.notifier).fromHive( + LineUpGraph.fromLegacyGroups([ + group.copyWith(items: [ + first.copyWith( + ability: first.ability.copyWith( + position: const Offset(500, 300), + ), + ), + second, + ]), + ]), + samePage: true, + ); + await tester.pump(); + } + container.read(lineUpProvider.notifier).fromHive( + LineUpGraph.fromLegacyGroups([ + group.copyWith(items: [second]), + ]), + samePage: true, + ); + await tester.pump(); + await gesture.up(); + await tester.pumpAndSettle(); + // A later reload brings it back, joined through the shared agent. + container.read(lineUpProvider.notifier).fromHive( + LineUpGraph.fromLegacyGroups([group]), + samePage: true, + ); + await tester.pumpAndSettle(); + expect(container.read(lineUpProvider).edit!.linkIds, hasLength(2)); - await tester.drag(abilityOf('breach-item'), const Offset(30, 20)); - await tester.pump(); - expect( - container.read(lineUpProvider).edit!.movedLandings['breach-item'], - isNotNull, - ); - await tester.pump(const Duration(seconds: 4)); - await tester.pumpAndSettle(); - }); + await tester.drag(abilityOf('breach-item'), const Offset(30, 20)); + await tester.pump(); + expect( + container.read(lineUpProvider).edit!.movedLandings['breach-item'], + isNotNull, + ); + await tester.pump(const Duration(seconds: 4)); + await tester.pumpAndSettle(); + }); + } testWidgets('lineup origin opens its menu on right-click', (tester) async { final container = _createContainer(); From deae14e199ccf9bbbea0d454c51942b7e614ca96 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Fri, 9 Oct 2026 23:34:38 -0400 Subject: [PATCH 5/7] Resolve a conflict item by item, not by reloading the page Use cloud and Keep both reloaded the whole page to show the cloud's version, which closed Edit placement and threw away its drags even when the conflict was on another lineup. Teammates' changes already apply item by item (#208); conflict resolution now goes the same way, holding what the user has open except what they chose the cloud's version of. A placement edit of a lineup in the conflict ends. This replaces the earlier commits' reopen-after-reload logic in the lineup provider and editor, which go back to #265's versions. Co-Authored-By: Claude Opus 5.5 --- lib/const/line_provider.dart | 116 +---- lib/providers/editor_operation_provider.dart | 8 + .../strategy_page_session_provider.dart | 103 ++++- lib/strategy/strategy_page_apply.dart | 8 +- lib/widgets/line_up_placement_editor.dart | 83 +--- test/lineup_add_item_interaction_test.dart | 430 ------------------ test/per_object_undo_test.dart | 74 --- test/screenshot_view_test.dart | 2 +- test/strategy_page_session_provider_test.dart | 244 +++++++--- 9 files changed, 317 insertions(+), 751 deletions(-) diff --git a/lib/const/line_provider.dart b/lib/const/line_provider.dart index aa66e5a24..06b4a0fe5 100644 --- a/lib/const/line_provider.dart +++ b/lib/const/line_provider.dart @@ -1048,32 +1048,25 @@ class LineUpProvider extends Notifier { void startEdit(String linkId) { final start = state.linkById(linkId); if (start == null) return; - state = state.copyWith(edit: _editOf([start], within: state)); - } - - /// An edit of [links] and every lineup in [within] joined to them through - /// a shared origin or landing, with nothing moved yet. - static LineUpPlacementEdit _editOf( - Iterable links, { - required LineUpState within, - }) { final linkIds = {}; final originIds = {}; final landingIds = {}; - final pending = [...links]; + final pending = [start]; while (pending.isNotEmpty) { final link = pending.removeLast(); if (!linkIds.add(link.id)) continue; originIds.add(link.originId); landingIds.add(link.landingId); - pending.addAll(within.links.where((other) => + pending.addAll(state.links.where((other) => other.originId == link.originId || other.landingId == link.landingId)); } - return LineUpPlacementEdit( - linkIds: linkIds, - originIds: originIds, - landingIds: landingIds, + state = state.copyWith( + edit: LineUpPlacementEdit( + linkIds: linkIds, + originIds: originIds, + landingIds: landingIds, + ), ); } @@ -1574,88 +1567,19 @@ class LineUpProvider extends Notifier { // --- Lifecycle and serialization ----------------------------------------- - /// Replaces the page's lineups. A placement edit ends with them unless - /// [graph] is a newer copy of the same page, say after a cloud conflict - /// was resolved: its ids may name lineups a copied page shares, and saving - /// it there would move the wrong page's lineups. - /// - /// On the same page the edit stays open on whichever of its lineups are - /// still there, joined to any that now share their spots. A drag stays - /// while the newer copy has its end where the drag started; an end the - /// newer copy moved or removed shows the newer copy, and the user is told. - void fromHive(LineUpGraph graph, {bool samePage = false}) { - final before = state; + /// Replaces the page's lineups. A placement edit ends with them: its ids + /// may name lineups a copied page shares, and saving it there would move + /// the wrong page's lineups. + void fromHive(LineUpGraph graph) { final copy = graph.deepCopy(); - final loaded = before.copyWith( + final droppedMoves = state.editMovesAnything; + state = state.copyWith( origins: copy.origins, landings: copy.landings, links: copy.links, edit: null, ); - // Set once, so nothing watching sees the edit end on the way. - final (edit, lost) = samePage - ? _reopenEdit(before, loaded) - : ( - null, - before.editMovesAnything - ? 'The page changed, so the placement edit closed without ' - 'saving.' - : null, - ); - state = loaded.copyWith(edit: edit); - if (lost != null) _toastDroppedEdit(lost); - } - - /// [before]'s placement edit on [loaded], a newer copy of the same page - /// (see [fromHive]), and what to tell the user if drags were lost. - static (LineUpPlacementEdit?, String?) _reopenEdit( - LineUpState before, - LineUpState loaded, - ) { - final edit = before.edit; - if (edit == null) return (null, null); - final survivors = edit.linkIds.map(loaded.linkById).nonNulls; - if (survivors.isEmpty) { - return ( - null, - before.editMovesAnything - ? 'The lineups you were moving are no longer on this page, so ' - 'the placement edit closed without saving.' - : null, - ); - } - final reopened = _editOf(survivors, within: loaded); - var dropped = false; - Map keptDrags( - Map drags, - Set ids, - Offset? Function(LineUpState state, String id) positionOf, - ) { - final kept = {}; - for (final MapEntry(key: id, value: draft) in drags.entries) { - final was = positionOf(before, id); - if (was == null || was == draft) continue; - if (ids.contains(id) && positionOf(loaded, id) == was) { - kept[id] = draft; - } else { - dropped = true; - } - } - return kept; - } - - return ( - reopened.copyWith( - movedOrigins: keptDrags(edit.movedOrigins, reopened.originIds, - (state, id) => state.originById(id)?.agent.position), - movedLandings: keptDrags(edit.movedLandings, reopened.landingIds, - (state, id) => state.landingById(id)?.ability.position), - ), - dropped - ? "The cloud's version changed a spot you dragged, so that drag " - 'was undone.' - : null, - ); + if (droppedMoves) _toastDroppedEdit(); } /// Takes [graph] as the server's lineups merged with the ones the user @@ -1705,15 +1629,13 @@ class LineUpProvider extends Notifier { void clearAll() { final droppedMoves = state.editMovesAnything; state = state.copyWith(origins: [], landings: [], links: [], edit: null); - if (droppedMoves) { - _toastDroppedEdit('The page changed, so the placement edit closed ' - 'without saving.'); - } + if (droppedMoves) _toastDroppedEdit(); } - void _toastDroppedEdit(String message) { + void _toastDroppedEdit() { Settings.showToast( - message: message, + message: 'The page changed, so the placement edit closed without ' + 'saving.', backgroundColor: Settings.tacticalVioletTheme.destructive, ); } diff --git a/lib/providers/editor_operation_provider.dart b/lib/providers/editor_operation_provider.dart index c970fbd72..8aed49159 100644 --- a/lib/providers/editor_operation_provider.dart +++ b/lib/providers/editor_operation_provider.dart @@ -85,6 +85,14 @@ class EditorPointersNotifier extends Notifier> { final editorHeldEntitiesProvider = Provider?>((ref) { final holds = ref.watch(editorPointersProvider).values; if (holds.any((hold) => !hold.onCanvas)) return null; + return ref.watch(editorHeldCanvasItemsProvider); +}); + +/// The canvas items the user is in the middle of changing, whatever else is +/// pressed: the items pressed on the canvas, open text drafts, a lineup +/// placement's pinned ends, and the lineups a placement edit is moving. +final editorHeldCanvasItemsProvider = Provider>((ref) { + final holds = ref.watch(editorPointersProvider).values; final placement = ref.watch(lineUpProvider.select((state) => state.placement)); // The edit's lineups stay the same while its ends move, so a drag does not diff --git a/lib/providers/strategy_page_session_provider.dart b/lib/providers/strategy_page_session_provider.dart index 0d315371c..f24eda99d 100644 --- a/lib/providers/strategy_page_session_provider.dart +++ b/lib/providers/strategy_page_session_provider.dart @@ -22,6 +22,7 @@ import 'package:icarus/providers/agent_provider.dart'; import 'package:icarus/const/hive_boxes.dart'; import 'package:icarus/const/transition_data.dart'; import 'package:icarus/providers/image_provider.dart'; +import 'package:icarus/providers/interaction_state_provider.dart'; import 'package:icarus/providers/drawing_provider.dart'; import 'package:icarus/providers/editor_operation_provider.dart'; import 'package:icarus/providers/collab/active_page_live_sync_models.dart'; @@ -836,14 +837,12 @@ class StrategyPageSessionNotifier extends Notifier { bool unchangedCanvas() => !waitingChanged() && _sameCanvas(savedCanvas); if (!unchangedCanvas()) return false; var redrawn = false; - await _applyLoadedPageData( + await _showCloudVersions( pageData, + of: rejected.keys.toSet(), strategyId: strategyId, - source: StrategySource.cloud, canApply: () => redrawn = unchangedCanvas(), - hydrationKey: _buildRemotePageHydrationKey(snapshot, targetPageId), - preserveTextDrafts: true, - loadedRemoteSnapshot: pageSource.loadedRemoteSnapshot, + loadedRemoteSnapshot: pageSource.loadedRemoteSnapshot!, preservedMetadata: rejected.containsKey(const EntitySyncKey.strategy()) ? null @@ -871,16 +870,15 @@ class StrategyPageSessionNotifier extends Notifier { targetPageId, discardedEntities: discarded, ); - await _applyLoadedPageData( + await _showCloudVersions( pageData, + of: discarded, strategyId: strategyId, - source: StrategySource.cloud, canApply: () => !_disposed && generation == _pageSessionGeneration && _sameCanvas(redrawnCanvas), - preserveTextDrafts: true, - loadedRemoteSnapshot: pageSource.loadedRemoteSnapshot, + loadedRemoteSnapshot: pageSource.loadedRemoteSnapshot!, preservedMetadata: discarded.contains(const EntitySyncKey.strategy()) ? null : localMetadata, @@ -914,6 +912,73 @@ class StrategyPageSessionNotifier extends Notifier { } } + /// Draws [pageData], which has the cloud's version of the entities in + /// [of]. The page on screen takes it item by item, as it takes a + /// teammate's change, so what the user is in the middle of stays as it is: + /// a placement edit of other lineups keeps its drags. A placement edit of + /// a lineup in [of] ends, since the user chose the cloud's version of it. + /// Any other page loads whole. + Future _showCloudVersions( + StrategyEditorPageData pageData, { + required Set of, + required String strategyId, + required bool Function() canApply, + required RemoteEditorSnapshot loadedRemoteSnapshot, + ({MapValue map, StrategyThemeState theme})? preservedMetadata, + }) async { + final pageId = pageData.pageId; + final liveSync = ref.read(activePageLiveSyncProvider.notifier); + final onScreen = + _lastHydratedRemotePageKey?.strategyPublicId == strategyId && + _lastHydratedRemotePageKey?.pageId == pageId && + ref.read(activePageLiveSyncProvider).hydratedPageId == pageId; + if (!onScreen) { + await _applyLoadedPageData( + pageData, + strategyId: strategyId, + source: StrategySource.cloud, + canApply: canApply, + preserveTextDrafts: true, + loadedRemoteSnapshot: loadedRemoteSnapshot, + preservedMetadata: preservedMetadata, + ); + return; + } + // The cloud's version of a page (one this device added, say) can change + // the list of pages. + final availablePageIds = + await _resolvePageSource(strategyId, StrategySource.cloud) + .listPageIds(); + if (!canApply()) return; + // Whether the canvas item [id] (an element, or a lineup or one of its + // spots) is one the user chose the cloud's version of. + bool chosen(String id) => + of.contains(EntitySyncKey.element(pageId, id)) || + switch (liveSync.lineupGroupOf(pageId, id)) { + final group? => of.contains(EntitySyncKey.lineup(pageId, group)), + null => false, + }; + if (ref.read(lineUpProvider).edit?.linkIds.any(chosen) ?? false) { + ref + .read(interactionStateProvider.notifier) + .update(InteractionState.navigation); + } + state = state.copyWith(availablePageIds: availablePageIds); + _mergeRemotePage( + pageData, + strategyId: strategyId, + snapshot: loadedRemoteSnapshot, + // What the user is in the middle of stays, except what they chose the + // cloud's version of. A press off the canvas (on the conflict panel, + // say) holds nothing on it. + holding: { + for (final id in ref.read(editorHeldCanvasItemsProvider)) + if (!chosen(id)) id, + }, + preservedMetadata: preservedMetadata, + ); + } + /// The strategy's map and theme on screen, while a change to them is still /// on its way: loading a page must keep them rather than take the older /// server copy (and so drop the change). @@ -1535,11 +1600,14 @@ class StrategyPageSessionNotifier extends Notifier { /// Brings the page on screen up to [snapshot] item by item, leaving what /// the user is in the middle of changing. A held item's change applies once - /// the hold ends. + /// the hold ends. [holding] defaults to what the editor holds now; + /// [preservedMetadata] is as for [_applyLoadedPageData]. void _mergeRemotePage( StrategyEditorPageData pageData, { required String strategyId, required RemoteEditorSnapshot snapshot, + Set? holding, + ({MapValue map, StrategyThemeState theme})? preservedMetadata, }) { final pageId = pageData.pageId; final liveSync = ref.read(activePageLiveSyncProvider.notifier); @@ -1551,13 +1619,14 @@ class StrategyPageSessionNotifier extends Notifier { ref, pageData, changed: liveSync.remoteChangesSinceHydration(snapshot, pageId), - holding: ref.read(editorHeldEntitiesProvider)!, - themeProfileId: - _resolveThemeProfileId(StrategySource.cloud, strategyId), - themeOverridePalette: _resolveThemeOverridePalette( - StrategySource.cloud, - strategyId, - ), + holding: holding ?? ref.read(editorHeldEntitiesProvider)!, + themeProfileId: preservedMetadata != null + ? preservedMetadata.theme.profileId + : _resolveThemeProfileId(StrategySource.cloud, strategyId), + themeOverridePalette: preservedMetadata != null + ? preservedMetadata.theme.overridePalette + : _resolveThemeOverridePalette(StrategySource.cloud, strategyId), + mapOverride: preservedMetadata?.map, ); } finally { state = state.copyWith(isApplyingPage: false); diff --git a/lib/strategy/strategy_page_apply.dart b/lib/strategy/strategy_page_apply.dart index 38088a19b..5fc118f68 100644 --- a/lib/strategy/strategy_page_apply.dart +++ b/lib/strategy/strategy_page_apply.dart @@ -32,6 +32,7 @@ Future applyStrategyEditorPageData( ref.read(textProvider.notifier).clearAll(); ref.read(placedImageProvider.notifier).clearAll(); ref.read(utilityProvider.notifier).clearAll(); + ref.read(lineUpProvider.notifier).clearAll(); if (!preserveHistory) { ref.read(actionProvider.notifier).clearActionHistory(); } @@ -41,12 +42,7 @@ Future applyStrategyEditorPageData( ref.read(textProvider.notifier).fromHive(data.texts); ref.read(placedImageProvider.notifier).fromHive(data.images); ref.read(utilityProvider.notifier).fromHive(data.utilities); - // Only a reload of the page on screen keeps its undo history, and only - // that one keeps a placement edit open; fromHive replaces the lineups, so - // they are not cleared first. - ref - .read(lineUpProvider.notifier) - .fromHive(data.lineUpGraph, samePage: preserveHistory); + ref.read(lineUpProvider.notifier).fromHive(data.lineUpGraph); ref .read(mapProvider.notifier) .fromHive(mapOverride ?? data.map, data.isAttack); diff --git a/lib/widgets/line_up_placement_editor.dart b/lib/widgets/line_up_placement_editor.dart index 80fafb9d2..c5dec70fc 100644 --- a/lib/widgets/line_up_placement_editor.dart +++ b/lib/widgets/line_up_placement_editor.dart @@ -29,10 +29,6 @@ String lineUpEditStatus(LineUpState state) { 'so they move together'; } -/// One end of an edited lineup: its id, and whether it is an origin (else a -/// landing). -typedef _End = (String id, bool origin); - /// Placement editing: the lineups being moved, drawn over the dimmed page at /// their draft positions. Each origin and landing follows the pointer with /// its lines; Save in LineupControlButtons writes the move. @@ -45,36 +41,21 @@ class LineUpPlacementEditor extends ConsumerStatefulWidget { } class _LineUpPlacementEditorState extends ConsumerState { - /// The drag under way: which end, how far the pointer has moved in window - /// pixels, and the end's top-left in this layer's pixels and its stored - /// position where the drag began. - ({ - _End end, - Offset moved, - Offset topLeft, - Offset position, - })? _drag; - - /// The end whose drag a reload of the page took back while the pointer - /// was still down. The rest of that gesture moves nothing; dragging - /// another end clears it. - _End? _takenBack; + /// The drag under way: how far the pointer has moved in window pixels, + /// and the end's top-left in this layer's pixels and its stored position + /// where the drag began. + ({Offset moved, Offset topLeft, Offset position})? _drag; - /// Where the drag of [end] puts its top-left, keeping the point that was - /// grabbed under the pointer, or null once a reload took the drag back. - /// The movement adds up the updates' deltas: the first update reports - /// where the pointer went down, not where it is. - Offset? _draggedTopLeft( + /// Where the drag puts the end's top-left, keeping the point that was + /// grabbed under the pointer. The movement adds up the updates' deltas: + /// the first update reports where the pointer went down, not where it is. + Offset _draggedTopLeft( DragUpdateDetails details, { - required _End end, required Offset topLeft, required Offset position, }) { - if (_takenBack == end) return null; - _takenBack = null; final box = context.findRenderObject() as RenderBox; final drag = _drag = ( - end: end, moved: (_drag?.moved ?? Offset.zero) + details.delta, topLeft: _drag?.topLeft ?? topLeft, position: _drag?.position ?? position, @@ -94,41 +75,13 @@ class _LineUpPlacementEditorState extends ConsumerState { }); } - /// A reload of the page reopens the edit, deciding which drags it keeps - /// (see LineUpProvider.fromHive). The drag under way carries on only if - /// the reload kept its draft; otherwise following the pointer would put - /// back a drag the reload undid. - void _onEditReopened(LineUpPlacementEdit? edit) { - bool inEdit(_End end) { - final (id, origin) = end; - return (origin ? edit?.originIds : edit?.landingIds)?.contains(id) ?? - false; - } - - // An end the reload removed has no gesture left to finish. - if (_takenBack case final end? when !inEdit(end)) _takenBack = null; - final drag = _drag; - if (drag == null) return; - final (id, origin) = drag.end; - final drafts = origin ? edit?.movedOrigins : edit?.movedLandings; - if (drafts?.containsKey(id) ?? false) return; - _drag = null; - if (inEdit(drag.end)) _takenBack = drag.end; - } - @override Widget build(BuildContext context) { final coordinateSystem = CoordinateSystem.instance; - ref.listen(lineUpProvider, (previous, next) { - // Drags keep the edit's lineups; a new set means it was reopened. - if (!identical(previous?.edit?.linkIds, next.edit?.linkIds)) { - _onEditReopened(next.edit); - } - }); final state = ref.watch(lineUpProvider); final edit = state.edit; - // The page's lineups were replaced (another page opened, or a reload - // took every lineup being moved) and the edit went with them. + // The page's lineups were replaced (another page opened) and the edit + // went with them. if (edit == null) { _leave(); return const SizedBox.shrink(); @@ -176,15 +129,7 @@ class _LineUpPlacementEditorState extends ConsumerState { return const SizedBox.shrink(); } - void endMove( - _End end, { - required bool outOfBounds, - required void Function() undo, - }) { - if (_takenBack == end) { - _takenBack = null; - return; - } + void endMove({required bool outOfBounds, required void Function() undo}) { // Dropped off the map: the end goes back to where the drag began. if (outOfBounds) undo(); _drag = null; @@ -218,7 +163,6 @@ class _LineUpPlacementEditorState extends ConsumerState { onMove: (details) { final topLeft = _draggedTopLeft( details, - end: (origin.id, true), topLeft: screenPositionForWidget( widget: origin.agent, coordinateSystem: coordinateSystem, @@ -227,7 +171,6 @@ class _LineUpPlacementEditorState extends ConsumerState { ), position: origin.agent.position, ); - if (topLeft == null) return; notifier.moveEditedOrigin( origin.id, storedAgentPositionForRenderedScreenPosition( @@ -243,7 +186,6 @@ class _LineUpPlacementEditorState extends ConsumerState { final position = ref.read(lineUpProvider).edit?.movedOrigins[origin.id]; endMove( - (origin.id, true), outOfBounds: position != null && coordinateSystem .isOutOfBounds(position + storedAgentAnchor), @@ -264,7 +206,6 @@ class _LineUpPlacementEditorState extends ConsumerState { final abilityData = landing.ability.data.abilityData!; final topLeft = _draggedTopLeft( details, - end: (landing.id, false), topLeft: screenPositionForWidget( widget: landing.ability, coordinateSystem: coordinateSystem, @@ -274,7 +215,6 @@ class _LineUpPlacementEditorState extends ConsumerState { ), position: landing.ability.position, ); - if (topLeft == null) return; notifier.moveEditedLanding( landing.id, storedAbilityPositionForRenderedScreenPosition( @@ -296,7 +236,6 @@ class _LineUpPlacementEditorState extends ConsumerState { mapScale: mapScale, ); endMove( - (landing.id, false), outOfBounds: position != null && coordinateSystem.isOutOfBounds(position + anchor), undo: () { diff --git a/test/lineup_add_item_interaction_test.dart b/test/lineup_add_item_interaction_test.dart index 089e34627..57bfa1959 100644 --- a/test/lineup_add_item_interaction_test.dart +++ b/test/lineup_add_item_interaction_test.dart @@ -158,9 +158,6 @@ Future _pumpLineupCanvas( ); } -/// How far [editAndDragAbility] drags the ability. -const _editDrag = Offset(-55, 65); - void main() { TestWidgetsFlutterBinding.ensureInitialized(); @@ -613,433 +610,6 @@ void main() { await tester.pumpAndSettle(); }); - /// Opens placement editing on the Breach lineup and drags its ability by - /// [_editDrag]. Returns the lineup as it was placed. - Future editAndDragAbility( - WidgetTester tester, - ProviderContainer container, - ) async { - final group = _breachGroup(); - container - .read(lineUpProvider.notifier) - .fromHive(LineUpGraph.fromLegacyGroups([group])); - container.read(interactionStateProvider.notifier).editLineUpPlacement( - container.read(lineUpProvider).links.single.id, - ); - - CoordinateSystem(playAreaSize: const Size(900, 600)); - await _pumpHarness( - tester, - container: container, - child: const SizedBox( - width: 900, - height: 600, - child: Stack( - children: [ - LineUpOverlay(), - Positioned.fill(child: LineUpPlacementEditor()), - Align( - alignment: Alignment.bottomRight, - child: LineupControlButtons(), - ), - ], - ), - ), - ); - await tester.drag( - find.byKey(const ValueKey('lineup-ability-drag-breach-item')), - _editDrag, - ); - await tester.pump(); - return group; - } - - /// The Breach lineup [group] with its ability at [abilityAt], as a newer - /// copy of the page would bring it. - LineUpGraph reloaded(LineUpGroup group, {required Offset abilityAt}) { - final item = group.items.single; - return LineUpGraph.fromLegacyGroups([ - group.copyWith(items: [ - item.copyWith(ability: item.ability.copyWith(position: abilityAt)), - ]), - ]); - } - - testWidgets('reloading the same page keeps a placement edit and its drag', - (tester) async { - final container = _createContainer(); - final group = await editAndDragAbility(tester, container); - final abilityFinder = - find.byKey(const ValueKey('lineup-ability-drag-breach-item')); - final draggedTopLeft = tester.getTopLeft(abilityFinder); - final draft = - container.read(lineUpProvider).edit!.movedLandings['breach-item']!; - - // Resolving a cloud conflict reloads the page on screen. - container.read(lineUpProvider.notifier).fromHive( - reloaded(group, abilityAt: group.items.single.ability.position), - samePage: true, - ); - await tester.pumpAndSettle(); - - expect( - container.read(interactionStateProvider), - InteractionState.lineUpEditing, - ); - expect(tester.getTopLeft(abilityFinder), draggedTopLeft); - expect(find.textContaining('undone'), findsNothing); - expect(find.textContaining('without saving'), findsNothing); - - await tester.tap(find.byKey(const ValueKey('lineup-edit-save'))); - await tester.pumpAndSettle(); - expect( - container - .read(lineUpProvider) - .landingById('breach-item')! - .ability - .position, - draft, - ); - expect(container.read(actionProvider), hasLength(1)); - }); - - testWidgets('a reload that moved a dragged spot undoes that drag and says so', - (tester) async { - final container = _createContainer(); - final group = await editAndDragAbility(tester, container); - const theirs = Offset(500, 300); - - container.read(lineUpProvider.notifier).fromHive( - reloaded(group, abilityAt: theirs), - samePage: true, - ); - await tester.pumpAndSettle(); - - // Still editing, the ability where the cloud has it. - expect( - container.read(interactionStateProvider), - InteractionState.lineUpEditing, - ); - expect(container.read(lineUpProvider).edit!.movedLandings, isEmpty); - expect( - tester.getTopLeft( - find.byKey(const ValueKey('lineup-ability-drag-breach-item'))), - screenPositionForWidget( - widget: container.read(lineUpProvider).landings.single.ability, - coordinateSystem: CoordinateSystem.instance, - mapScale: Maps.mapScale[MapValue.bind] ?? 1.0, - abilitySize: container.read(strategySettingsProvider).abilitySize, - isAttack: true, - ), - ); - expect( - find.text("The cloud's version changed a spot you dragged, so that " - 'drag was undone.'), - findsOneWidget, - ); - await tester.pump(const Duration(seconds: 4)); - await tester.pumpAndSettle(); - }); - - testWidgets('a reload without the lineups being moved ends the edit', - (tester) async { - final container = _createContainer(); - await editAndDragAbility(tester, container); - - // "Use cloud" took the lineup away. - container - .read(lineUpProvider.notifier) - .fromHive(LineUpGraph.empty, samePage: true); - await tester.pumpAndSettle(); - - expect(container.read(lineUpProvider).edit, isNull); - expect( - container.read(interactionStateProvider), - InteractionState.navigation, - ); - expect( - find.text('The lineups you were moving are no longer on this page, so ' - 'the placement edit closed without saving.'), - findsOneWidget, - ); - await tester.pump(const Duration(seconds: 4)); - await tester.pumpAndSettle(); - }); - - /// Opens placement editing on [group]'s lineups, without dragging. - Future openEdit( - WidgetTester tester, - ProviderContainer container, - LineUpGroup group, - ) async { - container - .read(lineUpProvider.notifier) - .fromHive(LineUpGraph.fromLegacyGroups([group])); - container.read(interactionStateProvider.notifier).editLineUpPlacement( - container.read(lineUpProvider).links.first.id, - ); - CoordinateSystem(playAreaSize: const Size(900, 600)); - await _pumpHarness( - tester, - container: container, - child: const SizedBox( - width: 900, - height: 600, - child: Stack( - children: [ - LineUpOverlay(), - Positioned.fill(child: LineUpPlacementEditor()), - ], - ), - ), - ); - } - - Future moveInSteps(TestGesture gesture, Offset by) async { - for (var step = 0; step < 4; step++) { - await gesture.moveBy(by / 4); - } - } - - Finder abilityOf(String landingId) => - find.byKey(ValueKey('lineup-ability-drag-$landingId')); - - testWidgets('a reload mid-drag that kept the spot lets the drag carry on', - (tester) async { - final container = _createContainer(); - final group = _breachGroup(); - await openEdit(tester, container, group); - final startTopLeft = tester.getTopLeft(abilityOf('breach-item')); - - final gesture = await tester.startGesture( - tester.getCenter(abilityOf('breach-item')), - kind: PointerDeviceKind.mouse, - ); - await moveInSteps(gesture, _editDrag / 2); - await tester.pump(); - container.read(lineUpProvider.notifier).fromHive( - reloaded(group, abilityAt: group.items.single.ability.position), - samePage: true, - ); - await tester.pump(); - await moveInSteps(gesture, _editDrag / 2); - await gesture.up(); - await tester.pump(); - - final expected = - CoordinateSystem.instance.screenToCoordinate(startTopLeft + _editDrag); - final draft = - container.read(lineUpProvider).edit!.movedLandings['breach-item']!; - expect(draft.dx, closeTo(expected.dx, 0.001)); - expect(draft.dy, closeTo(expected.dy, 0.001)); - }); - - testWidgets('a reload mid-drag that moved the spot stops that drag', - (tester) async { - final container = _createContainer(); - final group = _breachGroup(); - await openEdit(tester, container, group); - const theirs = Offset(500, 300); - - final gesture = await tester.startGesture( - tester.getCenter(abilityOf('breach-item')), - kind: PointerDeviceKind.mouse, - ); - await moveInSteps(gesture, _editDrag / 2); - await tester.pump(); - container.read(lineUpProvider.notifier).fromHive( - reloaded(group, abilityAt: theirs), - samePage: true, - ); - await tester.pump(); - // The pointer keeps going, but the drag it was making is gone. - await moveInSteps(gesture, _editDrag / 2); - await gesture.up(); - await tester.pumpAndSettle(); - - expect(container.read(lineUpProvider).edit!.movedLandings, isEmpty); - expect( - find.text("The cloud's version changed a spot you dragged, so that " - 'drag was undone.'), - findsOneWidget, - ); - await tester.pump(const Duration(seconds: 4)); - await tester.pumpAndSettle(); - }); - - testWidgets( - 'after a reload removes the lineup being dragged, the next drag starts ' - 'fresh', (tester) async { - final container = _createContainer(); - final breach = _breachGroup(); - final first = breach.items.single; - final second = first.copyWith( - id: 'breach-item-2', - ability: first.ability.copyWith( - id: 'breach-ability-2', - position: const Offset(520, 200), - ), - ); - final group = breach.copyWith(items: [first, second]); - await openEdit(tester, container, group); - - final gesture = await tester.startGesture( - tester.getCenter(abilityOf('breach-item')), - kind: PointerDeviceKind.mouse, - ); - await moveInSteps(gesture, _editDrag); - await tester.pump(); - // The cloud's version no longer has the lineup being dragged. - container.read(lineUpProvider.notifier).fromHive( - LineUpGraph.fromLegacyGroups([ - group.copyWith(items: [second]), - ]), - samePage: true, - ); - await tester.pump(); - await gesture.up(); - await tester.pumpAndSettle(); - expect(container.read(lineUpProvider).edit!.linkIds, hasLength(1)); - - final secondTopLeft = tester.getTopLeft(abilityOf('breach-item-2')); - await tester.drag(abilityOf('breach-item-2'), const Offset(30, 20)); - await tester.pump(); - final expected = CoordinateSystem.instance - .screenToCoordinate(secondTopLeft + const Offset(30, 20)); - final draft = - container.read(lineUpProvider).edit!.movedLandings['breach-item-2']!; - expect(draft.dx, closeTo(expected.dx, 0.001)); - expect(draft.dy, closeTo(expected.dy, 0.001)); - await tester.pump(const Duration(seconds: 4)); - await tester.pumpAndSettle(); - }); - - for (final cloudTakesFirstDrag in [false, true]) { - testWidgets( - 'a reload during a second drag of a spot ' - '${cloudTakesFirstDrag ? 'the cloud moved stops it' : 'it kept lets it carry on'}', - (tester) async { - final container = _createContainer(); - final group = _breachGroup(); - await openEdit(tester, container, group); - final startTopLeft = tester.getTopLeft(abilityOf('breach-item')); - Offset? draft() => - container.read(lineUpProvider).edit!.movedLandings['breach-item']; - - await tester.drag(abilityOf('breach-item'), _editDrag); - await tester.pump(); - final firstDraft = draft()!; - const second = Offset(40, 30); - final gesture = await tester.startGesture( - tester.getCenter(abilityOf('breach-item')), - kind: PointerDeviceKind.mouse, - ); - await moveInSteps(gesture, second / 2); - await tester.pump(); - // The cloud's version has the spot where the user's first drag put - // it, or where it was saved. - container.read(lineUpProvider.notifier).fromHive( - reloaded( - group, - abilityAt: cloudTakesFirstDrag - ? firstDraft - : group.items.single.ability.position, - ), - samePage: true, - ); - await tester.pump(); - await moveInSteps(gesture, second / 2); - await gesture.up(); - await tester.pumpAndSettle(); - - if (cloudTakesFirstDrag) { - expect(draft(), isNull); - expect( - find.text("The cloud's version changed a spot you dragged, so that " - 'drag was undone.'), - findsOneWidget, - ); - } else { - final expected = CoordinateSystem.instance - .screenToCoordinate(startTopLeft + _editDrag + second); - expect(draft()!.dx, closeTo(expected.dx, 0.001)); - expect(draft()!.dy, closeTo(expected.dy, 0.001)); - } - await tester.pump(const Duration(seconds: 4)); - await tester.pumpAndSettle(); - }); - } - - for (final movedFirst in [false, true]) { - testWidgets( - 'a lineup a reload removed mid-drag drags again once it is back' - '${movedFirst ? ', after an earlier reload took its drag back' : ''}', - (tester) async { - final container = _createContainer(); - final breach = _breachGroup(); - final first = breach.items.single; - final second = first.copyWith( - id: 'breach-item-2', - ability: first.ability.copyWith( - id: 'breach-ability-2', - position: const Offset(520, 200), - ), - ); - final group = breach.copyWith(items: [first, second]); - await openEdit(tester, container, group); - - final gesture = await tester.startGesture( - tester.getCenter(abilityOf('breach-item')), - kind: PointerDeviceKind.mouse, - ); - await moveInSteps(gesture, _editDrag); - await tester.pump(); - if (movedFirst) { - // The cloud moves the spot being dragged, then drops its lineup, all - // before the pointer lifts. - container.read(lineUpProvider.notifier).fromHive( - LineUpGraph.fromLegacyGroups([ - group.copyWith(items: [ - first.copyWith( - ability: first.ability.copyWith( - position: const Offset(500, 300), - ), - ), - second, - ]), - ]), - samePage: true, - ); - await tester.pump(); - } - container.read(lineUpProvider.notifier).fromHive( - LineUpGraph.fromLegacyGroups([ - group.copyWith(items: [second]), - ]), - samePage: true, - ); - await tester.pump(); - await gesture.up(); - await tester.pumpAndSettle(); - // A later reload brings it back, joined through the shared agent. - container.read(lineUpProvider.notifier).fromHive( - LineUpGraph.fromLegacyGroups([group]), - samePage: true, - ); - await tester.pumpAndSettle(); - expect(container.read(lineUpProvider).edit!.linkIds, hasLength(2)); - - await tester.drag(abilityOf('breach-item'), const Offset(30, 20)); - await tester.pump(); - expect( - container.read(lineUpProvider).edit!.movedLandings['breach-item'], - isNotNull, - ); - await tester.pump(const Duration(seconds: 4)); - await tester.pumpAndSettle(); - }); - } - testWidgets('lineup origin opens its menu on right-click', (tester) async { final container = _createContainer(); final group = _breachGroup(); diff --git a/test/per_object_undo_test.dart b/test/per_object_undo_test.dart index 19cdd9d39..d8c2c68a1 100644 --- a/test/per_object_undo_test.dart +++ b/test/per_object_undo_test.dart @@ -294,80 +294,6 @@ void main() { ); }); - test('a reload of the same page keeps a placement edit and one undo step', - () { - final container = _container(); - final lineUps = container.read(lineUpProvider.notifier) - ..fromHive(oneLineup()); - final history = container.read(actionProvider.notifier); - LineUpState lineUpState() => container.read(lineUpProvider); - - lineUps - ..startEdit('k') - ..moveEditedOrigin('o', const Offset(50, 50)) - ..saveEdit(); - lineUps - ..startEdit('k') - ..moveEditedLanding('l', const Offset(90, 90)); - // The same page comes back from the cloud, the earlier save in it. - lineUps.fromHive(lineUpState().graph, samePage: true); - - expect(lineUpState().edit!.linkIds, {'k'}); - expect(lineUpState().edit!.movedLandings, {'l': const Offset(90, 90)}); - lineUps.saveEdit(); - expect(container.read(actionProvider), hasLength(2)); - expect( - lineUpState().landingById('l')!.ability.position, - const Offset(90, 90), - ); - - // One undo takes back only the save made after the reload. - history.undoAction(); - expect( - lineUpState().landingById('l')!.ability.position, - const Offset(20, 80), - ); - expect(lineUpState().originById('o')!.agent.position, const Offset(50, 50)); - }); - - test('a lineup a reload adds on an edited spot joins the edit', () { - final container = _container(); - final lineUps = container.read(lineUpProvider.notifier) - ..fromHive(oneLineup()); - lineUps - ..startEdit('k') - ..moveEditedOrigin('o', const Offset(50, 50)); - - // A teammate added a lineup thrown from the same agent. - final graph = oneLineup(); - lineUps.fromHive( - LineUpGraph( - origins: graph.origins, - landings: [ - ...graph.landings, - LineUpLanding( - id: 'l2', - ability: PlacedAbility( - id: 'b2', - data: AgentData.agents[AgentType.jett]!.abilities.first, - position: const Offset(70, 80), - ), - ), - ], - links: [ - ...graph.links, - LineUpLink(id: 'k2', originId: 'o', landingId: 'l2'), - ], - ), - samePage: true, - ); - - final edit = container.read(lineUpProvider).edit!; - expect(edit.linkIds, {'k', 'k2'}); - expect(edit.landingIds, {'l', 'l2'}); - expect(edit.movedOrigins, {'o': const Offset(50, 50)}); - }); - test('one undo steps past entries that change nothing to one that does', () { final container = _container(); final agents = container.read(agentProvider.notifier) diff --git a/test/screenshot_view_test.dart b/test/screenshot_view_test.dart index c8b46ff5c..c22bf2049 100644 --- a/test/screenshot_view_test.dart +++ b/test/screenshot_view_test.dart @@ -162,7 +162,7 @@ class _NoopLineUpProvider extends LineUpProvider { LineUpState build() => const LineUpState(); @override - void fromHive(LineUpGraph graph, {bool samePage = false}) {} + void fromHive(LineUpGraph graph) {} } void main() { diff --git a/test/strategy_page_session_provider_test.dart b/test/strategy_page_session_provider_test.dart index 11239a986..6c9bcef4e 100644 --- a/test/strategy_page_session_provider_test.dart +++ b/test/strategy_page_session_provider_test.dart @@ -7372,83 +7372,100 @@ void main() { expect(container.read(lineupConflictsProvider), isNull); }); + /// The fan-in group's edit is refused beside a teammate's and waits, as + /// in refusedBesideTeammate, with lineup Z, a group of its own, on the + /// page too. The user then opens Edit placement on [editing] and drags + /// [landing] to [dragTo]. Returns the container. + Future refusedWhileEditing( + RemotePage page, { + required String editing, + required String landing, + required Offset dragTo, + }) async { + server = _FakeServer(page.publicId, lineups: [ + fanIn(page.publicId), + _lineup(page.publicId, 'z', sortIndex: 1), + ]); + final (container, _) = await openOnRealQueue(page); + server.teammateEdit('link-a', + (data) => _entry(data, 'links', 'link-b')['notes'] = 'theirs'); + container.read(lineUpProvider.notifier).updateLink(container + .read(lineUpProvider) + .linkById('link-a')! + .copyWith(notes: 'mine')); + await _until(() => container + .read(strategyOpQueueProvider) + .attentionByEntityKey + .containsKey(keyOf(page, 'link-a'))); + for (var i = 0; i < 10; i++) { + await _settle(); + } + container + .read(interactionStateProvider.notifier) + .editLineUpPlacement(editing); + container + .read(lineUpProvider.notifier) + .moveEditedLanding(landing, dragTo); + return container; + } + + /// Where the server has lineup Z's landing. + Offset serverLandingZ() => + lineUpGraphFromRemoteLineups([server.row('link-z')]) + .landings + .single + .ability + .position; + + Future settled(ProviderContainer container) async { + await _until( + () => container.read(strategyOpQueueProvider).pending.isEmpty); + for (var i = 0; i < 10; i++) { + await _settle(); + } + } + for (final keepBoth in [false, true]) { test( '${keepBoth ? 'Keep both' : 'Use cloud'} on another group keeps a ' 'placement edit open, and Save sends its drag as one undo step', () async { final page = _page('page-1', 0); - server = _FakeServer(page.publicId, lineups: [ - fanIn(page.publicId), - _lineup(page.publicId, 'z', sortIndex: 1), - ]); - final (container, _) = await openOnRealQueue(page); - final key = keyOf(page, 'link-a'); - StrategyOpQueueState queueState() => - container.read(strategyOpQueueProvider); - Offset serverLanding() => - lineUpGraphFromRemoteLineups([server.row('link-z')]) - .landings - .single - .ability - .position; - // The fan-in group's edit is refused beside a teammate's and waits, - // as in refusedBesideTeammate. - server.teammateEdit('link-a', - (data) => _entry(data, 'links', 'link-b')['notes'] = 'theirs'); - container.read(lineUpProvider.notifier).updateLink(container - .read(lineUpProvider) - .linkById('link-a')! - .copyWith(notes: 'mine')); - await _until(() => queueState().attentionByEntityKey.containsKey(key)); - for (var i = 0; i < 10; i++) { - await _settle(); - } - - // Mid-edit of lineup Z, its landing dragged, the user resolves the - // conflict on the other group. const dragTo = Offset(400, 250); - container - .read(interactionStateProvider.notifier) - .editLineUpPlacement('link-z'); - container - .read(lineUpProvider.notifier) - .moveEditedLanding('landing-z', dragTo); + final container = await refusedWhileEditing(page, + editing: 'link-z', landing: 'landing-z', dragTo: dragTo); + final session = container.read(strategyPageSessionProvider.notifier); if (keepBoth) { expect(await session.keepBothForRejected(), KeepBothOutcome.kept); } else { expect(await session.useCloudVersionsForRejected(), isTrue); } - await _until(() => queueState().pending.isEmpty); - for (var i = 0; i < 10; i++) { - await _settle(); - } + await settled(container); - // The page reloaded under the edit: the cloud's version of the other - // group is on screen, and the edit is still open with its drag. + // The cloud's version of the other group is on screen, and the edit + // is still open with its drag. expect( container.read(lineUpProvider).linkById('link-b')!.notes, 'theirs'); - expect(queueState().attentionByEntityKey, isEmpty); + expect(container.read(strategyOpQueueProvider).attentionByEntityKey, + isEmpty); expect(container.read(interactionStateProvider), InteractionState.lineUpEditing); final edit = container.read(lineUpProvider).edit!; expect(edit.linkIds, {'link-z'}); expect(edit.movedLandings, {'landing-z': dragTo}); - expect(serverLanding(), const Offset(30, 40)); + expect(serverLandingZ(), const Offset(30, 40)); final undoSteps = container.read(actionProvider).length; container.read(lineUpProvider.notifier).saveEdit(); container .read(interactionStateProvider.notifier) .update(InteractionState.navigation); - await _until(() => serverLanding() == dragTo); - await _until(() => queueState().pending.isEmpty); - for (var i = 0; i < 10; i++) { - await _settle(); - } + await _until(() => serverLandingZ() == dragTo); + await settled(container); - expect(queueState().attentionByEntityKey, isEmpty); + expect(container.read(strategyOpQueueProvider).attentionByEntityKey, + isEmpty); expect(container.read(actionProvider), hasLength(undoSteps + 1)); container.read(actionProvider.notifier).undoAction(); expect( @@ -7459,14 +7476,133 @@ void main() { .position, const Offset(30, 40), ); - await _until(() => serverLanding() == const Offset(30, 40)); - await _until(() => queueState().pending.isEmpty); - for (var i = 0; i < 10; i++) { - await _settle(); - } + await _until(() => serverLandingZ() == const Offset(30, 40)); + await settled(container); }); } + test( + 'Use cloud keeps a drag of a spot a teammate moved meanwhile, and ' + 'Save of it waits as a conflict instead of overwriting theirs', + () async { + final page = _page('page-1', 0); + const dragTo = Offset(400, 250); + const theirs = Offset(90, 90); + final container = await refusedWhileEditing(page, + editing: 'link-z', landing: 'landing-z', dragTo: dragTo); + server.teammateEdit( + 'link-z', + (data) => _entry(data, 'landings', 'landing-z')['ability'] = + _abilityJson('landing-z', theirs)); + + expect( + await container + .read(strategyPageSessionProvider.notifier) + .useCloudVersionsForRejected(), + isTrue); + await settled(container); + // The teammate's move waits behind the edit, which keeps the drag. + expect(container.read(lineUpProvider).edit!.movedLandings, + {'landing-z': dragTo}); + + container.read(lineUpProvider.notifier).saveEdit(); + container + .read(interactionStateProvider.notifier) + .update(InteractionState.navigation); + await _until(() => container + .read(strategyOpQueueProvider) + .attentionByEntityKey + .containsKey(keyOf(page, 'link-z'))); + + expect(serverLandingZ(), theirs); + expect( + container + .read(lineUpProvider) + .landingById('landing-z')! + .ability + .position, + dragTo, + ); + }); + + test( + 'Use cloud on the group being edited takes its cloud version and ' + 'ends the edit', () async { + final page = _page('page-1', 0); + final container = await refusedWhileEditing(page, + editing: 'link-a', + landing: 'landing', + dragTo: const Offset(400, 250)); + expect( + container.read(lineUpProvider).edit!.linkIds, {'link-a', 'link-b'}); + + expect( + await container + .read(strategyPageSessionProvider.notifier) + .useCloudVersionsForRejected(), + isTrue); + await settled(container); + + expect(container.read(lineUpProvider).edit, isNull); + expect(container.read(interactionStateProvider), + InteractionState.navigation); + final lineUps = container.read(lineUpProvider); + expect(lineUps.linkById('link-a')!.notes, 'remote lineup'); + expect(lineUps.linkById('link-b')!.notes, 'theirs'); + expect(lineUps.landingById('landing')!.ability.position, newer); + expect(container.read(strategyOpQueueProvider).attentionByEntityKey, + isEmpty); + }); + + test( + 'Use cloud takes the cloud version of a group the user is placing a ' + 'lineup from', () async { + final page = _page('page-1', 0); + final (container, _, _) = await refusedBesideTeammate(page); + container.read(lineUpProvider.notifier).startFromOrigin('origin-a'); + + expect( + await container + .read(strategyPageSessionProvider.notifier) + .useCloudVersionsForRejected(), + isTrue); + await settled(container); + + final lineUps = container.read(lineUpProvider); + expect(lineUps.linkById('link-a')!.notes, 'remote lineup'); + expect(lineUps.linkById('link-b')!.notes, 'theirs'); + expect(lineUps.placement?.pinnedOriginId, 'origin-a'); + }); + + test( + "a drag made while Use cloud loads the cloud's version stays, and " + 'the conflict still resolves', () async { + final page = _page('page-1', 0); + final container = await refusedWhileEditing(page, + editing: 'link-z', + landing: 'landing-z', + dragTo: const Offset(400, 250)); + final refreshes = liveRead.refreshCount; + final gate = liveRead.refreshGate = Completer(); + + final useCloud = container + .read(strategyPageSessionProvider.notifier) + .useCloudVersionsForRejected(); + await _until(() => liveRead.refreshCount > refreshes); + const dragTo = Offset(420, 260); + container + .read(lineUpProvider.notifier) + .moveEditedLanding('landing-z', dragTo); + gate.complete(); + + expect(await useCloud, isTrue); + await settled(container); + expect( + container.read(lineUpProvider).linkById('link-b')!.notes, 'theirs'); + expect(container.read(lineUpProvider).edit!.movedLandings, + {'landing-z': dragTo}); + }); + test( 'Keep both whose cloud version cannot be loaded changes nothing, so ' 'pressing it again cannot copy twice', () async { From eb4c0a16d2be47740f11855d8b2f93086a4bf1a0 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Fri, 9 Oct 2026 23:55:22 -0400 Subject: [PATCH 6/7] Restore failed discards over holds; resume held-back changes after When part of Use cloud's discard fails, the redraw that follows puts the user's version back on every refused entity, held or not; holding one kept the cloud's version on screen while the work still waited. A teammate's change held back by a hold the user let go of during the discard waited for a reapply that conflict resolution then cleared. It now resumes once resolution ends. Co-Authored-By: Claude Opus 5.5 --- .../strategy_page_session_provider.dart | 28 +++--- test/strategy_page_session_provider_test.dart | 98 ++++++++++++++++++- 2 files changed, 111 insertions(+), 15 deletions(-) diff --git a/lib/providers/strategy_page_session_provider.dart b/lib/providers/strategy_page_session_provider.dart index f24eda99d..71bad858e 100644 --- a/lib/providers/strategy_page_session_provider.dart +++ b/lib/providers/strategy_page_session_provider.dart @@ -865,14 +865,15 @@ class StrategyPageSessionNotifier extends Notifier { strategyId: strategyId, activePageId: () => state.activePageId, ); - // Redrawn only if nothing was edited since the first redraw. + // Redrawn only if nothing was edited since the first redraw. The + // work that stays comes back over the cloud's version drawn first. final pageData = await pageSource.loadAuthoritativePage( targetPageId, discardedEntities: discarded, ); await _showCloudVersions( pageData, - of: discarded, + of: rejected.keys.toSet(), strategyId: strategyId, canApply: () => !_disposed && @@ -905,19 +906,22 @@ class StrategyPageSessionNotifier extends Notifier { ref .read(strategyOpQueueProvider.notifier) .completeRemoteAdoption(discarded); - _pendingRemoteReapply = false; return discarded.length == rejected.length; } finally { _isResolvingConflicts = false; + // A teammate's change that waited for this, or for a hold the user + // let go of meanwhile, applies now. + if (!_disposed) _resumePendingRemoteReapplyIfPossible(); } } - /// Draws [pageData], which has the cloud's version of the entities in - /// [of]. The page on screen takes it item by item, as it takes a + /// Draws [pageData], which has the version of the entities in [of] that + /// the user's choice left: the cloud's, or their own where it could not be + /// discarded. The page on screen takes it item by item, as it takes a /// teammate's change, so what the user is in the middle of stays as it is: - /// a placement edit of other lineups keeps its drags. A placement edit of - /// a lineup in [of] ends, since the user chose the cloud's version of it. - /// Any other page loads whole. + /// a placement edit of other lineups keeps its drags. The entities in [of] + /// take [pageData]'s version even when held, and a placement edit of a + /// lineup among them ends. Any other page loads whole. Future _showCloudVersions( StrategyEditorPageData pageData, { required Set of, @@ -951,7 +955,7 @@ class StrategyPageSessionNotifier extends Notifier { .listPageIds(); if (!canApply()) return; // Whether the canvas item [id] (an element, or a lineup or one of its - // spots) is one the user chose the cloud's version of. + // spots) is in [of]. bool chosen(String id) => of.contains(EntitySyncKey.element(pageId, id)) || switch (liveSync.lineupGroupOf(pageId, id)) { @@ -968,9 +972,9 @@ class StrategyPageSessionNotifier extends Notifier { pageData, strategyId: strategyId, snapshot: loadedRemoteSnapshot, - // What the user is in the middle of stays, except what they chose the - // cloud's version of. A press off the canvas (on the conflict panel, - // say) holds nothing on it. + // What the user is in the middle of stays, except what is in [of]. A + // press off the canvas (on the conflict panel, say) holds nothing on + // it. holding: { for (final id in ref.read(editorHeldCanvasItemsProvider)) if (!chosen(id)) id, diff --git a/test/strategy_page_session_provider_test.dart b/test/strategy_page_session_provider_test.dart index 6c9bcef4e..e1bbc8412 100644 --- a/test/strategy_page_session_provider_test.dart +++ b/test/strategy_page_session_provider_test.dart @@ -6490,12 +6490,13 @@ void main() { /// sends to [server] through [repository] and reads it back after every /// batch. A batch waits for [hold] while it is set. The queue reaches /// the server while [_online] holds. Returns the container and the - /// batches sent. + /// batches sent. The server's header names [themeProfileId], if given. Future<(ProviderContainer, List>)> openOnRealQueue( RemotePage page, { DurableStrategyOutboxStore? store, Completer? hold, List otherPages = const [], + String? themeProfileId, }) async { final others = { for (final other in otherPages) @@ -6504,6 +6505,7 @@ void main() { var shown = page.publicId; RemoteEditorSnapshot read() => _editorSnapshot( pages: [page, ...otherPages], + themeProfileId: themeProfileId, activePage: others[shown] ?? _pageSnapshot( page, @@ -7375,18 +7377,22 @@ void main() { /// The fan-in group's edit is refused beside a teammate's and waits, as /// in refusedBesideTeammate, with lineup Z, a group of its own, on the /// page too. The user then opens Edit placement on [editing] and drags - /// [landing] to [dragTo]. Returns the container. + /// [landing] to [dragTo]. The outbox is kept in [store]. The server's + /// header has the theme the client uses, so redrawing the page queues no + /// strategy change of its own. Returns the container. Future refusedWhileEditing( RemotePage page, { required String editing, required String landing, required Offset dragTo, + DurableStrategyOutboxStore? store, }) async { server = _FakeServer(page.publicId, lineups: [ fanIn(page.publicId), _lineup(page.publicId, 'z', sortIndex: 1), ]); - final (container, _) = await openOnRealQueue(page); + final (container, _) = await openOnRealQueue(page, + store: store, themeProfileId: 'immutable-default-map-theme'); server.teammateEdit('link-a', (data) => _entry(data, 'links', 'link-b')['notes'] = 'theirs'); container.read(lineUpProvider.notifier).updateLink(container @@ -7494,6 +7500,11 @@ void main() { 'link-z', (data) => _entry(data, 'landings', 'landing-z')['ability'] = _abilityJson('landing-z', theirs)); + // The live read shows it; the edit holds it back. + showServer(); + for (var i = 0; i < 10; i++) { + await _settle(); + } expect( await container @@ -7560,6 +7571,9 @@ void main() { final page = _page('page-1', 0); final (container, _, _) = await refusedBesideTeammate(page); container.read(lineUpProvider.notifier).startFromOrigin('origin-a'); + container + .read(interactionStateProvider.notifier) + .update(InteractionState.lineUpPlacing); expect( await container @@ -7572,6 +7586,84 @@ void main() { expect(lineUps.linkById('link-a')!.notes, 'remote lineup'); expect(lineUps.linkById('link-b')!.notes, 'theirs'); expect(lineUps.placement?.pinnedOriginId, 'origin-a'); + expect(container.read(interactionStateProvider), + InteractionState.lineUpPlacing); + }); + + test( + "a teammate's move a placement edit held back applies once the user " + 'ends the edit while Use cloud discards', () async { + final page = _page('page-1', 0); + final store = _FailingOutboxStore(); + const theirs = Offset(90, 90); + final container = await refusedWhileEditing(page, + editing: 'link-z', + landing: 'landing-z', + dragTo: const Offset(400, 250), + store: store); + server.teammateEdit( + 'link-z', + (data) => _entry(data, 'landings', 'landing-z')['ability'] = + _abilityJson('landing-z', theirs)); + // The live read shows it; the edit holds it back. + showServer(); + for (var i = 0; i < 10; i++) { + await _settle(); + } + var ended = false; + store.onRemove = (storageKey) { + if (ended) return; + ended = true; + container + .read(interactionStateProvider.notifier) + .update(InteractionState.navigation); + }; + + expect( + await container + .read(strategyPageSessionProvider.notifier) + .useCloudVersionsForRejected(), + isTrue); + expect(ended, isTrue); + await settled(container); + + await _until(() => + container + .read(lineUpProvider) + .landingById('landing-z')! + .ability + .position == + theirs); + expect(serverLandingZ(), theirs); + }); + + test( + "Use cloud whose discard fails puts the user's version back on a " + 'group they are placing a lineup from', () async { + final page = _page('page-1', 0); + final store = _FailingOutboxStore(); + final (container, _, _) = await refusedBesideTeammate(page, store: store); + container.read(lineUpProvider.notifier).startFromOrigin('origin-a'); + container + .read(interactionStateProvider.notifier) + .update(InteractionState.lineUpPlacing); + store.failRemove = (storageKey) => true; + + expect( + await container + .read(strategyPageSessionProvider.notifier) + .useCloudVersionsForRejected(), + isFalse); + for (var i = 0; i < 10; i++) { + await _settle(); + } + + // The refused work still waits, and the canvas shows it. + final lineUps = container.read(lineUpProvider); + expect(lineUps.linkById('link-a')!.notes, 'mine'); + expect(lineUps.linkById('link-b')!.notes, 'remote lineup'); + expect(container.read(strategyOpQueueProvider).attentionByEntityKey.keys, + [keyOf(page, 'link-a')]); }); test( From b8c3f3026ca7b743d986980a73ca24743a8c3fec Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Sat, 10 Oct 2026 11:24:00 -0400 Subject: [PATCH 7/7] Pin how Save of a placement edit merges by field On a server that merges by field, Save names only the spots the user dragged, measured from the version the edit started from: the user's drag wins over a teammate's move of that spot, and the teammate's move of another spot the edit held back stays. The test that Save waits as a conflict now says it runs on a server that checks whole groups. Co-Authored-By: Claude Opus 5.5 --- test/strategy_page_session_provider_test.dart | 64 ++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/test/strategy_page_session_provider_test.dart b/test/strategy_page_session_provider_test.dart index 607dbd120..32f224c33 100644 --- a/test/strategy_page_session_provider_test.dart +++ b/test/strategy_page_session_provider_test.dart @@ -7995,8 +7995,11 @@ void main() { test( 'Use cloud keeps a drag of a spot a teammate moved meanwhile, and ' - 'Save of it waits as a conflict instead of overwriting theirs', + 'Save of it is checked against the version the edit started from', () async { + // This server checks each group whole, as servers did before they + // merged by field: Save waits as a conflict instead of overwriting the + // teammate's move. One that merges is the next test. final page = _page('page-1', 0); const dragTo = Offset(400, 250); const theirs = Offset(90, 90); @@ -8042,6 +8045,65 @@ void main() { ); }); + test( + "Save of a drag merges by spot: it wins over a teammate's move of " + 'that spot, and their move of another stays', () async { + final page = _page('page-1', 0); + const dragTo = Offset(400, 250); + server = _FakeServer(page.publicId, lineups: [fanIn(page.publicId)]) + ..mergesByField = true; + final (container, batches) = await openOnRealQueue(page, + themeProfileId: 'immutable-default-map-theme'); + for (var i = 0; i < 10; i++) { + await _settle(); + } + container + .read(interactionStateProvider.notifier) + .editLineUpPlacement('link-a'); + container + .read(lineUpProvider.notifier) + .moveEditedLanding('landing', dragTo); + // Meanwhile a teammate moves that landing and origin B, which the edit + // also holds: both wait behind it. + server.teammateEdit('link-a', (data) { + _entry(data, 'landings', 'landing')['ability'] = + _abilityJson('landing', const Offset(90, 90)); + _entry(data, 'origins', 'origin-b')['agent'] = + _agentJson('origin-b', const Offset(70, 70)); + }); + showServer(); + for (var i = 0; i < 10; i++) { + await _settle(); + } + + container.read(lineUpProvider.notifier).saveEdit(); + container + .read(interactionStateProvider.notifier) + .update(InteractionState.navigation); + await _until(() => sentFor(batches, keyOf(page, 'link-a')).isNotEmpty); + await settled(container); + + // Save names only the spot the user dragged, measured from the version + // the edit started from, so the origin the teammate moved is not sent + // back to where it was. + final saved = sentFor(batches, keyOf(page, 'link-a')).last; + expect(saved.merge?.fields, ['landings/landing']); + final row = lineUpGraphFromRemoteLineups([server.row('link-a')]); + expect(row.landings.single.ability.position, dragTo); + expect(row.origins.singleWhere((o) => o.id == 'origin-b').agent.position, + const Offset(70, 70)); + expect(container.read(strategyOpQueueProvider).attentionByEntityKey, + isEmpty); + showServer(); + for (var i = 0; i < 10; i++) { + await _settle(); + } + final lineUps = container.read(lineUpProvider); + expect(lineUps.landingById('landing')!.ability.position, dragTo); + expect( + lineUps.originById('origin-b')!.agent.position, const Offset(70, 70)); + }); + test( 'Use cloud on the group being edited takes its cloud version and ' 'ends the edit', () async {