Handle XHR response error #137

Merged
BehindTheMath merged 6 commits from feature/handle-response-error into master 2018-03-15 15:12:32 -05:00
2 changed files with 9 additions and 2 deletions
Showing only changes of commit b0748a2aa5 - Show all commits

View File

@@ -453,7 +453,7 @@ All events are fired from the _document_, not the link that was clicked.
* `pjax:send` - Fired after the Pjax request begins. * `pjax:send` - Fired after the Pjax request begins.
* `pjax:complete` - Fired after the Pjax request finishes. * `pjax:complete` - Fired after the Pjax request finishes.
* `pjax:success` - Fired after the Pjax request succeeds. * `pjax:success` - Fired after the Pjax request succeeds.
* `pjax:error` - Fired after the Pjax request fails. * `pjax:error` - Fired after the Pjax request fails. The request object will be passed along as `event.options.request`.
`send` and `complete` are a good pair of events to use if you are implementing a loading indicator (eg: [topbar](http://buunguyen.github.io/topbar/)) `send` and `complete` are a good pair of events to use if you are implementing a loading indicator (eg: [topbar](http://buunguyen.github.io/topbar/))

View File

@@ -5,7 +5,10 @@ var trigger = require("./lib/events/trigger.js")
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
module.export = function(responseText, request, href) { module.export = function(responseText, request, href) {
// Fail if unable to load HTML via AJAX // Fail if unable to load HTML via AJAX
if (responseText === false) { if (responseText === false) {
trigger(document, "pjax:complete pjax:error", this.options) var tempOptions = this.options
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
tempOptions.request = request
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
trigger(document, "pjax:complete pjax:error", tempOptions)
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
return return
} }
@@ -52,6 +55,10 @@ module.export = function(responseText, request, href) {
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
this.loadContent(responseText, this.options) this.loadContent(responseText, this.options)
} }
catch (e) { catch (e) {
var tempOptions2 = this.options
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
tempOptions2.request = request
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
trigger(document, "pjax:error", tempOptions2)
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
if (!this.options.debug) { if (!this.options.debug) {
if (console && console.error) { if (console && console.error) {
console.error("Pjax switch fail: ", e) console.error("Pjax switch fail: ", e)
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:01:00 -05:00 (Migrated from github.com)
Review

This needs to be tempOptions = clone(this.options), or you'll mutate the current instance's options

This needs to be `tempOptions = clone(this.options)`, or you'll mutate the current instance's options
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
robinnorth commented 2018-03-10 13:02:53 -05:00 (Migrated from github.com)
Review

This also needs to clone this.options, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.

This also needs to clone `this.options`, but you could probably just do this once at the top of the method rather than repeating the assignment of the request to your temporary event options.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:18:18 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.
BehindTheMath commented 2018-03-10 23:21:19 -05:00 (Migrated from github.com)
Review

Good point.

Good point.