diff --git a/.changeset/lower-activity-step-actions.md b/.changeset/lower-activity-step-actions.md new file mode 100644 index 000000000..3bad2c662 --- /dev/null +++ b/.changeset/lower-activity-step-actions.md @@ -0,0 +1,27 @@ +--- +"@stackflow/core": minor +"@stackflow/plugin-history-sync": minor +--- + +Allow step actions to target lower activities with targetActivityId + +**Core Changes:** +- Step actions (stepPush, stepPop, stepReplace) can now target any activity in the stack using `targetActivityId` option +- Previously only the top activity could be targeted +- Enables modifying lower activity steps when popping current activity + +**History Sync Plugin Changes:** +- Added intelligent synchronization when navigating to lower activities with removed steps +- When navigating back to a step that was removed via `stepPop`, automatically skips the removed entry +- Uses `history.back()` to navigate to the correct remaining step + +**Use Case:** +```typescript +// Remove a step from a lower activity while at top activity +actions.stepPop({ targetActivityId: lowerActivityId }); + +// When user navigates back, the removed step entry is automatically skipped +history.back(); // Skips removed step, lands on the correct step +``` + +This is backward compatible - existing code works without changes. diff --git a/core/src/activity-utils/findTargetActivityIndices.ts b/core/src/activity-utils/findTargetActivityIndices.ts index 496554ac5..c4041ec0e 100644 --- a/core/src/activity-utils/findTargetActivityIndices.ts +++ b/core/src/activity-utils/findTargetActivityIndices.ts @@ -77,30 +77,34 @@ export function findTargetActivityIndices( } case "StepPushed": case "StepReplaced": { - const latestActivity = findLatestActiveActivity(activities); + let targetActivity: Activity | undefined; - if (latestActivity) { - if ( - event.targetActivityId && - event.targetActivityId !== latestActivity.id - ) { - break; - } - targetActivities.push(activities.indexOf(latestActivity)); + if (event.targetActivityId) { + targetActivity = activities.find( + (activity) => activity.id === event.targetActivityId, + ); + } else { + targetActivity = findLatestActiveActivity(activities); + } + + if (targetActivity) { + targetActivities.push(activities.indexOf(targetActivity)); } break; } case "StepPopped": { - const latestActivity = findLatestActiveActivity(activities); - - if (latestActivity && latestActivity.steps.length > 1) { - if ( - event.targetActivityId && - event.targetActivityId !== latestActivity.id - ) { - break; - } - targetActivities.push(activities.indexOf(latestActivity)); + let targetActivity: Activity | undefined; + + if (event.targetActivityId) { + targetActivity = activities.find( + (activity) => activity.id === event.targetActivityId, + ); + } else { + targetActivity = findLatestActiveActivity(activities); + } + + if (targetActivity && targetActivity.steps.length > 1) { + targetActivities.push(activities.indexOf(targetActivity)); } break; diff --git a/core/src/aggregate.spec.ts b/core/src/aggregate.spec.ts index ec5c0e869..de6386527 100644 --- a/core/src/aggregate.spec.ts +++ b/core/src/aggregate.spec.ts @@ -4,6 +4,7 @@ import type { PoppedEvent, PushedEvent, ReplacedEvent, + StepPoppedEvent, StepPushedEvent, StepReplacedEvent, } from "./event-types"; @@ -3974,7 +3975,7 @@ test("aggregate - After Push > Push > Pop > Replace, first pushed activity shoul }); }); -test("aggregate - StepPushedEvent must be ignored when top activity is not target activity", () => { +test("aggregate - StepPushedEvent can target lower activity with targetActivityId", () => { const t = nowTime(); let pushedEvent1: PushedEvent; @@ -4002,7 +4003,7 @@ test("aggregate - StepPushedEvent must be ignored when top activity is not targe })), (stepPushedEvent = makeEvent("StepPushed", { stepId: "s1", - stepParams: {}, + stepParams: { tab: "profile" }, targetActivityId: pushedEvent1.activityId, eventDate: enoughPastTime(), })), @@ -4016,7 +4017,7 @@ test("aggregate - StepPushedEvent must be ignored when top activity is not targe id: "A", name: "sample", transitionState: "enter-done", - params: {}, + params: { tab: "profile" }, steps: [ { id: "A", @@ -4024,6 +4025,13 @@ test("aggregate - StepPushedEvent must be ignored when top activity is not targe enteredBy: pushedEvent1, zIndex: 0, }, + { + id: "s1", + params: { tab: "profile" }, + enteredBy: stepPushedEvent, + zIndex: 0, + hasZIndex: false, + }, ], enteredBy: pushedEvent1, isActive: false, @@ -4333,3 +4341,197 @@ test("aggregate - StepPushedEvent에 hasZIndex 필드가 true이면, Step에 zIn events, }); }); + +test("aggregate - StepReplacedEvent can target lower activity with targetActivityId", () => { + const t = nowTime(); + + let pushedEvent1: PushedEvent; + let pushedEvent2: PushedEvent; + let stepReplacedEvent: StepReplacedEvent; + + const events = [ + initializedEvent({ + transitionDuration: 300, + }), + registeredEvent({ + activityName: "sample", + }), + (pushedEvent1 = makeEvent("Pushed", { + activityId: "A", + activityName: "sample", + activityParams: {}, + eventDate: enoughPastTime(), + })), + (pushedEvent2 = makeEvent("Pushed", { + activityId: "B", + activityName: "sample", + activityParams: {}, + eventDate: enoughPastTime(), + })), + (stepReplacedEvent = makeEvent("StepReplaced", { + stepId: "s1", + stepParams: { newParam: "value" }, + targetActivityId: pushedEvent1.activityId, + eventDate: enoughPastTime(), + })), + ]; + + const output = aggregate(events, t + 300); + + expect(output).toStrictEqual({ + activities: [ + activity({ + id: "A", + name: "sample", + transitionState: "enter-done", + params: { newParam: "value" }, + steps: [ + { + id: "s1", + params: { newParam: "value" }, + enteredBy: stepReplacedEvent, + zIndex: 0, + hasZIndex: false, + }, + ], + enteredBy: pushedEvent1, + isActive: false, + isTop: false, + isRoot: true, + zIndex: 0, + }), + activity({ + id: "B", + name: "sample", + transitionState: "enter-done", + params: {}, + steps: [ + { + id: "B", + params: {}, + enteredBy: pushedEvent2, + zIndex: 1, + }, + ], + enteredBy: pushedEvent2, + isActive: true, + isTop: true, + isRoot: false, + zIndex: 1, + }), + ], + registeredActivities: [ + { + name: "sample", + }, + ], + transitionDuration: 300, + globalTransitionState: "idle", + events, + }); +}); + +test("aggregate - StepPoppedEvent can target lower activity with targetActivityId", () => { + const t = nowTime(); + + let pushedEvent1: PushedEvent; + let pushedEvent2: PushedEvent; + let stepPushedEvent1: StepPushedEvent; + let stepPushedEvent2: StepPushedEvent; + let stepPoppedEvent: StepPoppedEvent; + + const events = [ + initializedEvent({ + transitionDuration: 300, + }), + registeredEvent({ + activityName: "sample", + }), + (pushedEvent1 = makeEvent("Pushed", { + activityId: "A", + activityName: "sample", + activityParams: {}, + eventDate: enoughPastTime(), + })), + (stepPushedEvent1 = makeEvent("StepPushed", { + stepId: "s1", + stepParams: { step: "1" }, + eventDate: enoughPastTime(), + })), + (stepPushedEvent2 = makeEvent("StepPushed", { + stepId: "s2", + stepParams: { step: "2" }, + eventDate: enoughPastTime(), + })), + (pushedEvent2 = makeEvent("Pushed", { + activityId: "B", + activityName: "sample", + activityParams: {}, + eventDate: enoughPastTime(), + })), + (stepPoppedEvent = makeEvent("StepPopped", { + targetActivityId: pushedEvent1.activityId, + eventDate: enoughPastTime(), + })), + ]; + + const output = aggregate(events, t + 300); + + expect(output).toStrictEqual({ + activities: [ + activity({ + id: "A", + name: "sample", + transitionState: "enter-done", + params: { step: "1" }, + steps: [ + { + id: "A", + params: {}, + enteredBy: pushedEvent1, + zIndex: 0, + }, + { + id: "s1", + params: { step: "1" }, + enteredBy: stepPushedEvent1, + zIndex: 0, + hasZIndex: false, + }, + ], + enteredBy: pushedEvent1, + isActive: false, + isTop: false, + isRoot: true, + zIndex: 0, + }), + activity({ + id: "B", + name: "sample", + transitionState: "enter-done", + params: {}, + steps: [ + { + id: "B", + params: {}, + enteredBy: pushedEvent2, + zIndex: 1, + }, + ], + enteredBy: pushedEvent2, + isActive: true, + isTop: true, + isRoot: false, + zIndex: 1, + }), + ], + registeredActivities: [ + { + name: "sample", + }, + ], + transitionDuration: 300, + globalTransitionState: "idle", + events, + }); +}); diff --git a/extensions/plugin-history-sync/src/historySyncPlugin.spec.ts b/extensions/plugin-history-sync/src/historySyncPlugin.spec.ts index 5ba28d372..c225a9d37 100644 --- a/extensions/plugin-history-sync/src/historySyncPlugin.spec.ts +++ b/extensions/plugin-history-sync/src/historySyncPlugin.spec.ts @@ -1463,4 +1463,139 @@ describe("historySyncPlugin", () => { */ expect(queryResponse.data.hello).toEqual("world"); }); + + test("historySyncPlugin - stepPop on lower activity skips removed step when navigating back", async () => { + actions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "10", + title: "step1", + }, + }); + + actions.stepPush({ + stepId: "s1", + stepParams: { + articleId: "11", + title: "step2", + }, + }); + + actions.stepPush({ + stepId: "s2", + stepParams: { + articleId: "12", + title: "step3", + }, + }); + + await actions.push({ + activityId: "a2", + activityName: "Article", + activityParams: { + articleId: "20", + title: "second", + }, + }); + + expect(history.index).toEqual(4); + + // Pop step from lower activity a1 + actions.stepPop({ targetActivityId: "a1" }); + + // Navigate back - should skip the removed step3 entry and land on step2 + history.back(); + + const stack = await actions.getStack(); + const active = activeActivity(stack); + expect(active?.id).toEqual("a1"); + expect(active?.steps.length).toEqual(2); // step1 and step2 remain + expect(active?.steps[1]?.params.title).toEqual("step2"); + expect(path(history.location)).toEqual("/articles/11/?title=step2"); + }); + + test("historySyncPlugin - stepPush on lower activity does not affect current history", async () => { + actions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "10", + title: "first", + }, + }); + + await actions.push({ + activityId: "a2", + activityName: "Article", + activityParams: { + articleId: "20", + title: "second", + }, + }); + + const historyIndexBefore = history.index; + const historyPathBefore = path(history.location); + + // Add step to lower activity a1 while at a2 + await actions.stepPush({ + stepId: "s1", + stepParams: { + articleId: "15", + title: "added-step", + }, + targetActivityId: "a1", + }); + + // Current history should NOT change (this is the bug we're testing for) + expect(history.index).toEqual(historyIndexBefore); + expect(path(history.location)).toEqual(historyPathBefore); + expect(path(history.location)).toEqual("/articles/20/?title=second"); + + // Stack state should reflect the change + const stack = await actions.getStack(); + const lowerActivity = stack.activities.find((a) => a.id === "a1"); + expect(lowerActivity?.steps.length).toEqual(2); // initial params + added step + expect(lowerActivity?.steps[1]?.params.title).toEqual("added-step"); + }); + + test("historySyncPlugin - stepPush on lower activity syncs when navigating back", async () => { + actions.push({ + activityId: "a1", + activityName: "Article", + activityParams: { + articleId: "10", + title: "first", + }, + }); + + await actions.push({ + activityId: "a2", + activityName: "Article", + activityParams: { + articleId: "20", + title: "second", + }, + }); + + // Add step to lower activity a1 while at a2 + await actions.stepPush({ + stepId: "s1", + stepParams: { + articleId: "15", + title: "added-step", + }, + targetActivityId: "a1", + }); + + // Navigate back to a1 - should sync to show the added step + history.back(); + + const stack = await actions.getStack(); + const active = activeActivity(stack); + expect(active?.id).toEqual("a1"); + expect(active?.steps.length).toEqual(2); + expect(active?.steps[1]?.params.title).toEqual("added-step"); + expect(path(history.location)).toEqual("/articles/15/?title=added-step"); + }); }); diff --git a/extensions/plugin-history-sync/src/historySyncPlugin.tsx b/extensions/plugin-history-sync/src/historySyncPlugin.tsx index 2082dabb1..9ee62513f 100644 --- a/extensions/plugin-history-sync/src/historySyncPlugin.tsx +++ b/extensions/plugin-history-sync/src/historySyncPlugin.tsx @@ -373,9 +373,10 @@ export function historySyncPlugin< return; } - const targetActivity = state.activity; + // Use mutable variables so we can update them if needed + let targetActivity = state.activity; const targetActivityId = state.activity.id; - const targetStep = state.step; + let targetStep = state.step; const { activities } = getStack(); const currentActivity = activities.find( @@ -395,6 +396,54 @@ export function historySyncPlugin< (step) => step.id === targetStep?.id, ); + // Synchronize history state with core state for lower activity step modifications + // Only apply this logic for non-active (lower) activities to avoid interfering + // with normal step navigation on the active activity + if (nextActivity && !nextActivity.isActive) { + const coreSteps = nextActivity.steps; + const coreLastStep = last(coreSteps); + const historySteps = targetActivity.steps; + + let needsSync = false; + + // Case 1: History has a step that doesn't exist in core (stepPop or stepReplace) + if (targetStep && !coreSteps.some((step) => step.id === targetStep?.id)) { + if (coreSteps.length < historySteps.length) { + // Step Pop: Use history.back() to skip removed step entry + history.back(); + return; + } + needsSync = true; // stepReplace or complex operations + } + // Case 2: History has no step but core has steps (stepPush added to lower activity) + else if (!targetStep && coreSteps.length > 0) { + needsSync = true; + } + + if (needsSync) { + // Sync history with core state + const match = activityRoutes.find( + (r) => r.activityName === nextActivity.name, + )!; + const template = makeTemplate(match, options.urlPatternOptions); + const paramsForUrl = coreLastStep?.params ?? nextActivity.params; + + replaceState({ + history, + pathname: template.fill(paramsForUrl), + state: { + activity: nextActivity, + step: coreLastStep, + }, + useHash: options.useHash, + }); + + // Update target variables for normal navigation logic + targetActivity = nextActivity; + targetStep = coreLastStep; + } + } + const isBackward = () => currentActivity.id > targetActivityId; const isForward = () => currentActivity.id < targetActivityId; const isStep = () => currentActivity.id === targetActivityId; @@ -528,6 +577,11 @@ export function historySyncPlugin< return; } + // Only update history for active activity + if (!activity.isActive) { + return; + } + const match = activityRoutes.find( (r) => r.activityName === activity.name, )!; @@ -666,7 +720,12 @@ export function historySyncPlugin< } } }, - onBeforeStepPop({ actions: { getStack } }) { + onBeforeStepPop({ actionParams, actions: { getStack } }) { + // Only handle history for active activity step pops + if (actionParams?.targetActivityId) { + return; // Lower activity step pop, don't modify history + } + const { activities } = getStack(); const currentActivity = activities.find( (activity) => activity.isActive,