Broken by new version of Issues?

[parkr]

Was just trying to take a selfie but it wasn't working. Sad panda. :panda_face: :frowning:

[mattandrews]

Yep, same for me :(

[thieman]

AH CRAP will take a look this weekend, sorry friends

[thieman]

Alright, there's good news and bad news.

Good news: I've just merged and published a partial fix. If you get to the page where you want to add a selfie and then manually refresh the page, the selfie interface should show up and work as normal.

Bad news: Yeah, that whole "manual refresh" thing. GitHub seems to have made the entire site navigation client-side with this latest update. This screws with how the Chrome extension knows which scripts to fire, since usually it can say "Oh hey you're loading /repo/user/issues so I know to load issues.js now." It can no longer do that since that page load event never actually gets fired since the nav is now done in JavaScript.

I don't have the mental bandwidth to work on a more comprehensive fix right now, but it's an open source project if anyone else would like to give it a shot. :smile:

[mattandrews]

Thanks! I took a look and tried to fix it: seems like responding to pushState events in a Chrome extension is quite hard. My fix basically involved loading all your JS at once and calling the relevant function (eg. set up the /issues/ endpoint) either when the URL matches, or when a pushState event fires and it matches that string. This feels a bit hacky as your neatly-modularised code now becomes one large file – is it worth pursuing this method or would you be reluctant to merge something taking that route?

[thieman]

@mattandrews Messy, working code is always preferred to neat, broken code. :smile:

This approach sounds fine given the constraints of the problem. Thanks for taking a look!

[thieman]

Just something to think about, might we be able to get the best of both worlds if we add an additional compile step? Neatly modularized code that is then combined into a larger file so it works in the constraints of the extension? Maybe even with namespaces and such, can Google Closure or a similar project work for that?

[mattandrews]

Yeah, that could work – requireJS and Grunt might make this easier...

[sigmavirus24]

Google Closure tools will definitely work with that. One note is that if GitHub is still using pjax to handle all of this, there are events that it triggers (I'm pretty sure on the document) that you can listen for, e.g., pjax:end (although it's been a while since I've used them.) Can a Chrome extension listen for those? (There are also events to notify listeners that the location will be changing. There's some documentation here)

Edit: The list of events is here. I remember doing something with pjax:complete but I don't quite remember how I found it. I'm going to ping my friend @losingkeys to see if he can be of assistance.

[losingkeys]

Yeah I would use something like grunt-requirejs, grunt-closure-utilities, or just grunt-contrib-concat and grunt-contrib-uglify to compile everything together, then you could either detect the pjax events (like @sigmavirus24 pointed out), or use pushstate if that's available to chrome extensions. One possible wrinkle with pushstate: iirc, the last time I was using it it fired on page load in addition to back/forward in chrome or firefox... but not both, so that's something to watch out for. Hope this helps!

[parabuzzle]

this may or may not be related: https://github.com/thieman/github-selfies/issues/51

[cabello]

Currently broken for me. =(

[ShonM]

Here's what I've been doing: Re-firing an event "into" the extension from the page. In my page.js I have this:

$(document).on('pjax:end', function () {
    // Duplicate the event with a slightly different name
    var e = document.createEvent('Events');
    e.initEvent('_pjax:end', !'bubbles', !'cancelable');

    return document.dispatchEvent(e);
});

Then, in my page JS I have

    document.addEventListener('_pjax:end', function() {
        client.setup();
    }, false);

Of course clientSetup is just the method you want to happen on each PJAX finish. I really hope this helps! If not, I'm happy to post more code/info :)

[ShonM]

Oh! Derp. I forgot how page.js made it onto the actual page. Injection!

    var s = document.createElement('script');
    s.src = chrome.extension.getURL('scripts/page.js');
    s.onload = function() {
        this.parentNode.removeChild(this);
    };
    (document.head||document.documentElement).appendChild(s);

Ok, that should be everything required to make this function again :)