dstreet / dependsOn

a jQuery plugin for handling form field dependencies
http://dstreet.github.com/dependsOn
MIT License
106 stars 32 forks source link

dependsOn misses programmatic changes (including those it made itself) #17

Closed eboth closed 8 years ago

eboth commented 9 years ago

In a chain of dependencies, the dependency checks do not propagate down the chain.

The reason is that dependsOn (quite obviously) relies on the onchange event to trigger dependency checks, but the onchange event is only raised by user actions, not by changes caused by JS code.

If after a user action, dependsOn makes a change (disabling an element or changing its value), this may change the status of a different dependency. However, the corresponding dependency check is never triggered as the change made by dependsOn did not raise an onchange event.

Imagine a sequence of three dropdown lists in a car configurator form: make model trim line

Model depends on Make, and Trim line depends on Model. If you select a make, you get to choose from the available models, and upon selecting a model, you are shown a list of the trim lines available for that model. Now if you change your mind and select a different make, the model list is updated. However, dependsOn does not detect the change in the model list (as there has been no onchange event), and the previous (now wrong) trim line list continues to be displayed.

On all occasions I have observed this problem, it would have been desirable for dependsOn to send a change event on disabling, enabling, or changing an element. Technically, when enabling/disabling, the element's value does not really change, but the value of its container element can change. For instance if the selected option element is disabled, the value of the containing select changes to null (or, with multiple selections, its value is presumably removed from the array of selected values). To work around this, I have changed the source to trigger an onchange event whenever an element is enabled or disabled (the code below is for DependencyCollection.prototype.enable; the change is analogous for disable or for changing the value of valueSubject: New:

    // Remove the disabled attribute from the subject if allowed by the settings
    if ( this.settings.disable &&
                 this.$subject.attr( 'disabled' ) !== undefined ) {
        this.$subject.removeAttr( 'disabled' );
        this.$subject.trigger( 'change' );
    }

Old:

    // Remove the disabled attribute from the subject if allowed by the settings
    if ( this.settings.disable ) {
        this.$subject.removeAttr( 'disabled' );
    }

This obviously introduces the possibility of endless check loops for circular dependencies, but still I think a front-end option for this would be a great addition to dependsOn. Lacking a specific onchange option for dependsOn(), the obvious place to put this trigger('change') call would be onEnable and onDisable. However, the callback parameters triggeredDependency and triggeredEvent contain no reference to the dependency subjects, so that this information would need to be hardwired into the callback function, which means that a specific callback function would be needed for every dependent element.

Or maybe I have missed something obvious. Endre

eboth commented 9 years ago

Duh, just discovered the TODO "Trigger the "change" event when the subject values are altered." in the TODO list. Sorry for the epistle above.

dstreet commented 9 years ago

@eboth, thanks for the detailed write-up. That issue has been something I have needed to take care of for a while now, and is on my list for the next release. The onEnable and onDisable callback functions should set this to the subject.