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

Serialize get data-* from DOM, bypass data-* cache #92

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

arturu
Copy link

@arturu arturu commented Nov 16, 2017

The jQuery .data() function caching the data-attributes. If we need to dynamically edit the data attribute, the value isn't updated in the output.

Expected behavior

In this example the problem is fixed https://jsfiddle.net/4df99Lts/3/ (Nestable2 1.6.0-fixed)

Actual behavior

In this example the problem https://jsfiddle.net/4df99Lts/1/ (Nestable2 1.6.0)

Steps to reproduce the behavior

Touch "Configure" on ID: 5. Change value and save. In version https://jsfiddle.net/4df99Lts/3/ the problem is fixed, also, in the https://jsfiddle.net/4df99Lts/1/ no.

Best regards

@pjona
Copy link
Collaborator

pjona commented Nov 17, 2017

@arturu I'm willing to merge this PR, but I have one issue: backward compatibility. Before your changes:

[{"id":1,"value":"test"}]

After your changes (Integer to string convertion):

[{"id":"1","value":"test"}]

Your issue can be fixed without this PR, you need change this line:

$("#list-dd3 li[data-id=" + id + "]").attr('data-'+formInput[i],a);

with

$("#list-dd3 li[data-id=" + id + "]").data(formInput[i],a);

and probably in this line:

$("#list-dd3 li[data-id=" + id + "]").removeAttr('data-'+formInput[i]);

you need to start using .removeData().
Right now I will leave this PR open, and maybe we will merge it before next big release.
Let me know if it helps you.

@ThisIsntMyId
Copy link

Instead, replace line 452 with this item = $.extend({}, JSON.parse(JSON.stringify(this.dataset))),

Refer This

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants