search

Yapapaya / jquery-cloneya

A jquery plugin to clone DOM elements
MIT License
76 stars 28 forks source link

Fix #39 - respect dataClone and deepClone options #40

Closed acoulton closed 9 years ago

acoulton commented 9 years ago

Adds test coverage to prove the bug and a fix for issue #39 - child data is always cloned.

acoulton commented 9 years ago

@yapapaya this PR proves - and fixes - the reported issue.

I also found that the preserveChildCount option has been relying on the broken previous behaviour where data values were being copied regardless of the deepClone/dataClone options. I have fixed that code so that it fetches the initial count from the original element, rather than assuming that it will have been cloned to the new one.

That highlights that although this is a bugfix, it will break BC for anyone else who has been relying on the old behaviour of the plugin.

It might be worth changing the defaults for dataClone and deepClone to true? That will mean the plugin keeps working as it has been for anyone using default options, but will work properly for anyone who has set their own values.

It could still break BC for end-users who have been specifying dataClone=false but elsewhere relying on cloned data, but I can't think of a way round that and anyway at the moment their code is not working how they think it is.