Navigation: When going forward, grab transition from current entry

Our history entries contain exactly one transition: The one used to arrive at the entry. In addition, the index of the current entry and that of the previous entry is updated before the value for the transition is computed. So, it makes sense, when going backwards, to read the transition value from the previous entry, because that value stores the transition that was used to get there from where history is currently at. However, when going forward, it makes no sense to read the transition value from the previous entry, because that one stores the transition that was used to reach it, not the transition that must be used to reach the next entry. Closes jquery-archivegh-7485 Fixes jquery-archivegh-1867 Fixes jquery-archivegh-4951
agcolom · Nov 26, 2014 · 596ed33 · 596ed33
1 parent ff61174
commit 596ed33
Show file tree

Hide file tree

Showing 6 changed files with 259 additions and 15 deletions.
diff --git a/js/widgets/pagecontainer.js b/js/widgets/pagecontainer.js
@@ -166,9 +166,8 @@ define( [
 			return $.mobile.navigate.history;
 		},
 
-		// TODO use _getHistory
 		_getActiveHistory: function() {
-			return $.mobile.navigate.history.getActive();
+			return this._getHistory().getActive();
 		},
 
 		// TODO the document base should be determined at creation
@@ -244,6 +243,13 @@ define( [
 			return to || this._getInitialContent();
 		},
 
+		_transitionFromHistory: function( direction, defaultTransition ) {
+			var history = this._getHistory(),
+				entry = ( direction === "back" ? history.getLast() : history.getActive() );
+
+			return ( entry && entry.transition ) || defaultTransition;
+		},
+
 		_handleDialog: function( changePageOptions, data ) {
 			var to, active, activeContent = this.getActivePage();
 
@@ -270,7 +276,9 @@ define( [
 				// as most of this is lost by the domCache cleaning
 				$.extend( changePageOptions, {
 					role: active.role,
-					transition: active.transition,
+					transition: this._transitionFromHistory(
+						data.direction,
+						changePageOptions.transition ),
 					reverse: data.direction === "back"
 				});
 			}
@@ -285,7 +293,8 @@ define( [
 
 				// transition is false if it's the first page, undefined
 				// otherwise (and may be overridden by default)
-				transition = history.stack.length === 0 ? "none" : undefined,
+				transition = history.stack.length === 0 ? "none" :
+					this._transitionFromHistory( data.direction ),
 
 				// default options for the changPage calls made after examining
 				// the current state of the page and the hash, NOTE that the
@@ -297,7 +306,7 @@ define( [
 				};
 
 			$.extend( changePageOptions, data, {
-				transition: ( history.getLast() || {} ).transition || transition
+				transition: transition
 			});
 
 			// TODO move to _handleDestination ?

diff --git a/tests/integration/pagecontainer/transition-choice-dialog-tests.html b/tests/integration/pagecontainer/transition-choice-dialog-tests.html
@@ -0,0 +1,44 @@
+<!doctype html>
+<html lang="en">
+<head>
+	<meta charset="utf-8">
+	<meta name="viewport" content="width=device-width, initial-scale=1">
+	<title>jQuery Mobile Popup Test Suite</title>
+
+	<script src="../../../external/requirejs/require.js"></script>
+	<script src="../../../js/requirejs.config.js"></script>
+	<script src="../../../js/jquery.tag.inserter.js"></script>
+	<script src="../../jquery.setNameSpace.js"></script>
+	<script src="../../../tests/jquery.testHelper.js"></script>
+
+	<link rel="stylesheet"  href="../../../css/themes/default/jquery.mobile.css"/>
+	<link rel="stylesheet" href="../../../external/qunit/qunit.css"/>
+	<link rel="stylesheet" href="../../jqm-tests.css"/>
+	<script src="../../../external/qunit/qunit.js"></script>
+	<script>
+		$.testHelper.asyncLoad([
+			[ "widgets/dialog" ],
+			[ "init" ],
+			[
+				"transition_choice_dialog_core.js"
+			]
+		]);
+	</script>
+
+	<script src="../../swarminject.js"></script>
+</head>
+<body>
+	<div id="qunit"></div>
+
+	<div data-nstest-role="page" id="a">
+		<a id="go-to-b" data-nstest-transition="flip" data-nstest-rel="dialog" href="#b">Go to b</a>
+	</div>
+
+	<div data-nstest-role="dialog" id="b">
+		<a id="go-to-c" data-nstest-transition="slide" data-nstest-rel="dialog" href="#c">Go to c</a>
+	</div>
+
+	<div data-nstest-role="dialog" id="c">
+	</div>
+</body>
+</html>
diff --git a/tests/integration/pagecontainer/transition-choice-tests.html b/tests/integration/pagecontainer/transition-choice-tests.html
@@ -0,0 +1,43 @@
+<!doctype html>
+<html lang="en">
+<head>
+	<meta charset="utf-8">
+	<meta name="viewport" content="width=device-width, initial-scale=1">
+	<title>jQuery Mobile Popup Test Suite</title>
+
+	<script src="../../../external/requirejs/require.js"></script>
+	<script src="../../../js/requirejs.config.js"></script>
+	<script src="../../../js/jquery.tag.inserter.js"></script>
+	<script src="../../jquery.setNameSpace.js"></script>
+	<script src="../../../tests/jquery.testHelper.js"></script>
+
+	<link rel="stylesheet"  href="../../../css/themes/default/jquery.mobile.css"/>
+	<link rel="stylesheet" href="../../../external/qunit/qunit.css"/>
+	<link rel="stylesheet" href="../../jqm-tests.css"/>
+	<script src="../../../external/qunit/qunit.js"></script>
+	<script>
+		$.testHelper.asyncLoad([
+			[ "init" ],
+			[
+				"transition_choice_core.js"
+			]
+		]);
+	</script>
+
+	<script src="../../swarminject.js"></script>
+</head>
+<body>
+	<div id="qunit"></div>
+
+	<div data-nstest-role="page" id="a">
+		<a id="go-to-b" data-nstest-transition="flip" href="#b">Go to b</a>
+	</div>
+
+	<div data-nstest-role="page" id="b">
+		<a id="go-to-c" data-nstest-transition="slide" href="#c">Go to c</a>
+	</div>
+
+	<div data-nstest-role="page" id="c">
+	</div>
+</body>
+</html>
diff --git a/tests/integration/pagecontainer/transition_choice_core.js b/tests/integration/pagecontainer/transition_choice_core.js
@@ -0,0 +1,71 @@
+( function() {
+
+var origChange, callSequence;
+
+module( "Pagecontainer transition choice", {
+	setup: function() {
+		callSequence = [];
+		origChange = $.mobile.pagecontainer.prototype.change;
+		$.mobile.pagecontainer.prototype.change = function( url, options ) {
+			callSequence.push({
+				transition: options.transition,
+				reverse: !!( options.reverse )
+			});
+			return origChange.apply( this, arguments );
+		};
+	},
+	teardown: function() {
+		$.mobile.pagecontainer.prototype.change = origChange;
+	}
+});
+
+asyncTest( "Pagecontainer chooses correct transition", function() {
+	debugger;
+
+	var pageContainer = $( ":mobile-pagecontainer" );
+
+	$.testHelper.pageSequence([
+		function() {
+			$( "#go-to-b" ).click();
+		},
+		function() {
+			$( "#go-to-c" ).click();
+		},
+		function() {
+			pageContainer.pagecontainer( "back" );
+		},
+		function() {
+			pageContainer.pagecontainer( "forward" );
+		},
+		function() {
+			pageContainer.pagecontainer( "back" );
+		},
+		function() {
+			pageContainer.pagecontainer( "back" );
+		},
+		function() {
+			pageContainer.pagecontainer( "forward" );
+		},
+		function() {
+			pageContainer.pagecontainer( "back" );
+		},
+		function() {
+			deepEqual( callSequence,
+				[
+					{ transition: "flip", reverse: false },
+					{ transition: "slide", reverse: false },
+					{ transition: "slide", reverse: true },
+					{ transition: "slide", reverse: false },
+					{ transition: "slide", reverse: true },
+					{ transition: "flip", reverse: true },
+					{ transition: "flip", reverse: false },
+					{ transition: "flip", reverse: true }
+				],
+				"call sequence has resulted in the correct transitions" );
+
+			start();
+		}
+	]);
+});
+
+})();
diff --git a/tests/integration/pagecontainer/transition_choice_dialog_core.js b/tests/integration/pagecontainer/transition_choice_dialog_core.js
@@ -0,0 +1,55 @@
+( function() {
+
+var origChange, callSequence;
+
+module( "Pagecontainer transition choice", {
+	setup: function() {
+		callSequence = [];
+		origChange = $.mobile.pagecontainer.prototype.change;
+		$.mobile.pagecontainer.prototype.change = function( url, options ) {
+			callSequence.push({
+				transition: options.transition,
+				reverse: !!( options.reverse )
+			});
+			return origChange.apply( this, arguments );
+		};
+	},
+	teardown: function() {
+		$.mobile.pagecontainer.prototype.change = origChange;
+	}
+});
+
+asyncTest( "Pagecontainer chooses correct transition", function() {
+	debugger;
+
+	var pageContainer = $( ":mobile-pagecontainer" );
+
+	$.testHelper.pageSequence([
+		function() {
+			$( "#go-to-b" ).click();
+		},
+		function() {
+			$( "#go-to-c" ).click();
+		},
+		function() {
+			pageContainer.pagecontainer( "back" );
+		},
+		function() {
+			pageContainer.pagecontainer( "back" );
+		},
+		function() {
+			deepEqual( callSequence,
+				[
+					{ transition: "flip", reverse: false },
+					{ transition: "slide", reverse: false },
+					{ transition: "slide", reverse: true },
+					{ transition: "flip", reverse: true }
+				],
+				"call sequence has resulted in the correct transitions" );
+
+			start();
+		}
+	]);
+});
+
+})();
diff --git a/tests/unit/content/content_core.js b/tests/unit/content/content_core.js
@@ -123,23 +123,45 @@
 			return $( "<div>", {"class": "ui-dialog"} );
 		};
 
-		proto._getActiveHistory = function() {
+		proto._getHistory = function() {
 			return {
-				role: "foo",
-				transition: "bar"
+				length: 3,
+				activeIndex: 1,
+				lastIndex: 2,
+				stack: [
+					{
+						role: "page",
+						transition: "none"
+					},
+					{
+						role: "foo",
+						transition: "flip"
+					},
+					{
+						role: "page",
+						transition: "bar"
+					}
+				],
+				getLast: function() {
+					return this.stack[ this.lastIndex ];
+				},
+				getActive: function() {
+					return this.stack[ this.activeIndex ];
+				}
 			};
 		};
 
-		equal( opts.role, undefined );
-		equal( opts.transition, undefined );
-		equal( opts.reverse, undefined );
+		deepEqual( opts.role, undefined, "Initially, role is undefined" );
+		deepEqual( opts.transition, undefined, "Initially, transition is undefined" );
+		deepEqual( opts.reverse, undefined, "Initially, reverse is undefined" );
 
 		// the pageUrl is returned for use as the target url when the active content is a dialog
-		equal( proto._handleDialog( opts, {direction: "back", pageUrl: "baz" } ), "baz" );
+		deepEqual( proto._handleDialog( opts, { direction: "back", pageUrl: "baz" } ), "baz",
+			"pageUrl is returned" );
 
-		equal( opts.role, "foo" );
-		equal( opts.transition, "bar" );
-		equal( opts.reverse, true );
+		deepEqual( opts.role, "foo", "Afterwards, role is 'foo'" );
+		deepEqual( opts.transition, "bar", "Afterwards, transition is 'bar'" );
+		deepEqual( opts.reverse, true, "Afterwards, reverse is true" );
 	});
 
 	var base = "http://example.com/";