Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow pjax with "data-pjax" with no value in yii.js #13300

Closed
arogachev opened this issue Dec 30, 2016 · 2 comments
Closed

Allow pjax with "data-pjax" with no value in yii.js #13300

arogachev opened this issue Dec 30, 2016 · 2 comments
Labels
feature:pjax JS JavaScript related
Milestone

Comments

@arogachev
Copy link
Contributor

arogachev commented Dec 30, 2016

I found this issue during writing tests for yii.js.

After 9807b2e we lose the ability to have pjax links like

<a data-pjax href="#">

or

<a data-pjax="" href="#">

(which is essentially the same).

This is because of:

pjax = $e.data('pjax') || 0,

"" || 0 resolves to 0.

This condtradicts with confuring pjax described in the docs for example here:

Or try this selector that matches any <a data-pjax href=> links inside a <div data-pjax> container.

I think that the older behavior - pjax !== undefined && pjax !== 0 && $.support.pjax is more correct, also note that @arisk sent the PR including this check in #12737 (383aa47), then it was ignored.

It will be better if we disable pjax only if data-pjax attribute is set to 0 only:

usePjax = pjax !== undefined && pjax !== 0 && $.support.pjax,

Note however that this check in Yii2 pjax adaptation:

// Ignore links with data-pjax="0"
if ($(link).data('pjax') == 0) {
    return;
}

needs to be modified accordingly too, because "" == 0 will evaluate to true.

Can be:

$(link).data('pjax') === 0)

(data returns 0 as number).

Or in case of a string which is returend by attr for example:

$(link).attr('data-pjax') === '0'

Or maybe even better:

parseInt($(link).data('pjax')) === 0

Note: I already fixed it in yii.js in PR containing tests for yii.js. PR will be sent soon.

@samdark samdark added feature:pjax JS JavaScript related labels Dec 30, 2016
@samdark
Copy link
Member

samdark commented Dec 30, 2016

Agree that it should be fixed as you've described.

@Mirocow
Copy link

Mirocow commented Jan 23, 2018

Substituting this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature:pjax JS JavaScript related
Projects
None yet
Development

No branches or pull requests

3 participants