Attatch pjax events to injected links?

[pklada]

Should the pjax events attach to links that are injected through a previous pjax call by default or do you have to do something special to attach new pjax events to those newly inserted links? Currently, even though the links I am inserting follow the selector defined in elements, they are causing hard refreshes (not pjax events).

[pklada]

This is my init code:

new Pjax({
  elements: "a.js-pjax"
, selectors: ["title", "[name=analytics_page_name]", ".content", ".pjax-custom-js", ".pjax-mixpanel-track"]
});

new Pjax({
  elements: ".sct a"
, selectors: ["title", "[name=analytics_page_name]", ".hero", "main", ".pjax-custom-js", ".pjax-mixpanel-track"]
});

(note that I have two different pjax implementations, not sure if this would cause the issue.)

[MoOx]

You can just execute this code for every page, links that already get a event listener will be ignored (so everything will work "as expected".

I should update the readme with this example:

// edit: code removed because of the stupidity of the anwser

[pklada]

Ah, okay, good to know. I was unsure if calling pjax on the same element groups would effectively double the attached events. Thanks!

[MoOx]

Hum reopening to let people know that trick while is not documented

[pklada]

Note that I believe the docs say explicitly not to do this :p

[pklada]

Also IMO this is probably something that should be done by the library by default. I can hack at it later when time permits :)

[MoOx]

Haha my bad. I am stupid. The library should do that by default

pjax/src/pjax.js

Lines 467 to 469 in a17a6b9

	this.forEachSelectors(function(el) {
	this.parseDOM(el)
	}, this)

So you should not do what I put in the example above indeed. I'll edit the comment to remove this snippet.

[MoOx]

Take a look here how I'm doing on my website https://github.com/MoOx/moox.github.io/blob/40e2146b8483a7d05fd578c5a99cd136b24a65a9/src/assets/_scripts/vanilla.js

[MoOx]

So this should work by default is selector are ok. It should already reapply to all the selector (& skip the elements already affected)

[MoOx]

Are you using 0.1.4 ?

[pklada]

I am. I did some testing, it appears it works fine if both of my pjax calls use the same selectors, but as soon as i define different selectors per init, it seems to cause issues. I will continue to investigate.

You can see what I mean if you go to http://guidebook.com; clicking around the sub-nav works fine, clicking a main-nav link works fine, but if you click on a main-nav link and then return to the index and then use the sub-nav, it will cause a full reload. Not sure why.

[pklada]

Also, just tested that re-init'ing pjax on pjax:complete does in fact cause the links in question to work again, but the pjax events are bound/fired multiple times, so that doesn't work as a solution.

[MoOx]

I think I get it. For now only one instance is reparsing DOM for its own selectors, not for other pjax instances. So maybe you should use a pjax event to for reparsing for other part. I did do anything special to handle this case. Maybe we should create a pjax.refresh() method to force reload.
It seems like an edge case to use 2 instances that can update same part of the page :/

[pklada]

@MoOx cool, thanks for looking into it. I can take a stab at writing that method if I have time today.

[pklada]

@MoOx I'm going to take a crack at this. Whats the best way to get this repo up and running locally so I can hack at it? Do you just browserify the index.js file? Let me know what your setup looks like. Thanks!

[MoOx]

According to the package.json scripts section, you should just use npm test command (after npm install).
The current repo state is kind of broken (some tests aren't good because last time I work on this repo I didn't finish what I was working).

I started a refactoring (one big file to many files with tests) that I didn't finish.

What should be cool is to work with low level tests. You will probably need to extract some part of the code into new files (because I didn't finish the entire refactoring).
Take a look to this PR #33 that did that kind of stuff.

Tell me if that's enough for you to start.

[pklada]

@MoOx thanks. After playing around with it today I'm not sure the repo is in a working enough state for me to just go in and fix this issue. It seems like its kind of in a broken state, unfortunately :(

I might want to modify the existing version which is hosted on npm and update that to fix this issue. Is that possible?

[MoOx]

You can do that too. Just find a way to share the fix so I can bring it back in there :)

[pklada]

@MoOx pretty simple: pklada/pjax@MoOx:master...master

Just added a simple exposed refresh method which would refresh the document based on the elements in the options. Also added some logic so that calling this wouldnt double-bind pjax events. In the code on my side I just call this on pjax:complete.

var mainPjax = new Pjax({
  elements: "a.js-pjax"
, selectors: ["title", "[name=analytics_page_name]", ".content", ".pjax-custom-js", ".pjax-mixpanel-track"]
});

var sctPjax = new Pjax({
  elements: ".sct a"
, selectors: ["title", "[name=analytics_page_name]", ".hero", "main", ".pjax-custom-js", ".pjax-mixpanel-track"]
});

$(document).on('pjax:complete', function(){
  mainPjax.refresh();
  sctPjax.refresh();
});

Maybe not the most elegant solution, but it works.

[MoOx]

Good to know. Thanks for sharing I'll handle that next time I'll continue the refactoring.

[pklada]

@MoOx what are the plans w/ the refactor? Honestly I might move the current master to a new branch since its in a sort of unfinished state, maybe a refactor branch, and then roll back the current branch to .1.4. I can help in my free time as well as this library was really helpful to me, Id just like to keep the npm package, etc, up to date.

[MoOx]

The current master should work in a Commonjs env. If not, it should not be a big deal to fix since I mainly just move some code (split the big file into smaller tested pieces).

I were working on a branch but was not going forward enough so I decide to break things to move fast.
I understand your arguments but think we should iterate on the current master.

Sorry to not be in a hurry, I'm really busy with bigger OS projects for now (like cssnext or stylelint) but totally want to keep this repo up since I'm using it for my website (I plan to refactor it soon, so I'll end up to use an up to date version of this lib as well).

Note: I recently give write access to @igierard yesterday since he is interested to & have made a nice PR: #33.

Maybe we should just add more minimal tests to ensure the current master works (note that the broken test for now has been made on purpose, as a reminder to finish the refactoring) so we can release new version (reminder we are still under v1.0.0, so we can totally break things ^^).

[pklada]

^ okay, that's fair. I updated my project w/ my forked version for now. (guidebook.com if you want to check it out working) Thanks.

[MoOx]

If you are interested to contribute to the project, just make a nice PR as a start & we will see what happen :)

[pklada]

Closing as my fixes got added to master.