From ea7d1d2fce0bb3f9e61e642653e2cef7fc4e9806 Mon Sep 17 00:00:00 2001 From: Isaac Gierard Date: Tue, 18 Nov 2014 14:45:50 -0800 Subject: [PATCH 1/3] prevent scrollTo from being converted from false to 0 In the loadUrl function there is an explicit check for false as a means to disable the scrollTo behavior however if the scrollTo option is passed to the constructor as false the gaud statement was converting false to 0. --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index 94bcc09..be22d7c 100644 --- a/index.js +++ b/index.js @@ -26,7 +26,7 @@ var Pjax = function(options) { ga("send", "pageview", {page: options.url, title: options.title}) } } - this.options.scrollTo = this.options.scrollTo || 0 + this.options.scrollTo = (typeof this.options.scrollTo === 'undefined') ? 0 : this.options.scrollTo; this.options.debug = this.options.debug || false this.maxUid = this.lastUid = newUid() From d4ba34e5edc4dcb2edd1f9db272c6dd8c667c029 Mon Sep 17 00:00:00 2001 From: Isaac Gierard Date: Thu, 20 Nov 2014 14:39:48 -0800 Subject: [PATCH 2/3] moved options parsing into separate file and created test moved the options paring system into it's own function so that it may be testable --- index.js | 39 ++----------------- lib/proto/parse-options.js | 35 +++++++++++++++++ tests/lib/proto/parse-options.js | 66 ++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 36 deletions(-) create mode 100644 lib/proto/parse-options.js create mode 100644 tests/lib/proto/parse-options.js diff --git a/index.js b/index.js index be22d7c..44a5ed3 100644 --- a/index.js +++ b/index.js @@ -9,44 +9,11 @@ var trigger = require("./lib/events/trigger.js") var Pjax = function(options) { this.firstrun = true - this.options = options - this.options.elements = this.options.elements || "a[href], form[action]" - this.options.selectors = this.options.selectors || ["title", ".js-Pjax"] - this.options.switches = this.options.switches || {} - this.options.switchesOptions = this.options.switchesOptions || {} - this.options.history = this.options.history || true - this.options.analytics = this.options.analytics || function(options) { - // options.backward or options.foward can be true or undefined - // by default, we do track back/foward hit - // https://productforums.google.com/forum/#!topic/analytics/WVwMDjLhXYk - if (window._gaq) { - _gaq.push(["_trackPageview"]) - } - if (window.ga) { - ga("send", "pageview", {page: options.url, title: options.title}) - } - } - this.options.scrollTo = (typeof this.options.scrollTo === 'undefined') ? 0 : this.options.scrollTo; - this.options.debug = this.options.debug || false - - this.maxUid = this.lastUid = newUid() - - // we can’t replace body.outerHTML or head.outerHTML - // it create a bug where new body or new head are created in the dom - // if you set head.outerHTML, a new body tag is appended, so the dom get 2 body - // & it break the switchFallback which replace head & body - if (!this.options.switches.head) { - this.options.switches.head = this.switchElementsAlt - } - if (!this.options.switches.body) { - this.options.switches.body = this.switchElementsAlt - } - + var parseOptions = require("./lib/proto/parse-options.js"); + parseOptions.apply(this,options) this.log("Pjax options", this.options) - if (typeof options.analytics !== "function") { - options.analytics = function() {} - } + this.maxUid = this.lastUid = newUid() this.parseDOM(document) diff --git a/lib/proto/parse-options.js b/lib/proto/parse-options.js new file mode 100644 index 0000000..76df548 --- /dev/null +++ b/lib/proto/parse-options.js @@ -0,0 +1,35 @@ +module.exports = function parseOptions(options){ + this.options = options + this.options.elements = this.options.elements || "a[href], form[action]" + this.options.selectors = this.options.selectors || ["title", ".js-Pjax"] + this.options.switches = this.options.switches || {} + this.options.switchesOptions = this.options.switchesOptions || {} + this.options.history = this.options.history || true + this.options.analytics = this.options.analytics || function(options) { + // options.backward or options.foward can be true or undefined + // by default, we do track back/foward hit + // https://productforums.google.com/forum/#!topic/analytics/WVwMDjLhXYk + if (window._gaq) { + _gaq.push(["_trackPageview"]) + } + if (window.ga) { + ga("send", "pageview", {page: options.url, title: options.title}) + } + } + this.options.scrollTo = (typeof this.options.scrollTo === 'undefined') ? 0 : this.options.scrollTo; + this.options.debug = this.options.debug || false + + // we can’t replace body.outerHTML or head.outerHTML + // it create a bug where new body or new head are created in the dom + // if you set head.outerHTML, a new body tag is appended, so the dom get 2 body + // & it break the switchFallback which replace head & body + if (!this.options.switches.head) { + this.options.switches.head = this.switchElementsAlt + } + if (!this.options.switches.body) { + this.options.switches.body = this.switchElementsAlt + } + if (typeof options.analytics !== "function") { + options.analytics = function() {} + } +} \ No newline at end of file diff --git a/tests/lib/proto/parse-options.js b/tests/lib/proto/parse-options.js new file mode 100644 index 0000000..6b7b2a6 --- /dev/null +++ b/tests/lib/proto/parse-options.js @@ -0,0 +1,66 @@ +var tape = require("tape") + +var parseOptions = require("../../../lib/proto/parse-options") +tape("test parse initalization options function", function(t) { + // via http://stackoverflow.com/questions/1173549/how-to-determine-if-an-object-is-an-object-literal-in-javascript + function isObjLiteral(_obj) { + var _test = _obj; + return ( typeof _obj !== 'object' || _obj === null ? + false : + ( + (function () { + while (!false) { + if ( Object.getPrototypeOf( _test = Object.getPrototypeOf(_test) ) === null) { + break; + } + } + return Object.getPrototypeOf(_obj) === _test; + })() + ) + ); + } + function enumerableKeys(_obj) { + var c = 0; + for(var n in _obj){ c++; } + return cl + } + + var body_1 = {}; + var options_1 = {}; + parseOptions.apply(body_1,options_1); + + t.deepEqual(body_1.options.elements,"a[href], form[action]"); + t.deepEqual(body_1.options.selectors.length,2); + t.deepEqual(body_1.options.selectors[0],"title"); + t.deepEqual(body_1.options.selectors[0],".js-Pjax"); + + t.deepEqual(isObjLiteral(body_1.options.switches),true); + t.deepEqual(enumerableKeys(body_1.options.switches),0); + + t.deepEqual(isObjLiteral(body_1.options.switchesOptions),true); + t.deepEqual(enumerableKeys(body_1.options.switchesOptions),0); + + t.deepEqual(body_1.options.history,true); + + //TODO analytics is a little weird right now + t.deepEqual(typeof body_1.options.analytics,"function"); + + t.deepEqual(body_1.options.scrollTo,0); + t.deepEqual(body_1.options.debug,false); + + //verify analytics always ends up as a function even when passed not a function + var body_2 = {}; + var options_2 = {analytics:"some string"}; + parseOptions.apply(body_2,options_2); + + t.deepEqual(typeof body_2.options.analytics,"function"); + + //verify that the value false for scrollTo is not squashed + var body_3 = {}; + var options_3 = {scrollTo:false}; + parseOptions.apply(body_3,options_3); + + t.deepEqual( body_2.options.scrollTo,false); + + t.end() +}) \ No newline at end of file From 7e81f791a954e96572d98504021a03fef6d61dae Mon Sep 17 00:00:00 2001 From: Isaac Gierard Date: Thu, 20 Nov 2014 16:21:19 -0800 Subject: [PATCH 3/3] fixing issues with parse-options and related tests --- index.js | 1 - lib/proto/parse-options.js | 4 +- tests/lib/proto/parse-options.js | 68 +++++++++++++++++--------------- 3 files changed, 40 insertions(+), 33 deletions(-) diff --git a/index.js b/index.js index 44a5ed3..bc68870 100644 --- a/index.js +++ b/index.js @@ -1,4 +1,3 @@ -/* global _gaq: true, ga: true */ var newUid = require("./lib/uniqueid.js") diff --git a/lib/proto/parse-options.js b/lib/proto/parse-options.js index 76df548..75069f2 100644 --- a/lib/proto/parse-options.js +++ b/lib/proto/parse-options.js @@ -1,4 +1,6 @@ -module.exports = function parseOptions(options){ +/* global _gaq: true, ga: true */ + +module.exports = function(options){ this.options = options this.options.elements = this.options.elements || "a[href], form[action]" this.options.selectors = this.options.selectors || ["title", ".js-Pjax"] diff --git a/tests/lib/proto/parse-options.js b/tests/lib/proto/parse-options.js index 6b7b2a6..1fb30be 100644 --- a/tests/lib/proto/parse-options.js +++ b/tests/lib/proto/parse-options.js @@ -1,6 +1,6 @@ var tape = require("tape") -var parseOptions = require("../../../lib/proto/parse-options") +var parseOptions = require("../../../lib/proto/parse-options.js") tape("test parse initalization options function", function(t) { // via http://stackoverflow.com/questions/1173549/how-to-determine-if-an-object-is-an-object-literal-in-javascript function isObjLiteral(_obj) { @@ -21,46 +21,52 @@ tape("test parse initalization options function", function(t) { } function enumerableKeys(_obj) { var c = 0; - for(var n in _obj){ c++; } - return cl + for(var n in _obj){ n = n; c++; } + return c; } - - var body_1 = {}; - var options_1 = {}; - parseOptions.apply(body_1,options_1); + t.test("- default options", function(t){ + var body_1 = {}; + var options_1 = {}; + parseOptions.apply(body_1,[options_1]); - t.deepEqual(body_1.options.elements,"a[href], form[action]"); - t.deepEqual(body_1.options.selectors.length,2); - t.deepEqual(body_1.options.selectors[0],"title"); - t.deepEqual(body_1.options.selectors[0],".js-Pjax"); - - t.deepEqual(isObjLiteral(body_1.options.switches),true); - t.deepEqual(enumerableKeys(body_1.options.switches),0); + t.deepEqual(body_1.options.elements,"a[href], form[action]"); + t.deepEqual(body_1.options.selectors.length,2,"selectors length"); + t.deepEqual(body_1.options.selectors[0],"title"); + t.deepEqual(body_1.options.selectors[1],".js-Pjax"); + + t.deepEqual(isObjLiteral(body_1.options.switches),true); + t.deepEqual(enumerableKeys(body_1.options.switches),2);//head and body - t.deepEqual(isObjLiteral(body_1.options.switchesOptions),true); - t.deepEqual(enumerableKeys(body_1.options.switchesOptions),0); + t.deepEqual(isObjLiteral(body_1.options.switchesOptions),true); + t.deepEqual(enumerableKeys(body_1.options.switchesOptions),0); - t.deepEqual(body_1.options.history,true); + t.deepEqual(body_1.options.history,true); - //TODO analytics is a little weird right now - t.deepEqual(typeof body_1.options.analytics,"function"); + //TODO analytics is a little weird right now + t.deepEqual(typeof body_1.options.analytics,"function"); - t.deepEqual(body_1.options.scrollTo,0); - t.deepEqual(body_1.options.debug,false); + t.deepEqual(body_1.options.scrollTo,0); + t.deepEqual(body_1.options.debug,false); + t.end(); + }); //verify analytics always ends up as a function even when passed not a function - var body_2 = {}; - var options_2 = {analytics:"some string"}; - parseOptions.apply(body_2,options_2); - - t.deepEqual(typeof body_2.options.analytics,"function"); + t.test("- analytics is a function", function(t){ + var body_2 = {}; + var options_2 = {analytics:"some string"}; + parseOptions.apply(body_2,[options_2]); + t.deepEqual(typeof body_2.options.analytics,"function"); + t.end(); + }); //verify that the value false for scrollTo is not squashed - var body_3 = {}; - var options_3 = {scrollTo:false}; - parseOptions.apply(body_3,options_3); - - t.deepEqual( body_2.options.scrollTo,false); + t.test("- scrollTo remains false", function(t){ + var body_3 = {}; + var options_3 = {scrollTo:false}; + parseOptions.apply(body_3,[options_3]); + t.deepEqual( body_3.options.scrollTo,false); + t.end(); + }); t.end() }) \ No newline at end of file