codedance / jquery.AreYouSure

A light-weight jQuery "dirty forms" Plugin - it monitors html forms and alerts users to unsaved changes if they attempt to close the browser or navigate away from the page. (Are you sure?)
508 stars 145 forks source link

addRemoveFieldsMarksDirty doesn't seem to work without Rescan. #77

Open brianlagerman opened 9 years ago

brianlagerman commented 9 years ago

I would think that addRemoveFieldsMarksDirty should fire simply by adding new fields, but it appears that triggering rescan is needed for it to work. I don't understand why though, as I can see that the field count has changed compared to ays-orig-field-count, so the check should be able to evaluate that without a rescan.

codedance commented 9 years ago

The check/scanning is done on a form element change event such as a keypress. Modifications to the DOM such as adding fields do not automatically fire a scan. At a technical level it would be possible to implement with a MutationObserver of equiv. - cross-browser may take a little work.

I'd welcome some thoughts on how important this is. Could it be addressed with a bit of documentation, and then implement when we have a few use cases? I'm keen not to invest in new features (goal to keep things lite) until the need is strong.

brianlagerman commented 9 years ago

In my case I have a form that can have a few thousand fields (I know, I know...) so triggering rescan really hurt. What I did instead was add a manual SetDirty trigger which I could call in place of the rescan, and then just drop the addRemoveFieldsMarksDirty flag as I no longer needed it.

However, if I had to do it again (and I still may) I think it may be better to move the field count check logic from the checkForm function to the beforeUnload event just before the dirtyForms filter is applied.

Something like this, but I haven't tested it one bit...

...
$(window).bind('beforeunload', function () {

    if (settings.addRemoveFieldsMarksDirty) {
        $("form").each(function() {
            var origCount = $(this).data("ays-orig-field-count");
            var fields = $(this).find(settings.fieldSelector);
            // Check if field count has changed
            if (origCount != $fields.length) {
                setDirtyStatus($form, true);
            }
        }); 
    }
 $dirtyForms = $("form").filter('.' + settings.dirtyClass);
 ...

I would just want to make sure that it doesn't incur a big performance hit on large forms (even though it would remain pay-to-play)...

brianlagerman commented 9 years ago

Sorry, didn't mean to close...

codedance commented 9 years ago

My feeling at the moment that this is a niche case. I think your solution is correct for your situation. I'd encourage you to maintain a fork for now, or maybe turn off addRemoveFieldsMarksDirty and do this via your own 'side code'. If we find this requirement pops up again and proves to be less of a side case then we could consider a change to support the optimization.

My gut feel (hope!) is that forms with a few thousand fields will remain niche :-)