From 688810fbae9022e15f7a1ce0254d332cade00f63 Mon Sep 17 00:00:00 2001 From: Robin North Date: Thu, 25 Jan 2018 10:08:23 +0000 Subject: [PATCH 01/11] Add tests for `lib/util/noop.js` --- tests/lib/util/noop.js | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 tests/lib/util/noop.js diff --git a/tests/lib/util/noop.js b/tests/lib/util/noop.js new file mode 100644 index 0000000..b9eda26 --- /dev/null +++ b/tests/lib/util/noop.js @@ -0,0 +1,9 @@ +var tape = require("tape") + +var noop = require("../../../lib/util/noop") + +tape("test noop function", function(t) { + t.equal(typeof noop, "function", "noop is a function") + t.equal(noop(), undefined, "noop() returns nothing") + t.end() +}) -- 2.49.1 From ee82d0e0c70323048b9994cc9a8c3239b464e47c Mon Sep 17 00:00:00 2001 From: Behind The Math Date: Mon, 19 Mar 2018 22:08:56 -0400 Subject: [PATCH 02/11] Fix bug when checking if elements were parsed already parse-element.js checks if the element was already parsed by checking for the `data-pjax-click-state` attribute. However, this attribute was not added until the link is clicked. Originally, there was a separate attribute, `data-pjax-enabled`, which tracked if the element was parsed already, but that was changed in 9a86044. This commit merges the attributes for mouse clicks and key presses into one and adds that attribute when the element is initially parsed. --- lib/proto/attach-link.js | 24 +++++++++--------------- lib/proto/parse-element.js | 4 ++-- tests/lib/proto/attach-link.js | 2 +- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/lib/proto/attach-link.js b/lib/proto/attach-link.js index 7de3317..bb48902 100644 --- a/lib/proto/attach-link.js +++ b/lib/proto/attach-link.js @@ -1,8 +1,7 @@ var on = require("../events/on") var clone = require("../util/clone") -var attrClick = "data-pjax-click-state" -var attrKey = "data-pjax-keyup-state" +var attrState = "data-pjax-state" var linkAction = function(el, event) { // Since loadUrl modifies options and we may add our own modifications below, @@ -11,7 +10,7 @@ var linkAction = function(el, event) { // Don’t break browser special behavior on links (like page in new window) if (event.which > 1 || event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) { - el.setAttribute(attrClick, "modifier") + el.setAttribute(attrState, "modifier") return } @@ -20,25 +19,25 @@ var linkAction = function(el, event) { // Ignore external links. if (el.protocol !== window.location.protocol || el.host !== window.location.host) { - el.setAttribute(attrClick, "external") + el.setAttribute(attrState, "external") return } // Ignore click if we are on an anchor on the same page if (el.pathname === window.location.pathname && el.hash.length > 0) { - el.setAttribute(attrClick, "anchor-present") + el.setAttribute(attrState, "anchor-present") return } // Ignore anchors on the same page (keep native behavior) if (el.hash && el.href.replace(el.hash, "") === window.location.href.replace(location.hash, "")) { - el.setAttribute(attrClick, "anchor") + el.setAttribute(attrState, "anchor") return } // Ignore empty anchor "foo.html#" if (el.href === window.location.href.split("#")[0] + "#") { - el.setAttribute(attrClick, "anchor-empty") + el.setAttribute(attrState, "anchor-empty") return } @@ -49,12 +48,12 @@ var linkAction = function(el, event) { this.options.currentUrlFullReload && el.href === window.location.href.split("#")[0] ) { - el.setAttribute(attrClick, "reload") + el.setAttribute(attrState, "reload") this.reload() return } - el.setAttribute(attrClick, "load") + el.setAttribute(attrState, "load") options.triggerElement = el this.loadUrl(el.href, options) @@ -71,6 +70,7 @@ module.exports = function(el) { if (isDefaultPrevented(event)) { return } + el.setAttribute(attrState, "") linkAction.call(that, el, event) }) @@ -80,12 +80,6 @@ module.exports = function(el) { return } - // Don’t break browser special behavior on links (like page in new window) - if (event.which > 1 || event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) { - el.setAttribute(attrKey, "modifier") - return - } - if (event.keyCode === 13) { linkAction.call(that, el, event) } diff --git a/lib/proto/parse-element.js b/lib/proto/parse-element.js index e79b4e0..02659d9 100644 --- a/lib/proto/parse-element.js +++ b/lib/proto/parse-element.js @@ -2,14 +2,14 @@ module.exports = function(el) { switch (el.tagName.toLowerCase()) { case "a": // only attach link if el does not already have link attached - if (!el.hasAttribute("data-pjax-click-state")) { + if (!el.hasAttribute("data-pjax-state")) { this.attachLink(el) } break case "form": // only attach link if el does not already have link attached - if (!el.hasAttribute("data-pjax-click-state")) { + if (!el.hasAttribute("data-pjax-state")) { this.attachForm(el) } break diff --git a/tests/lib/proto/attach-link.js b/tests/lib/proto/attach-link.js index cae020a..0886340 100644 --- a/tests/lib/proto/attach-link.js +++ b/tests/lib/proto/attach-link.js @@ -5,7 +5,7 @@ var trigger = require("../../../lib/events/trigger") var attachLink = require("../../../lib/proto/attach-link") var a = document.createElement("a") -var attr = "data-pjax-click-state" +var attr = "data-pjax-state" var preventDefault = function(e) { e.preventDefault() } tape("test attach link prototype method", function(t) { -- 2.49.1 From 494f1f35ac219ca1627321fd715ab1a8acdb2af0 Mon Sep 17 00:00:00 2001 From: Behind The Math Date: Tue, 20 Mar 2018 18:42:39 -0400 Subject: [PATCH 03/11] Bug fixes --- lib/eval-script.js | 2 +- lib/parse-options.js | 20 ++++++++++---------- lib/send-request.js | 4 ++-- lib/util/extend.js | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/eval-script.js b/lib/eval-script.js index b508006..0569e76 100644 --- a/lib/eval-script.js +++ b/lib/eval-script.js @@ -31,7 +31,7 @@ module.exports = function(el) { // execute parent.appendChild(script) // avoid pollution only in head or body tags - if (["head", "body"].indexOf(parent.tagName.toLowerCase()) > 0) { + if (parent instanceof HTMLHeadElement || parent instanceof HTMLBodyElement) { parent.removeChild(script) } diff --git a/lib/parse-options.js b/lib/parse-options.js index a3f5239..b65cd48 100644 --- a/lib/parse-options.js +++ b/lib/parse-options.js @@ -9,16 +9,7 @@ module.exports = function(options) { options.switches = options.switches || {} options.switchesOptions = options.switchesOptions || {} options.history = options.history || true - options.analytics = (typeof options.analytics === "function" || options.analytics === false) ? - options.analytics : - function() { - if (window._gaq) { - _gaq.push(["_trackPageview"]) - } - if (window.ga) { - ga("send", "pageview", {page: location.pathname, title: document.title}) - } - } + options.analytics = (typeof options.analytics === "function" || options.analytics === false) ? options.analytics : defaultAnalytics options.scrollTo = (typeof options.scrollTo === "undefined") ? 0 : options.scrollTo options.scrollRestoration = (typeof options.scrollRestoration !== "undefined") ? options.scrollRestoration : true options.cacheBust = (typeof options.cacheBust === "undefined") ? true : options.cacheBust @@ -38,3 +29,12 @@ module.exports = function(options) { return options } + +function defaultAnalytics() { + if (window._gaq) { + _gaq.push(["_trackPageview"]) + } + if (window.ga) { + ga("send", "pageview", {page: location.pathname, title: document.title}) + } +} diff --git a/lib/send-request.js b/lib/send-request.js index e251e61..1c4a7f8 100644 --- a/lib/send-request.js +++ b/lib/send-request.js @@ -15,7 +15,7 @@ module.exports = function(location, options, callback) { if (request.status === 200) { callback(request.responseText, request, location) } - else { + else if (request.status !== 0) { callback(null, request, location) } } @@ -65,7 +65,7 @@ module.exports = function(location, options, callback) { // Send the proper header information for POST forms if (requestPayload && requestMethod === "POST") { - request.setRequestHeader("Content-type", "application/x-www-form-urlencoded") + request.setRequestHeader("Content-Type", "application/x-www-form-urlencoded") } request.send(requestPayload) diff --git a/lib/util/extend.js b/lib/util/extend.js index 07efde9..dae7e12 100644 --- a/lib/util/extend.js +++ b/lib/util/extend.js @@ -1,6 +1,6 @@ module.exports = function(target) { if (target == null) { - return target + return null } var to = Object(target) -- 2.49.1 From dddb4dd2fb794b27153be9ffd8fb39bf309fdb7d Mon Sep 17 00:00:00 2001 From: Behind The Math Date: Tue, 20 Mar 2018 18:43:05 -0400 Subject: [PATCH 04/11] Fix documentation for currentUrlFullReload --- README.md | 8 +++++++- example/index.html | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 12b2b22..037db2d 100644 --- a/README.md +++ b/README.md @@ -445,10 +445,14 @@ Enables verbose mode. Useful to debug page layout differences. When set to true, clicking on a link that points to the current URL will trigger a full page reload. -The default is `false`, so clicking on such a link will do nothing. +When set to `false`, clicking on such a link will cause Pjax to load the +current page like any page. If you want to add some custom behavior, add a click listener to the link, and set `preventDefault` to true, to prevent Pjax from receiving the event. +Note: this must be done before Pjax is instantiated. Otherwise, Pjax's +event handler will be called first, and preventDefault() won't be called yet. + Here is some sample code: ```js @@ -465,6 +469,8 @@ Here is some sample code: } }) } + + var pjax = new Pjax() ``` (Note that if `cacheBust` is set to true, the code that checks if the href diff --git a/example/index.html b/example/index.html index 39cd261..f22b1ec 100644 --- a/example/index.html +++ b/example/index.html @@ -11,7 +11,7 @@

Index

Hello. Go to Page 2 or Page 3 and view your console to see Pjax events. - Clicking on this page will just reload the page entirely. + Clicking on this page will do nothing.

Manual URL loading

-- 2.49.1 From 31a6f44c97a25c95cc8e9aba3561f670972c8070 Mon Sep 17 00:00:00 2001 From: Behind The Math Date: Tue, 20 Mar 2018 18:43:34 -0400 Subject: [PATCH 05/11] Ignore lines from coverage if they can't be tested --- lib/eval-script.js | 2 ++ lib/parse-options.js | 1 + lib/util/clone.js | 1 + 3 files changed, 4 insertions(+) diff --git a/lib/eval-script.js b/lib/eval-script.js index 0569e76..fd13afa 100644 --- a/lib/eval-script.js +++ b/lib/eval-script.js @@ -13,6 +13,7 @@ module.exports = function(el) { script.type = "text/javascript" + /* istanbul ignore if */ if (src !== "") { script.src = src script.async = false // force synchronous loading of peripheral JS @@ -23,6 +24,7 @@ module.exports = function(el) { script.appendChild(document.createTextNode(code)) } catch (e) { + /* istanbul ignore next */ // old IEs have funky script nodes script.text = code } diff --git a/lib/parse-options.js b/lib/parse-options.js index b65cd48..948dee0 100644 --- a/lib/parse-options.js +++ b/lib/parse-options.js @@ -30,6 +30,7 @@ module.exports = function(options) { return options } +/* istanbul ignore next */ function defaultAnalytics() { if (window._gaq) { _gaq.push(["_trackPageview"]) diff --git a/lib/util/clone.js b/lib/util/clone.js index 674baa3..c4e755b 100644 --- a/lib/util/clone.js +++ b/lib/util/clone.js @@ -1,4 +1,5 @@ module.exports = function(obj) { + /* istanbul ignore if */ if (null === obj || "object" !== typeof obj) { return obj } -- 2.49.1 From cd7be77d999cace6db990651125b8cb429b88bc6 Mon Sep 17 00:00:00 2001 From: Behind The Math Date: Tue, 20 Mar 2018 18:44:15 -0400 Subject: [PATCH 06/11] Refactor attach-link and attach-form --- lib/proto/attach-form.js | 64 ++++++++++++++++++------------------ lib/proto/attach-link.js | 71 +++++++++++++++++++--------------------- 2 files changed, 66 insertions(+), 69 deletions(-) diff --git a/lib/proto/attach-form.js b/lib/proto/attach-form.js index 9ad4c77..2ee84a4 100644 --- a/lib/proto/attach-form.js +++ b/lib/proto/attach-form.js @@ -1,9 +1,13 @@ var on = require("../events/on") var clone = require("../util/clone") -var attrClick = "data-pjax-click-state" +var attrState = "data-pjax-state" var formAction = function(el, event) { + if (isDefaultPrevented(event)) { + return + } + // Since loadUrl modifies options and we may add our own modifications below, // clone it so the changes don't persist var options = clone(this.options) @@ -19,27 +23,9 @@ var formAction = function(el, event) { var virtLinkElement = document.createElement("a") virtLinkElement.setAttribute("href", options.requestOptions.requestUrl) - // Ignore external links. - if (virtLinkElement.protocol !== window.location.protocol || virtLinkElement.host !== window.location.host) { - el.setAttribute(attrClick, "external") - return - } - - // Ignore click if we are on an anchor on the same page - if (virtLinkElement.pathname === window.location.pathname && virtLinkElement.hash.length > 0) { - el.setAttribute(attrClick, "anchor-present") - return - } - - // Ignore empty anchor "foo.html#" - if (virtLinkElement.href === window.location.href.split("#")[0] + "#") { - el.setAttribute(attrClick, "anchor-empty") - return - } - - // if declared as a full reload, just normally submit the form - if (options.currentUrlFullReload) { - el.setAttribute(attrClick, "reload") + var attrValue = checkIfShouldAbort(virtLinkElement, options) + if (attrValue) { + el.setAttribute(attrState, attrValue) return } @@ -59,12 +45,34 @@ var formAction = function(el, event) { } } - el.setAttribute(attrClick, "submit") + el.setAttribute(attrState, "submit") options.triggerElement = el this.loadUrl(virtLinkElement.href, options) } +function checkIfShouldAbort(virtLinkElement, options) { + // Ignore external links. + if (virtLinkElement.protocol !== window.location.protocol || virtLinkElement.host !== window.location.host) { + return "external" + } + + // Ignore click if we are on an anchor on the same page + if (virtLinkElement.hash && virtLinkElement.href.replace(virtLinkElement.hash, "") === window.location.href.replace(location.hash, "")) { + return "anchor" + } + + // Ignore empty anchor "foo.html#" + if (virtLinkElement.href === window.location.href.split("#")[0] + "#") { + return "anchor-empty" + } + + // if declared as a full reload, just normally submit the form + if (options.currentUrlFullReload && virtLinkElement.href === window.location.href.split("#")[0]) { + return "reload" + } +} + var isDefaultPrevented = function(event) { return event.defaultPrevented || event.returnValue === false } @@ -72,19 +80,13 @@ var isDefaultPrevented = function(event) { module.exports = function(el) { var that = this - on(el, "submit", function(event) { - if (isDefaultPrevented(event)) { - return - } + el.setAttribute(attrState, "") + on(el, "submit", function(event) { formAction.call(that, el, event) }) on(el, "keyup", function(event) { - if (isDefaultPrevented(event)) { - return - } - if (event.keyCode === 13) { formAction.call(that, el, event) } diff --git a/lib/proto/attach-link.js b/lib/proto/attach-link.js index bb48902..46e9d29 100644 --- a/lib/proto/attach-link.js +++ b/lib/proto/attach-link.js @@ -4,40 +4,17 @@ var clone = require("../util/clone") var attrState = "data-pjax-state" var linkAction = function(el, event) { + if (isDefaultPrevented(event)) { + return + } + // Since loadUrl modifies options and we may add our own modifications below, // clone it so the changes don't persist var options = clone(this.options) - // Don’t break browser special behavior on links (like page in new window) - if (event.which > 1 || event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) { - el.setAttribute(attrState, "modifier") - return - } - - // we do test on href now to prevent unexpected behavior if for some reason - // user have href that can be dynamically updated - - // Ignore external links. - if (el.protocol !== window.location.protocol || el.host !== window.location.host) { - el.setAttribute(attrState, "external") - return - } - - // Ignore click if we are on an anchor on the same page - if (el.pathname === window.location.pathname && el.hash.length > 0) { - el.setAttribute(attrState, "anchor-present") - return - } - - // Ignore anchors on the same page (keep native behavior) - if (el.hash && el.href.replace(el.hash, "") === window.location.href.replace(location.hash, "")) { - el.setAttribute(attrState, "anchor") - return - } - - // Ignore empty anchor "foo.html#" - if (el.href === window.location.href.split("#")[0] + "#") { - el.setAttribute(attrState, "anchor-empty") + var attrValue = checkIfShouldAbort(el, event) + if (attrValue) { + el.setAttribute(attrState, attrValue) return } @@ -59,6 +36,31 @@ var linkAction = function(el, event) { this.loadUrl(el.href, options) } +function checkIfShouldAbort(el, event) { + // Don’t break browser special behavior on links (like page in new window) + if (event.which > 1 || event.metaKey || event.ctrlKey || event.shiftKey || event.altKey) { + return "modifier" + } + + // we do test on href now to prevent unexpected behavior if for some reason + // user have href that can be dynamically updated + + // Ignore external links. + if (el.protocol !== window.location.protocol || el.host !== window.location.host) { + return "external" + } + + // Ignore anchors on the same page (keep native behavior) + if (el.hash && el.href.replace(el.hash, "") === window.location.href.replace(location.hash, "")) { + return "anchor" + } + + // Ignore empty anchor "foo.html#" + if (el.href === window.location.href.split("#")[0] + "#") { + return "anchor-empty" + } +} + var isDefaultPrevented = function(event) { return event.defaultPrevented || event.returnValue === false } @@ -66,20 +68,13 @@ var isDefaultPrevented = function(event) { module.exports = function(el) { var that = this - on(el, "click", function(event) { - if (isDefaultPrevented(event)) { - return - } el.setAttribute(attrState, "") + on(el, "click", function(event) { linkAction.call(that, el, event) }) on(el, "keyup", function(event) { - if (isDefaultPrevented(event)) { - return - } - if (event.keyCode === 13) { linkAction.call(that, el, event) } -- 2.49.1 From f3dd755a97556b716036cb5f8e648fd1cbf2c351 Mon Sep 17 00:00:00 2001 From: Behind The Math Date: Tue, 20 Mar 2018 18:45:41 -0400 Subject: [PATCH 07/11] Fix and refactors tests --- tests/lib/eval-scripts.js | 4 ++-- tests/lib/execute-scripts.js | 2 +- tests/lib/proto/attach-form.js | 41 ++++++++++++++++++-------------- tests/lib/proto/attach-link.js | 36 +++++++++++++--------------- tests/lib/proto/parse-element.js | 7 +++--- tests/lib/switch-selectors.js | 16 +++++++------ tests/lib/util/extend.js | 4 +--- 7 files changed, 56 insertions(+), 54 deletions(-) diff --git a/tests/lib/eval-scripts.js b/tests/lib/eval-scripts.js index 183fb76..64a66fe 100644 --- a/tests/lib/eval-scripts.js +++ b/tests/lib/eval-scripts.js @@ -14,8 +14,8 @@ tape("test evalScript method", function(t) { t.equal(document.body.className, "executed", "script has been properly executed") script.innerHTML = "document.write('failure')" - document.body.text = "document.write hasn't been executed" - var bodyText = document.body.text + var bodyText = "document.write hasn't been executed" + document.body.text = bodyText evalScript(script) t.equal(document.body.text, bodyText, "document.write hasn't been executed") diff --git a/tests/lib/execute-scripts.js b/tests/lib/execute-scripts.js index bfea9b1..2fdd39e 100644 --- a/tests/lib/execute-scripts.js +++ b/tests/lib/execute-scripts.js @@ -2,7 +2,7 @@ var tape = require("tape") var executeScripts = require("../../lib/execute-scripts") -tape("test executeScripts method", function(t) { +tape("test executeScripts method when the script tag is inside a container", function(t) { document.body.className = "" var container = document.createElement("div") diff --git a/tests/lib/proto/attach-form.js b/tests/lib/proto/attach-form.js index a9923a1..fbb243d 100644 --- a/tests/lib/proto/attach-form.js +++ b/tests/lib/proto/attach-form.js @@ -4,20 +4,18 @@ var on = require("../../../lib/events/on") var trigger = require("../../../lib/events/trigger") var attachForm = require("../../../lib/proto/attach-form") -var form = document.createElement("form") -var attr = "data-pjax-click-state" -var preventDefault = function(e) { e.preventDefault() } +var attr = "data-pjax-state" tape("test attach form prototype method", function(t) { - t.plan(7) + var form = document.createElement("form") + var loadUrlCalled = false attachForm.call({ - options: {}, - reload: function() { - t.equal(form.getAttribute(attr), "reload", "triggering a simple reload will just submit the form") + options: { + currentUrlFullReload: true }, loadUrl: function() { - t.equal(form.getAttribute(attr), "submit", "triggering a post to the next page") + loadUrlCalled = true } }, form) @@ -29,50 +27,57 @@ tape("test attach form prototype method", function(t) { form.action = internalUri + "#anchor" trigger(form, "submit") - t.equal(form.getAttribute(attr), "anchor-present", "internal anchor stop behavior") + t.equal(form.getAttribute(attr), "anchor", "internal anchor stop behavior") window.location.hash = "#anchor" form.action = internalUri + "#another-anchor" trigger(form, "submit") - t.notEqual(form.getAttribute(attr), "anchor", "differents anchors stop behavior") + t.equal(form.getAttribute(attr), "anchor", "different anchors stop behavior") window.location.hash = "" form.action = internalUri + "#" trigger(form, "submit") t.equal(form.getAttribute(attr), "anchor-empty", "empty anchor stop behavior") - form.action = internalUri + form.action = window.location.href trigger(form, "submit") - // see reload defined above + t.equal(form.getAttribute(attr), "reload", "submitting when currentUrlFullReload is true will submit normally, without XHR") + t.equal(loadUrlCalled, false, "loadUrl() not called") form.action = window.location.protocol + "//" + window.location.host + "/internal" form.method = "POST" trigger(form, "submit") - // see post defined above + t.equal(form.getAttribute(attr), "submit", "triggering a POST request to the next page") + t.equal(loadUrlCalled, true, "loadUrl() called correctly") + loadUrlCalled = false + form.setAttribute(attr, "") form.action = window.location.protocol + "//" + window.location.host + "/internal" form.method = "GET" trigger(form, "submit") - // see post defined above + t.equal(form.getAttribute(attr), "submit", "triggering a GET request to the next page") + t.equal(loadUrlCalled, true, "loadUrl() called correctly") t.end() }) tape("test attach form preventDefaulted events", function(t) { - var callbacked = false + var loadUrlCalled = false var form = document.createElement("form") + // This needs to be before the call to attachFormk() + on(form, "submit", function(event) { event.preventDefault() }) + attachForm.call({ options: {}, loadUrl: function() { - callbacked = true + loadUrlCalled = true } }, form) form.action = "#" - on(form, "submit", preventDefault) trigger(form, "submit") - t.equal(callbacked, false, "events that are preventDefaulted should not fire callback") + t.equal(loadUrlCalled, false, "events that are preventDefaulted should not fire callback") t.end() }) diff --git a/tests/lib/proto/attach-link.js b/tests/lib/proto/attach-link.js index 0886340..113ea83 100644 --- a/tests/lib/proto/attach-link.js +++ b/tests/lib/proto/attach-link.js @@ -4,27 +4,22 @@ var on = require("../../../lib/events/on") var trigger = require("../../../lib/events/trigger") var attachLink = require("../../../lib/proto/attach-link") -var a = document.createElement("a") var attr = "data-pjax-state" -var preventDefault = function(e) { e.preventDefault() } tape("test attach link prototype method", function(t) { - t.plan(7) + var a = document.createElement("a") + var loadUrlCalled = false attachLink.call({ options: {}, - reload: function() { - t.equal(a.getAttribute(attr), "reload", "triggering exact same url reload the page") - }, loadUrl: function() { - t.equal(a.getAttribute(attr), "load", "triggering a internal link actually load the page") + loadUrlCalled = true } }, a) var internalUri = window.location.protocol + "//" + window.location.host + window.location.pathname + window.location.search a.href = internalUri - on(a, "click", preventDefault) // to avoid link to be open (break testing env) trigger(a, "click", {metaKey: true}) t.equal(a.getAttribute(attr), "modifier", "event key modifier stop behavior") @@ -32,46 +27,47 @@ tape("test attach link prototype method", function(t) { trigger(a, "click") t.equal(a.getAttribute(attr), "external", "external url stop behavior") + window.location.hash = "#anchor" a.href = internalUri + "#anchor" trigger(a, "click") - t.equal(a.getAttribute(attr), "anchor-present", "internal anchor stop behavior") + t.equal(a.getAttribute(attr), "anchor", "internal anchor stop behavior") - window.location.hash = "#anchor" a.href = internalUri + "#another-anchor" trigger(a, "click") - t.notEqual(a.getAttribute(attr), "anchor", "differents anchors stop behavior") + t.equal(a.getAttribute(attr), "anchor", "different anchors stop behavior") window.location.hash = "" a.href = internalUri + "#" trigger(a, "click") t.equal(a.getAttribute(attr), "anchor-empty", "empty anchor stop behavior") - a.href = internalUri - trigger(a, "click") - // see reload defined above - a.href = window.location.protocol + "//" + window.location.host + "/internal" trigger(a, "click") - // see loadUrl defined above + t.equals(a.getAttribute(attr), "load", "triggering an internal link sets the state attribute to 'load'") + t.equals(loadUrlCalled, true, "triggering an internal link actually loads the page") t.end() }) tape("test attach link preventDefaulted events", function(t) { - var callbacked = false + var loadUrlCalled = false var a = document.createElement("a") + // This needs to be before the call to attachLink() + on(a, "click", function(event) { + event.preventDefault() + }) + attachLink.call({ options: {}, loadUrl: function() { - callbacked = true + loadUrlCalled = true } }, a) a.href = "#" - on(a, "click", preventDefault) trigger(a, "click") - t.equal(callbacked, false, "events that are preventDefaulted should not fire callback") + t.equal(loadUrlCalled, false, "events that are preventDefaulted should not fire callback") t.end() }) diff --git a/tests/lib/proto/parse-element.js b/tests/lib/proto/parse-element.js index 082c6ac..9d64e34 100644 --- a/tests/lib/proto/parse-element.js +++ b/tests/lib/proto/parse-element.js @@ -1,7 +1,8 @@ var tape = require("tape") var parseElement = require("../../../lib/proto/parse-element") -var protoMock = { + +var pjax = { attachLink: function() { return true }, attachForm: function() { return true } } @@ -9,12 +10,12 @@ var protoMock = { tape("test parse element prototype method", function(t) { t.doesNotThrow(function() { var a = document.createElement("a") - parseElement.call(protoMock, a) + parseElement.call(pjax, a) }, " element can be parsed") t.doesNotThrow(function() { var form = document.createElement("form") - parseElement.call(protoMock, form) + parseElement.call(pjax, form) }, "
element can be parsed") t.end() diff --git a/tests/lib/switch-selectors.js b/tests/lib/switch-selectors.js index 1e32e04..a999d9a 100644 --- a/tests/lib/switch-selectors.js +++ b/tests/lib/switch-selectors.js @@ -1,18 +1,20 @@ var tape = require("tape") var switchesSelectors = require("../../lib/switches-selectors.js") +var noop = require("../../lib/util/noop") + +var pjax = { + onSwitch: function() { + console.log("Switched") + }, + state: {}, + log: noop +} // @author darylteo tape("test switchesSelectors", function(t) { // switchesSelectors relies on a higher level function callback // should really be passed in instead so I'll leave it here as a TODO: - var pjax = { - onSwitch: function() { - console.log("Switched") - }, - state: {} - } - var tmpEl = document.implementation.createHTMLDocument() // a div container is used because swapping the containers diff --git a/tests/lib/util/extend.js b/tests/lib/util/extend.js index 0319797..f90468b 100644 --- a/tests/lib/util/extend.js +++ b/tests/lib/util/extend.js @@ -4,12 +4,10 @@ var extend = require("../../../lib/util/extend") tape("test extend method", function(t) { var obj = {one: 1, two: 2} + var extended = extend({}, obj, {two: "two", three: 3}) - t.notEqual(obj, extended, "extended object isn't the original object") - t.notSame(obj, extended, "extended object doesn't have the same values as original object") - t.notSame(obj.two, extended.two, "extended object value overwrites value from original object") t.end() -- 2.49.1 From 39a96e51e7ec4e82c11489c8827892fbf40e2868 Mon Sep 17 00:00:00 2001 From: Behind The Math Date: Tue, 20 Mar 2018 18:45:56 -0400 Subject: [PATCH 08/11] Add tests --- tests/lib/execute-scripts.js | 13 ++++++ tests/lib/proto/attach-form.js | 54 ++++++++++++++++++++++ tests/lib/proto/attach-link.js | 53 ++++++++++++++++++++++ tests/lib/proto/parse-element.js | 5 ++ tests/lib/send-request.js | 78 ++++++++++++++++++++++++++++++++ tests/lib/switch-selectors.js | 30 ++++++++++++ tests/lib/util/extend.js | 3 ++ 7 files changed, 236 insertions(+) diff --git a/tests/lib/execute-scripts.js b/tests/lib/execute-scripts.js index 2fdd39e..0d93e15 100644 --- a/tests/lib/execute-scripts.js +++ b/tests/lib/execute-scripts.js @@ -14,3 +14,16 @@ tape("test executeScripts method when the script tag is inside a container", fun t.end() }) + +tape("test executeScripts method with just a script tag", function(t) { + document.body.className = "" + + var script = document.createElement("script") + script.innerHTML = "document.body.className = 'executed correctly';" + + t.equal(document.body.className, "", "script hasn't been executed yet") + executeScripts(script) + t.equal(document.body.className, "executed correctly", "script has been properly executed") + + t.end() +}) diff --git a/tests/lib/proto/attach-form.js b/tests/lib/proto/attach-form.js index fbb243d..249afc8 100644 --- a/tests/lib/proto/attach-form.js +++ b/tests/lib/proto/attach-form.js @@ -98,3 +98,57 @@ tape("test options are not modified by attachForm", function(t) { t.end() }) + +tape("test submit triggered by keyboard", function(t) { + var form = document.createElement("form") + var pjax = { + options: {}, + loadUrl: function() { + t.equal(form.getAttribute(attr), "submit", "triggering a internal link actually submits the form") + } + } + + t.plan(2) + + attachForm.call(pjax, form) + + form.action = window.location.protocol + "//" + window.location.host + "/internal" + + trigger(form, "keyup", {keyCode: 14}) + t.equal(form.getAttribute(attr), "", "keycode other than 13 doesn't trigger anything") + + trigger(form, "keyup", {keyCode: 13}) + // see loadUrl defined above + + t.end() +}) + +tape("test form elements parsed correctly", function(t) { + t.plan(1) + + var form = document.createElement("form") + var input = document.createElement("input") + input.name = "input" + input.value = "value" + form.appendChild(input) + + var params = [{ + name: "input", + value: "value" + }] + var pjax = { + options: {}, + loadUrl: function(href, options) { + t.same(options.requestOptions.requestParams, params, "form elements parsed correctly") + } + } + + attachForm.call(pjax, form) + + form.action = window.location.protocol + "//" + window.location.host + "/internal" + + trigger(form, "submit") + // see loadUrl defined above + + t.end() +}) diff --git a/tests/lib/proto/attach-link.js b/tests/lib/proto/attach-link.js index 113ea83..fb88682 100644 --- a/tests/lib/proto/attach-link.js +++ b/tests/lib/proto/attach-link.js @@ -88,3 +88,56 @@ tape("test options are not modified by attachLink", function(t) { t.end() }) + +tape("test link triggered by keyboard", function(t) { + var a = document.createElement("a") + var pjax = { + options: {}, + loadUrl: function() { + t.equal(a.getAttribute(attr), "load", "triggering a internal link actually loads the page") + } + } + + t.plan(3) + + attachLink.call(pjax, a) + + a.href = window.location.protocol + "//" + window.location.host + "/internal" + + trigger(a, "keyup", {keyCode: 14}) + t.equal(a.getAttribute(attr), "", "keycode other than 13 doesn't trigger anything") + + trigger(a, "keyup", {keyCode: 13, metaKey: true}) + t.equal(a.getAttribute(attr), "modifier", "event key modifier stop behavior") + + trigger(a, "keyup", {keyCode: 13}) + // see loadUrl defined above + + t.end() +}) + +tape("test link with the same URL as the current one, when currentUrlFullReload set to true", function(t) { + var a = document.createElement("a") + var pjax = { + options: { + currentUrlFullReload: true + }, + reload: function() { + t.pass("this.reload() was called correctly") + }, + loadUrl: function() { + t.fail("loadUrl() was called wrongly") + } + } + + t.plan(2) + + attachLink.call(pjax, a) + + a.href = window.location.href + + trigger(a, "click") + t.equal(a.getAttribute(attr), "reload", "reload stop behavior") + + t.end() +}) diff --git a/tests/lib/proto/parse-element.js b/tests/lib/proto/parse-element.js index 9d64e34..e58ef67 100644 --- a/tests/lib/proto/parse-element.js +++ b/tests/lib/proto/parse-element.js @@ -18,5 +18,10 @@ tape("test parse element prototype method", function(t) { parseElement.call(pjax, form) }, " element can be parsed") + t.throws(function() { + var el = document.createElement("div") + parseElement.call(pjax, el) + }, "
element cannot be parsed") + t.end() }) diff --git a/tests/lib/send-request.js b/tests/lib/send-request.js index 8b86726..32d6de9 100644 --- a/tests/lib/send-request.js +++ b/tests/lib/send-request.js @@ -57,3 +57,81 @@ tape("request headers are sent properly", function(t) { }) }) +tape("HTTP status codes other than 200 are handled properly", function(t) { + var url = "https://httpbin.org/status/400" + + sendRequest(url, {}, function(responseText, request) { + t.equals(responseText, null, "responseText is null") + t.equals(request.status, 400, "HTTP status code is correct") + + t.end() + }) +}) + +tape.skip("XHR error is handled properly", function(t) { + var url = "https://encrypted.google.com/foobar" + + sendRequest(url, {}, function(responseText) { + t.equals(responseText, null, "responseText is null") + + t.end() + }) +}) + +tape("POST body data is sent properly", function(t) { + var url = "https://httpbin.org/post" + var params = [{ + name: "test", + value: "1" + }]; + var options = { + requestOptions: { + requestMethod: "POST", + requestParams: params + } + } + + sendRequest(url, options, function(responseText) { + var response = JSON.parse(responseText) + + t.same(response.form[params[0].name], params[0].value, "requestParams were sent properly") + t.equals(response.headers["Content-Type"], "application/x-www-form-urlencoded", "Content-Type header was set properly") + + t.end() + }) +}) + +tape("GET query data is sent properly", function(t) { + var url = "https://httpbin.org/get" + var params = [{ + name: "test", + value: "1" + }]; + var options = { + requestOptions: { + requestParams: params + } + } + + sendRequest(url, options, function(responseText) { + var response = JSON.parse(responseText) + + t.same(response.args[params[0].name], params[0].value, "requestParams were sent properly") + + t.end() + }) +}) + +tape("XHR timeout is handled properly", function(t) { + var url = "https://httpbin.org/delay/5" + var options = { + timeout: 1000 + } + + sendRequest(url, options, function(responseText) { + t.equals(responseText, null, "responseText is null") + + t.end() + }) +}) + diff --git a/tests/lib/switch-selectors.js b/tests/lib/switch-selectors.js index a999d9a..cea115d 100644 --- a/tests/lib/switch-selectors.js +++ b/tests/lib/switch-selectors.js @@ -42,3 +42,33 @@ tape("test switchesSelectors", function(t) { t.end() }) + +tape("test switchesSelectors when number of elements don't match", function(t) { + var newTempDoc = document.implementation.createHTMLDocument() + var originalTempDoc = document.implementation.createHTMLDocument() + + // a div container is used because swapping the containers + // will generate a new element, so things get weird + // using "body" generates a lot of testling cruft that I don't + // want so let's avoid that + var container = originalTempDoc.createElement("div") + container.innerHTML = "

Original text

No change" + originalTempDoc.body.appendChild(container) + + var container2 = newTempDoc.createElement("div") + container2.innerHTML = "

New text

More new text

New span" + newTempDoc.body.appendChild(container2) + + var switchSelectorsFn = switchesSelectors.bind(pjax, + {}, // switches + {}, // switchesOptions + ["p"], // selectors, + newTempDoc, // fromEl + originalTempDoc, // toEl, + {} // options + ) + + t.throws(switchSelectorsFn, null, "error was thrown properly since number of elements don't match") + + t.end() +}) diff --git a/tests/lib/util/extend.js b/tests/lib/util/extend.js index f90468b..add707f 100644 --- a/tests/lib/util/extend.js +++ b/tests/lib/util/extend.js @@ -10,5 +10,8 @@ tape("test extend method", function(t) { t.notSame(obj, extended, "extended object doesn't have the same values as original object") t.notSame(obj.two, extended.two, "extended object value overwrites value from original object") + extended = extend(null) + t.equals(extended, null, "passing null returns null") + t.end() }) -- 2.49.1 From b3f7fcefeec0bc7ef2dc70a38f0afee67091314e Mon Sep 17 00:00:00 2001 From: Behind The Math Date: Tue, 20 Mar 2018 21:32:01 -0400 Subject: [PATCH 09/11] Bug fixes and tests for switches --- lib/switches.js | 9 +++++++- tests/lib/switches.js | 54 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/lib/switches.js b/lib/switches.js index cdacc0e..0502fc2 100644 --- a/lib/switches.js +++ b/lib/switches.js @@ -8,7 +8,14 @@ module.exports = { innerHTML: function(oldEl, newEl) { oldEl.innerHTML = newEl.innerHTML - oldEl.className = newEl.className + + if (newEl.className === "") { + oldEl.removeAttribute("class") + } + else { + oldEl.className = newEl.className + } + this.onSwitch() }, diff --git a/tests/lib/switches.js b/tests/lib/switches.js index 1752cb8..ecf5ed8 100644 --- a/tests/lib/switches.js +++ b/tests/lib/switches.js @@ -2,6 +2,60 @@ var tape = require("tape") var switches = require("../../lib/switches") var noop = require("../../lib/util/noop") +tape("test outerHTML switch", function(t) { + var outerHTML = switches.outerHTML + + var doc = document.implementation.createHTMLDocument() + + var container = doc.createElement("div") + container.innerHTML = "

Original Text

" + doc.body.appendChild(container) + + var p = doc.createElement("p") + p.innerHTML = "New Text" + + outerHTML.bind({ + onSwitch: noop + })(doc.querySelector("p"), p) + + t.equals(doc.querySelector("p").innerHTML, "New Text", "Elements correctly switched") + t.notEquals(doc.querySelector("p").id, "p", "other attributes overwritten correctly") + + t.end() +}) + +tape("test innerHTML switch", function(t) { + var innerHTML = switches.innerHTML + + var doc = document.implementation.createHTMLDocument() + + var container = doc.createElement("div") + container.innerHTML = "

Original Text

" + doc.body.appendChild(container) + + var p = doc.createElement("p") + p.innerHTML = "New Text" + p.className = "p" + + innerHTML.bind({ + onSwitch: noop + })(doc.querySelector("p"), p) + + t.equals(doc.querySelector("p").innerHTML, "New Text", "Elements correctly switched") + t.equals(doc.querySelector("p").className, "p", "classname set correctly") + t.equals(doc.querySelector("p").id, "p", "other attributes set correctly") + + p.removeAttribute("class") + + innerHTML.bind({ + onSwitch: noop + })(doc.querySelector("p"), p) + + t.equals(doc.querySelector("p").className, "", "classname set correctly") + + t.end() +}) + tape("test replaceNode switch", function(t) { var replaceNode = switches.replaceNode -- 2.49.1 From 4c0b6d6251f3a57cf73bd2ca54cf25dd5f48a5d0 Mon Sep 17 00:00:00 2001 From: Behind The Math Date: Tue, 20 Mar 2018 21:32:56 -0400 Subject: [PATCH 10/11] Add TS definitions for options.requestOptions --- index.d.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/index.d.ts b/index.d.ts index ed9b993..0787688 100644 --- a/index.d.ts +++ b/index.d.ts @@ -168,9 +168,23 @@ declare namespace Pjax { * will not work, due to the query string appended to force a cache bust). */ currentUrlFullReload: boolean; + + /** + * Hold the information to make an XHR request. + */ + requestOptions?: { + requestUrl?: string; + requestMethod?: string; + requestParams?: IRequestParams[]; + } } export type Switch = (oldEl: Element, newEl: Element, options?: IOptions, switchesOptions?: StringKeyedObject) => void; + + export interface IRequestParams { + name: string, + value: string + } } interface StringKeyedObject { -- 2.49.1 From a42ae57448d4fedfad05ada8b69ae1413fb118ee Mon Sep 17 00:00:00 2001 From: Behind The Math Date: Mon, 9 Apr 2018 17:39:20 -0400 Subject: [PATCH 11/11] Code cleanup --- lib/proto/parse-element.js | 6 ++++-- tests/lib/proto/attach-form.js | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/proto/parse-element.js b/lib/proto/parse-element.js index 02659d9..8214a67 100644 --- a/lib/proto/parse-element.js +++ b/lib/proto/parse-element.js @@ -1,15 +1,17 @@ +var attrState = "data-pjax-state" + module.exports = function(el) { switch (el.tagName.toLowerCase()) { case "a": // only attach link if el does not already have link attached - if (!el.hasAttribute("data-pjax-state")) { + if (!el.hasAttribute(attrState)) { this.attachLink(el) } break case "form": // only attach link if el does not already have link attached - if (!el.hasAttribute("data-pjax-state")) { + if (!el.hasAttribute(attrState)) { this.attachForm(el) } break diff --git a/tests/lib/proto/attach-form.js b/tests/lib/proto/attach-form.js index 249afc8..865e993 100644 --- a/tests/lib/proto/attach-form.js +++ b/tests/lib/proto/attach-form.js @@ -65,7 +65,7 @@ tape("test attach form preventDefaulted events", function(t) { var loadUrlCalled = false var form = document.createElement("form") - // This needs to be before the call to attachFormk() + // This needs to be before the call to attachForm() on(form, "submit", function(event) { event.preventDefault() }) attachForm.call({ -- 2.49.1