Skip to content

Commit ef6a24a

Browse files
author
vakrilov
committed
FIX: isBackNavigation worng when navigation away form page that is not backstack visible
1 parent fa48f4e commit ef6a24a

File tree

3 files changed

+152
-57
lines changed

3 files changed

+152
-57
lines changed

tests/app/navigation/navigation-tests.ts

Lines changed: 142 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import {Color} from "color";
55
import helper = require("../ui/helper");
66

77
// Creates a random colorful page full of meaningless stuff.
8-
var id = 0;
9-
var pageFactory = function (): Page {
10-
var page = new Page();
8+
let id = 0;
9+
let pageFactory = function (): Page {
10+
const page = new Page();
1111
page.actionBarHidden = true;
1212
page.id = `NavTestPage${id++}`;
1313
page.style.backgroundColor = new Color(255, Math.round(Math.random() * 255), Math.round(Math.random() * 255), Math.round(Math.random() * 255));
@@ -22,17 +22,28 @@ function androidGC() {
2222
}
2323
}
2424

25+
function attachEventListeners(page: Page, events: Array<string>) {
26+
let argsToString = (args: NavigatedData) => {
27+
return `${(<Page>args.object).id} ${args.eventName} ${(args.isBackNavigation ? "back" : "forward")}`;
28+
};
29+
30+
page.on(Page.navigatingFromEvent, (args: NavigatedData) => { events.push(argsToString(args)); });
31+
page.on(Page.navigatedFromEvent, (args: NavigatedData) => { events.push(argsToString(args)); });
32+
page.on(Page.navigatingToEvent, (args: NavigatedData) => { events.push(argsToString(args)); });
33+
page.on(Page.navigatedToEvent, (args: NavigatedData) => { events.push(argsToString(args)); });
34+
}
35+
2536
function _test_backstackVisible(transition?: NavigationTransition) {
2637
let topmost = topmostFrame();
2738
let mainTestPage = topmost.currentPage;
2839
helper.navigateWithEntry({ create: pageFactory, transition: transition, animated: true });
29-
40+
3041
// page1 should not be added to the backstack
3142
let page0 = topmost.currentPage;
3243
helper.navigateWithEntry({ create: pageFactory, backstackVisible: false, transition: transition, animated: true });
3344

3445
helper.navigateWithEntry({ create: pageFactory, transition: transition, animated: true });
35-
46+
3647
helper.goBack();
3748

3849
// From page2 we have to go directly to page0, skipping page1.
@@ -42,39 +53,42 @@ function _test_backstackVisible(transition?: NavigationTransition) {
4253
TKUnit.assertEqual(topmost.currentPage, mainTestPage, "We should be on the main test page at the end of the test.");
4354
}
4455

45-
export var test_backstackVisible = function () {
56+
export function test_backstackVisible() {
4657
androidGC();
4758
_test_backstackVisible();
4859
}
4960

50-
export var test_backstackVisible_WithTransition = function () {
61+
export function test_backstackVisible_WithTransition() {
5162
androidGC();
5263
_test_backstackVisible({ name: "fade" });
5364
}
5465

5566
function _test_backToEntry(transition?: NavigationTransition) {
5667
let topmost = topmostFrame();
5768
let page = (tag) => () => {
58-
var p = new Page();
69+
const p = new Page();
5970
p.actionBarHidden = true;
6071
p.id = `NavTestPage${id++}`;
6172
p["tag"] = tag;
6273
return p;
63-
}
74+
};
75+
6476
let mainTestPage = topmost.currentPage;
6577
let waitFunc = tag => TKUnit.waitUntilReady(() => topmost.currentPage["tag"] === tag, 1);
6678
let navigate = tag => {
6779
topmost.navigate({ create: page(tag), transition: transition, animated: true });
6880
waitFunc(tag);
69-
70-
}
81+
82+
};
83+
7184
let back = pages => {
7285
topmost.goBack(topmost.backStack[topmost.backStack.length - pages]);
73-
}
86+
};
87+
7488
let currentPageMustBe = tag => {
7589
waitFunc(tag); // TODO: Add a timeout...
7690
TKUnit.assert(topmost.currentPage["tag"] === tag, "Expected current page to be " + tag + " it was " + topmost.currentPage["tag"] + " instead.");
77-
}
91+
};
7892

7993
navigate("page1");
8094
navigate("page2");
@@ -99,14 +113,14 @@ function _test_backToEntry(transition?: NavigationTransition) {
99113
TKUnit.assertEqual(topmost.currentPage, mainTestPage, "We should be on the main test page at the end of the test.");
100114
}
101115

102-
export var test_backToEntry = function () {
116+
export function test_backToEntry() {
103117
androidGC();
104118
_test_backToEntry();
105119
}
106120

107-
export var test_backToEntry_WithTransition = function () {
121+
export function test_backToEntry_WithTransition() {
108122
androidGC();
109-
_test_backToEntry({name: "flip"});
123+
_test_backToEntry({ name: "flip" });
110124
}
111125

112126
function _test_ClearHistory(transition?: NavigationTransition) {
@@ -141,17 +155,17 @@ function _test_ClearHistory(transition?: NavigationTransition) {
141155
}
142156

143157
// Clearing the history messes up the tests app.
144-
export var test_ClearHistory = function () {
158+
export function test_ClearHistory() {
145159
androidGC();
146160
_test_ClearHistory();
147161
}
148162

149-
export var test_ClearHistory_WithTransition = function () {
163+
export function test_ClearHistory_WithTransition() {
150164
androidGC();
151165
_test_ClearHistory({ name: "fade" });
152166
}
153167

154-
export var test_ClearHistory_WithTransition_WithCachePagesOnNavigate = function () {
168+
export function test_ClearHistory_WithTransition_WithCachePagesOnNavigate() {
155169
androidGC();
156170
let topmost = topmostFrame();
157171
if (!topmost.android) {
@@ -165,7 +179,7 @@ export var test_ClearHistory_WithTransition_WithCachePagesOnNavigate = function
165179
}
166180

167181
// Test case for https://github.com/NativeScript/NativeScript/issues/1948
168-
export var test_ClearHistoryWithTransitionDoesNotBreakNavigation = function () {
182+
export function test_ClearHistoryWithTransitionDoesNotBreakNavigation() {
169183
androidGC();
170184
let topmost = topmostFrame();
171185
let mainTestPage = topmost.currentPage;
@@ -175,28 +189,29 @@ export var test_ClearHistoryWithTransitionDoesNotBreakNavigation = function () {
175189

176190
// Go to details-page
177191
helper.navigateWithEntry({ create: pageFactory, clearHistory: false, animated: true });
178-
192+
179193
// Go back to main-page with clearHistory
180194
topmost.transition = { name: "fade" };
181195
helper.navigateWithEntry({ create: mainPageFactory, clearHistory: true, animated: true });
182-
196+
183197
// Go to details-page AGAIN
184198
helper.navigateWithEntry({ create: pageFactory, clearHistory: false, animated: true });
185-
199+
186200
// Go back to main-page with clearHistory
187201
topmost.transition = { name: "fade" };
188202
helper.navigateWithEntry({ create: mainPageFactory, clearHistory: true, animated: true });
189-
203+
190204
// Clean up
191205
topmost.transition = undefined;
192206

193207
TKUnit.assertEqual(topmost.currentPage, mainTestPage, "We should be on the main test page at the end of the test.");
194208
TKUnit.assertEqual(topmost.backStack.length, 0, "Back stack should be empty at the end of the test.");
195209
}
196210

197-
export var test_ClearHistoryWithTransitionDoesNotBreakNavigation_WithLocalTransition = function () {
211+
export function test_ClearHistoryWithTransitionDoesNotBreakNavigation_WithLocalTransition() {
198212
androidGC();
199-
let topmost = topmostFrame();
213+
const topmost = topmostFrame();
214+
200215
let originalCachePagesOnNavigate: boolean;
201216
if (topmost.android) {
202217
originalCachePagesOnNavigate = topmost.android.cachePagesOnNavigate;
@@ -210,16 +225,16 @@ export var test_ClearHistoryWithTransitionDoesNotBreakNavigation_WithLocalTransi
210225

211226
// Go to 1st page
212227
helper.navigateWithEntry({ create: pageFactory, clearHistory: false, transition: { name: "fade" }, animated: true });
213-
228+
214229
// Go to 2nd page
215230
helper.navigateWithEntry({ create: pageFactory, clearHistory: false, transition: { name: "fade" }, animated: true });
216-
231+
217232
// Go to 3rd page with clearHistory
218233
helper.navigateWithEntry({ create: pageFactory, clearHistory: true, transition: { name: "fade" }, animated: true });
219-
234+
220235
// Go back to main
221236
helper.navigateWithEntry({ create: mainPageFactory, clearHistory: true, transition: { name: "fade" }, animated: true });
222-
237+
223238
if (topmost.android) {
224239
topmostFrame().android.cachePagesOnNavigate = originalCachePagesOnNavigate;
225240
}
@@ -229,39 +244,30 @@ export var test_ClearHistoryWithTransitionDoesNotBreakNavigation_WithLocalTransi
229244
}
230245

231246
function _test_NavigationEvents(transition?: NavigationTransition) {
232-
let topmost = topmostFrame();
233-
let argsToString = (args: NavigatedData) => {
234-
return `${(<Page>args.object).id} ${args.eventName} ${(args.isBackNavigation ? "back" : "forward") }`
235-
};
247+
const topmost = topmostFrame();
248+
const mainTestPage = topmost.currentPage;
249+
const originalMainPageId = mainTestPage.id;
236250

237-
let mainTestPage = topmost.currentPage;
238-
let originalMainPageId = mainTestPage.id;
239251
mainTestPage.id = "main-page";
240252
let actualMainPageEvents = new Array<string>();
241-
mainTestPage.on(Page.navigatingFromEvent, (args: NavigatedData) => { actualMainPageEvents.push(argsToString(args)); });
242-
mainTestPage.on(Page.navigatedFromEvent, (args: NavigatedData) => { actualMainPageEvents.push(argsToString(args)); });
243-
mainTestPage.on(Page.navigatingToEvent, (args: NavigatedData) => { actualMainPageEvents.push(argsToString(args)); });
244-
mainTestPage.on(Page.navigatedToEvent, (args: NavigatedData) => { actualMainPageEvents.push(argsToString(args)); });
253+
attachEventListeners(mainTestPage, actualMainPageEvents);
245254

246255
let actualSecondPageEvents = new Array<string>();
247256
let secondPageFactory = function (): Page {
248-
var secondPage = new Page();
249-
secondPage.actionBarHidden = true;
250-
secondPage.id = "second-page"
251-
secondPage.on(Page.navigatingFromEvent, (args: NavigatedData) => { actualSecondPageEvents.push(argsToString(args)); });
252-
secondPage.on(Page.navigatedFromEvent, (args: NavigatedData) => { actualSecondPageEvents.push(argsToString(args)); });
253-
secondPage.on(Page.navigatingToEvent, (args: NavigatedData) => { actualSecondPageEvents.push(argsToString(args)); });
254-
secondPage.on(Page.navigatedToEvent, (args: NavigatedData) => { actualSecondPageEvents.push(argsToString(args)); });
257+
const secondPage = new Page();
258+
secondPage.actionBarHidden = true;
259+
secondPage.id = "second-page";
260+
attachEventListeners(secondPage, actualSecondPageEvents);
255261
secondPage.style.backgroundColor = new Color(255, Math.round(Math.random() * 255), Math.round(Math.random() * 255), Math.round(Math.random() * 255));
256262
return secondPage;
257263
};
258264

259265
// Go to other page
260266
helper.navigateWithEntry({ create: secondPageFactory, transition: transition, animated: true });
261-
267+
262268
// Go back to main
263269
helper.goBack();
264-
270+
265271
mainTestPage.id = originalMainPageId;
266272

267273
let expectedMainPageEvents = [
@@ -283,12 +289,97 @@ function _test_NavigationEvents(transition?: NavigationTransition) {
283289
TKUnit.assertEqual(topmost.currentPage, mainTestPage, "We should be on the main test page at the end of the test.");
284290
}
285291

286-
export var test_NavigationEvents = function () {
292+
export function test_NavigationEvents() {
287293
androidGC();
288294
_test_NavigationEvents();
289295
}
290296

291-
export var test_NavigationEvents_WithTransition = function () {
297+
export function test_NavigationEvents_WithTransition() {
292298
androidGC();
293299
_test_NavigationEvents({ name: "fade" });
300+
}
301+
302+
function _test_NavigationEvents_WithBackstackVisibile_False_Forward_Back(transition?: NavigationTransition) {
303+
const topmost = topmostFrame();
304+
const mainTestPage = topmost.currentPage;
305+
306+
let actualSecondPageEvents = new Array<string>();
307+
let secondPageFactory = function (): Page {
308+
const secondPage = new Page();
309+
secondPage.actionBarHidden = true;
310+
secondPage.id = "second-page";
311+
attachEventListeners(secondPage, actualSecondPageEvents);
312+
secondPage.style.backgroundColor = new Color(255, Math.round(Math.random() * 255), Math.round(Math.random() * 255), Math.round(Math.random() * 255));
313+
return secondPage;
314+
};
315+
316+
// Go to other page
317+
helper.navigateWithEntry({ create: secondPageFactory, transition: transition, animated: true, backstackVisible: false });
318+
319+
// Go back to main
320+
helper.goBack();
321+
322+
let expectedSecondPageEvents = [
323+
"second-page navigatingTo forward",
324+
"second-page navigatedTo forward",
325+
"second-page navigatingFrom back",
326+
"second-page navigatedFrom back"
327+
];
328+
TKUnit.arrayAssert(actualSecondPageEvents, expectedSecondPageEvents, "Actual second-page events are different from expected.");
329+
330+
TKUnit.assertEqual(topmost.currentPage, mainTestPage, "We should be on the main test page at the end of the test.");
331+
}
332+
333+
export function test_NavigationEvents_WithBackstackVisibile_False_Forward_Back() {
334+
androidGC();
335+
_test_NavigationEvents_WithBackstackVisibile_False_Forward_Back();
336+
}
337+
338+
export function test_NavigationEvents_WithBackstackVisibile_False_Forward_Back_WithTransition() {
339+
androidGC();
340+
_test_NavigationEvents_WithBackstackVisibile_False_Forward_Back({ name: "fade" });
341+
}
342+
343+
function _test_NavigationEvents_WithBackstackVisibile_False_Forward_Forward(transition?: NavigationTransition) {
344+
const topmost = topmostFrame();
345+
const mainTestPage = topmost.currentPage;
346+
347+
let actualSecondPageEvents = new Array<string>();
348+
let secondPageFactory = function (): Page {
349+
const secondPage = new Page();
350+
secondPage.actionBarHidden = true;
351+
secondPage.id = "second-page";
352+
attachEventListeners(secondPage, actualSecondPageEvents);
353+
secondPage.style.backgroundColor = new Color(255, Math.round(Math.random() * 255), Math.round(Math.random() * 255), Math.round(Math.random() * 255));
354+
return secondPage;
355+
};
356+
357+
// Go to other page
358+
helper.navigateWithEntry({ create: secondPageFactory, transition: transition, animated: true, backstackVisible: false });
359+
360+
// Go forward to third page
361+
helper.navigateWithEntry({ create: pageFactory, transition: transition, animated: true });
362+
363+
// Go back to main
364+
helper.goBack();
365+
366+
let expectedSecondPageEvents = [
367+
"second-page navigatingTo forward",
368+
"second-page navigatedTo forward",
369+
"second-page navigatingFrom forward",
370+
"second-page navigatedFrom forward"
371+
];
372+
TKUnit.arrayAssert(actualSecondPageEvents, expectedSecondPageEvents, "Actual second-page events are different from expected.");
373+
374+
TKUnit.assertEqual(topmost.currentPage, mainTestPage, "We should be on the main test page at the end of the test.");
375+
}
376+
377+
export function test_NavigationEvents_WithBackstackVisibile_False_Forward_Forward() {
378+
androidGC();
379+
_test_NavigationEvents_WithBackstackVisibile_False_Forward_Forward();
380+
}
381+
382+
export function test_NavigationEvents_WithBackstackVisibile_False_Forward_Forward_WithTransition() {
383+
androidGC();
384+
_test_NavigationEvents_WithBackstackVisibile_False_Forward_Forward({ name: "fade" });
294385
}

tns-core-modules/ui/frame/frame.ios.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,8 @@ export class Frame extends frameCommon.Frame {
145145
viewController.navigationItem.hidesBackButton = this.backStack.length === 0;
146146

147147
// swap the top entry with the new one
148+
var skippedNavController = newControllers.lastObject;
149+
skippedNavController.isBackstackSkipped = true;
148150
newControllers.removeLastObject();
149151
newControllers.addObject(viewController);
150152

0 commit comments

Comments
 (0)