slightlyoff / cassowary.js

Cassowary/JS, better, faster, future-ready
Other
1.69k stars 107 forks source link

(Issue #16) Variable change notification support. #35

Closed asolove closed 10 years ago

asolove commented 11 years ago
cacaodev commented 11 years ago

Correct me if i am wrong but it seems that this change assumes the variable name is unique and i don't think it's a requirement when you create a Variable. In a UI environment, it would be useful to have the prefix property referring to the element that owns the variables (DOM Element or whatever) and the name property referring to a given property of this element (left, top, width, height).

Also, why don't we let the user decide what changes should be delivered and eventually group them given their element.

Something like this in SimplexSolver.js

// ..... in SimplexSolver.js
var changes = {};

if (newValue !== v.value)
{
    v.value = newValue;
    this.onvaluechange(v, changes);
}
// .....
this.onsolved(changes);

... and you init the solver like this:


solver.onvaluechange = function(variable, changes)
{
    var prefix = variable._prefix,
         changesByPrefix = changes[prefix];

    if (typeof changesByPrefix == 'undefined')
    {
        changesByPrefix = {};
        changes[prefix] = changesByPrefix;
    }

    changesByPrefix[variable.name] = variable.value;
}

solver.onsolved = function(changes)
{
    for (var prefix in changes)
    {
        var changedValuesByName = changes[prefix];
        // get element identified by prefix
        // Apply the new values to element given the variables names.
    }
}
asolove commented 11 years ago

@cacaodev Thanks for the thoughtful feedback. I hadn't considered being able to use these notifications outside the simple new api, where variable name uniqueness is enforced.

I'm not sure about adding the idea of a prefix just for the event notifications. (@slightlyoff can comment if that's generally useful for the library.) Another way to get around this is to simply expose an api for getting change notifications on a specific variable, like

var total = new c.Variable({ name: "total", value: 3 });
total.on("change", function(v){ /*..*/ });

A variable event system could also be used in #17 for notifying when the constraint that is restricting a variable changes.

asolove commented 11 years ago

Ok I'm having some trouble making this into futures. When we create the future after running through parse and compile, we can immediately resolve it with the batched-up set of variable changes from the solver. But the api for this future also has the curious property that, when .then() is called on it, it should trigger some mutation in the solver to clear out the batched-up changes.

This leads to odd behavior, and the question: if you have a future from an old solution sitting around, should calling then on it for the first time clear out batched changes that were made after this promise was created?

slightlyoff commented 10 years ago

I'm gonna merge this now and then figure something out. We need to enforce unique names for variables. Not doing so is madness.