diff --git a/packages/core/ui/frame/fragment.transitions.android.ts b/packages/core/ui/frame/fragment.transitions.android.ts index 57bb455951..70ce2aeb9f 100644 --- a/packages/core/ui/frame/fragment.transitions.android.ts +++ b/packages/core/ui/frame/fragment.transitions.android.ts @@ -119,7 +119,7 @@ export function _setAndroidFragmentTransitions(animated: boolean, navigationTran curve: transition.getCurve(), }, newEntry, - transition + transition, ); if (currentFragmentNeedsDifferentAnimation) { setupCurrentFragmentCustomTransition( @@ -128,7 +128,7 @@ export function _setAndroidFragmentTransitions(animated: boolean, navigationTran curve: transition.getCurve(), }, currentEntry, - transition + transition, ); } } else if (name === 'default') { @@ -356,7 +356,10 @@ function getTransitionListener(entry: ExpandedEntry, transition: androidx.transi @Interfaces([(androidx).transition.Transition.TransitionListener]) class TransitionListenerImpl extends java.lang.Object implements androidx.transition.Transition.TransitionListener { public backEntry?: BackstackEntry; - constructor(public entry: ExpandedEntry, public transition: androidx.transition.Transition) { + constructor( + public entry: ExpandedEntry, + public transition: androidx.transition.Transition, + ) { super(); return global.__native(this); @@ -702,7 +705,7 @@ function transitionOrAnimationCompleted(entry: ExpandedEntry, backEntry: Backsta if (entries.size === 0) { // We have 0 or 1 entry per frameId in completedEntries // So there is no need to make it to Set like waitingQueue - const previousCompletedAnimationEntry = completedEntries.get(frameId); + const previousCompletedEntry = completedEntries.get(frameId); completedEntries.delete(frameId); waitingQueue.delete(frameId); @@ -716,8 +719,8 @@ function transitionOrAnimationCompleted(entry: ExpandedEntry, backEntry: Backsta const navigationContext = frame._executingContext || { navigationType: NavigationType.back, }; - let current = frame.isCurrent(entry) ? previousCompletedAnimationEntry : entry; - current = current || entry; + const current = frame.isCurrent(entry) && previousCompletedEntry ? previousCompletedEntry : entry; + // Will be null if Frame is shown modally... // transitionOrAnimationCompleted fires again (probably bug in android). if (current) { diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index d05542f5dc..067abb817d 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -41,7 +41,7 @@ export class FrameBase extends CustomLayoutView { private _animated: boolean; private _transition: NavigationTransition; private _backStack = new Array(); - _navigationQueue = new Array(); + private _navigationQueue = new Array(); public actionBarVisibility: 'auto' | 'never' | 'always'; public _currentEntry: BackstackEntry; @@ -300,7 +300,14 @@ export class FrameBase extends CustomLayoutView { this._backStack.pop(); } else if (!isReplace) { if (entry.entry.clearHistory) { - this._backStack.forEach((e) => this._removeEntry(e)); + this._backStack.forEach((e) => { + if (e !== entry) { + this._removeEntry(e); + } else { + // This case is extremely rare but can occur when fragment resumes + Trace.write(`Failed to dispose backstack entry ${entry}. This entry is the one frame is navigating to.`, Trace.categories.Navigation, Trace.messageType.warn); + } + }); this._backStack.length = 0; } else if (FrameBase._isEntryBackstackVisible(current)) { this._backStack.push(current); @@ -370,6 +377,16 @@ export class FrameBase extends CustomLayoutView { return entry; } + public getNavigationQueueContextByEntry(entry: BackstackEntry): NavigationContext { + for (const context of this._navigationQueue) { + if (context.entry === entry) { + return context; + } + } + + return null; + } + public navigationQueueIsEmpty(): boolean { return this._navigationQueue.length === 0; } @@ -429,16 +446,18 @@ export class FrameBase extends CustomLayoutView { @profile performGoBack(navigationContext: NavigationContext) { - let backstackEntry = navigationContext.entry; const backstack = this._backStack; - if (!backstackEntry) { - backstackEntry = backstack[backstack.length - 1]; + const backstackEntry = navigationContext.entry || backstack[backstack.length - 1]; + + if (backstackEntry) { navigationContext.entry = backstackEntry; - } - this._executingContext = navigationContext; - this._onNavigatingTo(backstackEntry, true); - this._goBackCore(backstackEntry); + this._executingContext = navigationContext; + this._onNavigatingTo(backstackEntry, true); + this._goBackCore(backstackEntry); + } else { + Trace.write('Frame.performGoBack: No backstack entry found to navigate back to', Trace.categories.Navigation, Trace.messageType.warn); + } } public _goBackCore(backstackEntry: BackstackEntry) { diff --git a/packages/core/ui/frame/index.d.ts b/packages/core/ui/frame/index.d.ts index de5e064850..d305ba6e45 100644 --- a/packages/core/ui/frame/index.d.ts +++ b/packages/core/ui/frame/index.d.ts @@ -200,6 +200,10 @@ export class Frame extends FrameBase { * @param navigationType */ setCurrent(entry: BackstackEntry, navigationType: NavigationType): void; + /** + * @private + */ + getNavigationQueueContextByEntry(entry: BackstackEntry): NavigationContext; /** * @private */ diff --git a/packages/core/ui/page/index.ios.ts b/packages/core/ui/page/index.ios.ts index 83a7c30b66..8313c5c18a 100644 --- a/packages/core/ui/page/index.ios.ts +++ b/packages/core/ui/page/index.ios.ts @@ -18,7 +18,7 @@ const DELEGATE = '_delegate'; const TRANSITION = '_transition'; const NON_ANIMATED_TRANSITION = 'non-animated'; -function isBackNavigationTo(page: Page, entry): boolean { +function isBackNavigationTo(page: Page, entry: BackstackEntry): boolean { const frame = page.frame; if (!frame) { return false; @@ -37,14 +37,8 @@ function isBackNavigationTo(page: Page, entry): boolean { return true; } - const navigationQueue = (frame)._navigationQueue; - for (let i = 0; i < navigationQueue.length; i++) { - if (navigationQueue[i].entry === entry) { - return navigationQueue[i].navigationType === NavigationType.back; - } - } - - return false; + const queueContext = frame.getNavigationQueueContextByEntry(entry); + return queueContext && queueContext.navigationType === NavigationType.back; } function isBackNavigationFrom(controller: UIViewControllerImpl, page: Page): boolean { @@ -121,7 +115,7 @@ class UIViewControllerImpl extends UIViewController { } const frame: Frame = this.navigationController ? (this.navigationController).owner : null; - const newEntry = this[ENTRY]; + const newEntry: BackstackEntry = this[ENTRY]; // Don't raise event if currentPage was showing modal page. if (!owner._presentedViewController && newEntry && (!frame || frame.currentPage !== owner)) { diff --git a/tools/notes/CodingConvention.md b/tools/notes/CodingConvention.md index 1f8ceebe46..b6c289c2e9 100644 --- a/tools/notes/CodingConvention.md +++ b/tools/notes/CodingConvention.md @@ -2,11 +2,11 @@ ## Tabs vs Spaces -Use 4 spaces indentation. +Use tab width 2 indentation. ## Line length -Try to limit your lines to 80 characters. +Try to limit your lines to 600 characters. ## Semicolons, statement Termination @@ -26,18 +26,18 @@ let x = 1 ## Quotes -Use double quotes for strings: +Use single quotes for strings: *Right:* ```TypeScript -let foo = "bar"; +let foo = 'bar'; ``` *Wrong:* ```TypeScript -let foo = 'bar'; +let foo = "bar"; ``` ## Braces @@ -48,7 +48,7 @@ Your opening braces go on the same line as the statement. ```TypeScript if (true) { - console.log("winning"); + console.log('winning'); } ``` @@ -57,7 +57,7 @@ if (true) { ```TypeScript if (true) { - console.log("losing"); + console.log('losing'); } ``` @@ -69,9 +69,9 @@ Follow the JavaScript convention of stacking `else/catch` clauses on the same li ```TypeScript if (i % 2 === 0) { - console.log("even"); + console.log('even'); } else { - console.log("odd"); + console.log('odd'); } ``` @@ -79,10 +79,10 @@ if (i % 2 === 0) { ```TypeScript if (i % 2 === 0) { - console.log("even"); + console.log('even'); } else { - console.log("odd"); + console.log('odd'); } ``` @@ -96,7 +96,7 @@ Declare variables with `let` instead of `var`. Use `const` when possible. const button = new Button(); for (let i = 0; i < items.length; i++) { - // do something + // do something } ``` @@ -106,7 +106,7 @@ for (let i = 0; i < items.length; i++) { var button = new Button(); for (var i = 0; i < items.length; i++) { - // do something + // do something } ``` @@ -120,13 +120,13 @@ uncommon abbreviations should generally be avoided unless it is something well k *Right:* ```TypeScript -let adminUser = db.query("SELECT * FROM users ..."); +let adminUser = db.query('SELECT * FROM users ...'); ``` *Wrong:* ```TypeScript -let admin_user = db.query("SELECT * FROM users ..."); +let admin_user = db.query('SELECT * FROM users ...'); ``` [camelcase]: https://en.wikipedia.org/wiki/camelCase#Variations_and_synonyms @@ -139,7 +139,7 @@ Type names should be capitalized using [upper camel case][camelcase]. ```TypeScript class UserAccount() { - this.field = "a"; + this.field = 'a'; } ``` @@ -147,7 +147,7 @@ class UserAccount() { ```TypeScript class userAccount() { - this.field = "a"; + this.field = 'a'; } ``` @@ -176,10 +176,10 @@ keys when your interpreter complains: *Right:* ```TypeScript -let a = ["hello", "world"]; +let a = ['hello', 'world']; let b = { - good: "code", - "is generally": "pretty", + good: 'code', + 'is generally': 'pretty', }; ``` @@ -187,23 +187,23 @@ let b = { ```TypeScript let a = [ - "hello", "world" + 'hello', 'world' ]; -let b = {"good": "code" - , is generally: "pretty" +let b = {'good': 'code' + , is generally: 'pretty' }; ``` ## Equality operator -Use the [strict comparison operators][comparisonoperators]. The triple equality operator helps to maintain data type integrity throughout the code. +Use the [strict comparison operators][comparisonoperators] when needed. The triple equality operator helps to maintain data type integrity throughout the code. *Right:* ```TypeScript let a = 0; -if (a === "") { - console.log("winning"); +if (a === '') { + console.log('winning'); } ``` @@ -212,8 +212,8 @@ if (a === "") { ```TypeScript let a = 0; -if (a == "") { - console.log("losing"); +if (a == '') { + console.log('losing'); } ``` @@ -227,8 +227,8 @@ Try to avoid short-hand operators except in very simple scenarios. ```TypeScript let default = x || 50; -let extraLarge = "xxl"; -let small = "s" +let extraLarge = 'xxl'; +let small = 's' let big = (x > 10) ? extraLarge : small; ``` @@ -247,7 +247,7 @@ Always use curly braces even in the cases of one line conditional operations. ```TypeScript if (a) { - return "winning"; + return 'winning'; } ``` @@ -257,9 +257,9 @@ if (a) { ```TypeScript if (a) - return "winning"; + return 'winning'; -if (a) return "winning"; +if (a) return 'winning'; ``` ## Boolean comparisons @@ -271,11 +271,11 @@ if (a) return "winning"; ```TypeScript if (condition) { - console.log("winning"); + console.log('winning'); } if (!condition) { - console.log("winning"); + console.log('winning'); } ``` @@ -285,15 +285,15 @@ if (!condition) { ```TypeScript if (condition === true) { - console.log("losing"); + console.log('losing'); } if (condition !== true) { - console.log("losing"); + console.log('losing'); } if (condition !== false) { - console.log("losing"); + console.log('losing'); } ``` @@ -306,7 +306,7 @@ Do not use the **Yoda Conditions** when writing boolean expressions: ```TypeScript let num; if (num >= 0) { - console.log("winning"); + console.log('winning'); } ``` @@ -315,14 +315,14 @@ if (num >= 0) { ```TypeScript let num; if (0 <= num) { - console.log("losing"); + console.log('losing'); } ``` **NOTE** It is OK to use constants on the left when comparing for a range. ```TypeScript if (0 <= num && num <= 100) { - console.log("winning"); + console.log('winning'); } ``` @@ -341,22 +341,22 @@ as possible. In certain routines, once you know the answer, you want to return i ```TypeScript function getSomething(val) { - if (val < 0) { - return false; - } - - if (val > 100) { - return false; - } - - let res1 = doOne(); - let res2 = doTwo(); - let options = { - a: 1, - b: 2 - }; - let result = doThree(res1, res2, options); - return result; + if (val < 0) { + return false; + } + + if (val > 100) { + return false; + } + + let res1 = doOne(); + let res2 = doTwo(); + let options = { + a: 1, + b: 2 + }; + let result = doThree(res1, res2, options); + return result; } ``` @@ -364,24 +364,24 @@ function getSomething(val) { ```TypeScript function getSomething(val) { - if (val >= 0) { - if (val < 100) { - let res1 = doOne(); - let res2 = doTwo(); - let options = { - a: 1, - b: 2 - }; - let result = doThree(res1, res2, options); - return result; - } - else { - return false; - } + if (val >= 0) { + if (val < 100) { + let res1 = doOne(); + let res2 = doTwo(); + let options = { + a: 1, + b: 2 + }; + let result = doThree(res1, res2, options); + return result; } else { - return false; + return false; } + } + else { + return false; + } } ``` @@ -392,10 +392,10 @@ Use arrow functions over anonymous function expressions. Typescript will take ca *Right:* ```TypeScript -req.on("end", () => { - exp1(); - exp2(); - this.doSomething(); +req.on('end', () => { + exp1(); + exp2(); + this.doSomething(); }); ``` @@ -403,10 +403,10 @@ req.on("end", () => { ```TypeScript let that = this; -req.on("end", function () { - exp1(); - exp2(); - that.doSomething(); +req.on('end', function () { + exp1(); + exp2(); + that.doSomething(); }); ``` @@ -449,16 +449,16 @@ When you **need** to keep a reference to **this** use **that** as the name of th *Right:* ```TypeScript let that = this; -doSomething(function(){ - that.doNothing(); +doSomething(function() { + that.doNothing(); }); ``` *Wrong:* ```TypeScript let me = this; -doSomething(function(){ - me.doNothing(); +doSomething(function() { + me.doNothing(); }); ``` @@ -468,34 +468,34 @@ Although there is the **private** keyword in TypeScript, it is only a syntax sug *Right:* ```TypeScript class Foo { - private _myBoolean: boolean; - - public publicAPIMethod() { - } - - public _frameworkMethod() { - // this method is for internal use only - } - - private _doSomething() { - } + private _myBoolean: boolean; + + public publicAPIMethod() { + } + + public _frameworkMethod() { + // this method is for internal use only + } + + private _doSomething() { + } } ``` *Wrong:* ```TypeScript class Foo { - private myBoolean: boolean; - - public _publicAPIMethod() { - } - - public frameworkMethod() { - // this method is for internal use only - } - - private doSomething() { - } + private myBoolean: boolean; + + public _publicAPIMethod() { + } + + public frameworkMethod() { + // this method is for internal use only + } + + private doSomething() { + } } ``` @@ -509,14 +509,14 @@ export declare function concat(...categories: string[]): string; // implementation export function concat(): string { - let i; - let result: string; - // use the arguments object to iterate the parameters - for (i = 0; i < arguments.length; i++) { - // do something - } + let i; + let result: string; + // use the arguments object to iterate the parameters + for (i = 0; i < arguments.length; i++) { + // do something + } - return result; + return result; } ``` @@ -527,14 +527,14 @@ export declare function concat(...categories: string[]): string; // implementation export function concat(...categories: string[]): string { - let i; - let result: string; - // use the arguments object to iterate the parameters - for (i = 0; i < categories.length; i++) { - // do something - } + let i; + let result: string; + // use the arguments object to iterate the parameters + for (i = 0; i < categories.length; i++) { + // do something + } - return result; + return result; } ``` @@ -544,6 +544,6 @@ Name your test function with `test_` so that our test runner can find them and a *Right:* ```TypeScript export function test_goToVisualState_NoState_ShouldResetStyledProperties() { - // Test code here. + // Test code here. } ```