From e774efd68e7bd49b83461fdcae0d110c2b69da90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Augustin=20=C5=A0ulc?= Date: Fri, 28 Jan 2022 10:38:28 +0100 Subject: [PATCH] Fixed navigationPrefix handling for child screens --- .../conductors/activeChildConductor.test.ts | 4 +++- .../navigation/lifecycleScreenNavigatorBase.test.ts | 2 +- .../src/navigation/conductors/activeChildConductor.ts | 10 +++++----- .../conductors/allChildrenActiveConductor.ts | 4 ++-- .../navigation/conductors/oneOfListActiveConductor.ts | 10 ++++++---- .../src/navigation/lifecycleScreenNavigatorBase.ts | 11 +++++++++-- .../screens/src/navigation/simpleScreenNavigator.ts | 6 +----- 7 files changed, 27 insertions(+), 20 deletions(-) diff --git a/packages/screens/__tests__/navigation/conductors/activeChildConductor.test.ts b/packages/screens/__tests__/navigation/conductors/activeChildConductor.test.ts index c5eb3566..5fc3103d 100644 --- a/packages/screens/__tests__/navigation/conductors/activeChildConductor.test.ts +++ b/packages/screens/__tests__/navigation/conductors/activeChildConductor.test.ts @@ -7,7 +7,7 @@ import { testLifecycle } from "../navigator.testHelpers"; describe("ActiveChildConductor", () => { testLifecycle((screen, eventHub) => { - const conductor = new ActiveChildConductor(screen, eventHub); + const conductor = new ActiveChildConductor(screen, undefined, eventHub); conductor.findNavigationChild = () => Promise.resolve({ newChild: undefined }); return conductor; }); @@ -161,4 +161,6 @@ describe("ActiveChildConductor", () => { expect(childNavigator.deactivate).toBeCalledWith(true); }); }); + + // TODO test proper child finding with navigationPrefix used }); diff --git a/packages/screens/__tests__/navigation/lifecycleScreenNavigatorBase.test.ts b/packages/screens/__tests__/navigation/lifecycleScreenNavigatorBase.test.ts index 42ff079f..2a32d0a4 100644 --- a/packages/screens/__tests__/navigation/lifecycleScreenNavigatorBase.test.ts +++ b/packages/screens/__tests__/navigation/lifecycleScreenNavigatorBase.test.ts @@ -4,5 +4,5 @@ import { testLifecycle } from "./navigator.testHelpers"; class TestNavigator extends LifecycleScreenNavigatorBase {} describe("LifecycleScreenNavigatorBase", () => { - testLifecycle((screen, eventHub) => new TestNavigator(screen, eventHub)); + testLifecycle((screen, eventHub) => new TestNavigator(screen, undefined, eventHub)); }); diff --git a/packages/screens/src/navigation/conductors/activeChildConductor.ts b/packages/screens/src/navigation/conductors/activeChildConductor.ts index 5f085478..9f462a04 100644 --- a/packages/screens/src/navigation/conductors/activeChildConductor.ts +++ b/packages/screens/src/navigation/conductors/activeChildConductor.ts @@ -81,16 +81,16 @@ export default class ActiveChildConductor< const currentChildNavigator = getNavigator(currentChild); await currentChildNavigator?.deactivate?.(!!childResult.closePrevious); + if (isChildFoundResult(childResult) && childResult.attachToParent !== false) { + this.connectChild(childResult.newChild); + } + runInAction(() => (this.activeChildValue = childResult.newChild)); } if (isChildFoundResult(childResult)) { - if (childResult.attachToParent !== false) { - this.connectChild(childResult.newChild); - } - const newChildNavigator = getNavigator(childResult.newChild); - await newChildNavigator?.navigate(childResult.pathForChild ?? path.slice(1)); + await newChildNavigator?.navigate(childResult.pathForChild ?? path.slice(this.getNavigationStateLength())); } } diff --git a/packages/screens/src/navigation/conductors/allChildrenActiveConductor.ts b/packages/screens/src/navigation/conductors/allChildrenActiveConductor.ts index 26c519c7..e1fdcae3 100644 --- a/packages/screens/src/navigation/conductors/allChildrenActiveConductor.ts +++ b/packages/screens/src/navigation/conductors/allChildrenActiveConductor.ts @@ -11,8 +11,8 @@ export default class AllChildrenActiveConductor< > extends LifecycleScreenNavigatorBase { readonly children: TChild[]; - constructor(screen?: TScreen, eventHub?: ScreenLifecycleEventHub) { - super(screen, eventHub); + constructor(screen?: TScreen, navigationPrefix?: string, eventHub?: ScreenLifecycleEventHub) { + super(screen, navigationPrefix, eventHub); const children = observable.array([], { deep: false }); children.intercept(this.handleChildrenChanged); diff --git a/packages/screens/src/navigation/conductors/oneOfListActiveConductor.ts b/packages/screens/src/navigation/conductors/oneOfListActiveConductor.ts index fb45b8ac..27418a2b 100644 --- a/packages/screens/src/navigation/conductors/oneOfListActiveConductor.ts +++ b/packages/screens/src/navigation/conductors/oneOfListActiveConductor.ts @@ -17,8 +17,8 @@ export default class OneOfListActiveConductor< /** When set to `true`, navigating directly to the conductor (with no child path specified) activates the previously set `activeChild`. */ preserveActiveChild = false; - constructor(screen?: TScreen, eventHub?: ScreenLifecycleEventHub) { - super(screen, eventHub); + constructor(screen?: TScreen, navigationPrefix?: string, eventHub?: ScreenLifecycleEventHub) { + super(screen, navigationPrefix, eventHub); const children = observable.array([], { deep: false }); children.intercept(this.handleChildrenChanged); @@ -26,7 +26,8 @@ export default class OneOfListActiveConductor< } canChangeActiveChild = async (context: NavigationContext, currentChild: TChild | undefined) => { - const newNavigationName = context.path[1]?.name; + const pathElementsToSkip = this.getNavigationStateLength(); + const newNavigationName = context.path[pathElementsToSkip]?.name; const activeChildNavigator = getNavigator(this.activeChild); if (!activeChildNavigator || activeChildNavigator.navigationName === newNavigationName) { @@ -37,7 +38,8 @@ export default class OneOfListActiveConductor< }; findNavigationChild = (context: NavigationContext, currentChild: TChild | undefined) => { - const searchedNavigationName = context.path[1]?.name; + const pathElementsToSkip = this.getNavigationStateLength(); + const searchedNavigationName = context.path[pathElementsToSkip]?.name; const newChild = this.findChild(searchedNavigationName); return { newChild, closePrevious: false } as FindChildResult; }; diff --git a/packages/screens/src/navigation/lifecycleScreenNavigatorBase.ts b/packages/screens/src/navigation/lifecycleScreenNavigatorBase.ts index 7aae25fc..d5c7b68f 100644 --- a/packages/screens/src/navigation/lifecycleScreenNavigatorBase.ts +++ b/packages/screens/src/navigation/lifecycleScreenNavigatorBase.ts @@ -32,9 +32,14 @@ export default abstract class LifecycleScreenNavigatorBase< parent: ScreenNavigator | undefined = undefined; - constructor(screen?: TScreen, eventHub?: ScreenLifecycleEventHub) { + constructor(screen?: TScreen, navigationPrefix?: string, eventHub?: ScreenLifecycleEventHub) { this.screenValue = screen; this.eventHub = eventHub ?? screen?.events; + + if (navigationPrefix) { + this.getNavigationState = () => [{ name: navigationPrefix }, this.createDefaultNavigationState()]; + this.getNavigationStateLength = () => 2; + } } canNavigate(path: PathElement[]) { @@ -50,8 +55,10 @@ export default abstract class LifecycleScreenNavigatorBase< getNavigationParams?: () => TNavigationParams | undefined; getNavigationState: () => PathElement[] = () => [this.createDefaultNavigationState()]; + // Current NavigationState can contain multiple elements (not just one). In that case, we need to skip all of them. + getNavigationStateLength: () => number = () => 1; - createDefaultNavigationState() { + protected createDefaultNavigationState() { return { name: this.navigationName, params: this.getNavigationParams?.(), diff --git a/packages/screens/src/navigation/simpleScreenNavigator.ts b/packages/screens/src/navigation/simpleScreenNavigator.ts index 95192f87..ff2ab61b 100644 --- a/packages/screens/src/navigation/simpleScreenNavigator.ts +++ b/packages/screens/src/navigation/simpleScreenNavigator.ts @@ -8,10 +8,6 @@ export default class SimpleScreenNavigator< TNavigationParams extends Record = Record > extends LifecycleScreenNavigatorBase { constructor(screen?: TScreen, navigationPrefix?: string, eventHub?: ScreenLifecycleEventHub) { - super(screen, eventHub); - - if (navigationPrefix) { - this.getNavigationState = () => [{ name: navigationPrefix }, this.createDefaultNavigationState()]; - } + super(screen, navigationPrefix, eventHub); } }