views:

213

answers:

3

Writing a widget to be able to rename files by clicking on the text name and entering the new name. I didn't find any ready-to-use solutions, maybe you can point me to one?

Here is where I ended up and it doesn't work: for some reason, only the last input box is changing, and the first and second aren't referenced:

<span id="text_name_0">Hello, world. Click me please.</span>
<input type="hidden" id="name_changer_0" />
<input type="hidden" id="done_changing_0" value="Done"/>
<br/>
<span id="text_name_1">Hello, world. Click me please.</span>
<input type="hidden" id="name_changer_1" />
<input type="hidden" id="done_changing_1" value="Done"/>
<br/>
<span id="text_name_2">Hello, world. Click me please.</span>
<input type="hidden" id="name_changer_2" />
<input type="hidden" id="done_changing_2" value="Done"/>

<script type="text/javascript">

function TextChanger(id) {
 this.textNode = document.getElementById('text_name_' + id);
 this.textValue = this.textNode.firstChild.nodeValue;
 this.textboxNode = document.getElementById('name_changer_' + id);
 this.doneButton = document.getElementById('done_changing_' + id);
} 

TextChanger.prototype.change = function(node) {
    node.textboxNode.setAttribute('value', node.textValue);
    node.textNode.style.display = 'none';
    node.textboxNode.setAttribute('type','text');
    node.doneButton.setAttribute('type','button');
} 

TextChanger.prototype.changeBack = function(node) {
    node.textNode.firstChild.nodeValue = node.textboxNode.value;
    node.textNode.style.display = 'block';
    node.textboxNode.setAttribute('type', 'hidden');
    node.doneButton.setAttribute('type','hidden');
}

for (var i=0; i < 3; i++) {
  changer = new TextChanger(i);
  changer.textNode.addEventListener("click", function() {
   changer.change(changer);
  }, false);

  changer.doneButton.addEventListener("click", function() {
   changer.changeBack(changer);
  }, false);
}
</script>

Thanks.

A: 

If you don't mind the jQuery dependency, I've used jquery-in-place-editor to edit fields before.

Justin Love
Thanks - added it, works perfectly
alex
A: 

The problem is that when the listener functions added here fire, they contain references to the global variable "changer". At the time they fire the loop has already completed, and so "changer" points to the last item in the loop.

Additionally, adding listeners can be messy cross-browser, it's safer to use a library like jQuery or YUI to do so. This will also enable you to pass an object to each event listener (in a cross-browser way), so for example you could do:

for (var i=0; i < 3; i++) {
                var changer = new TextChanger(i);
                YAHOO.util.Event.addListener(changer.textNode, "click", changer.change, changer); 
                ...
Ben
thank you - now I see the reason. javascript is using 'by-ref', not 'by-value'.
alex
A: 

This is a classic loop-variable binding problem. See this question for some discussion.

Your closure is ineffective because it closes over the copy of changer in use inside the loop, which the loop will change. To bind it, you need another closure to take a copy of the current version of changer:

function changebind(c) {
    return function() {
        c.change(c);
    };
}

for (var i=0; i<3; i++) {
    var changer= new TextChanger(i);
    changer.textNode.addEventListener('click', changebind(changer), false);

(You may prefer to ditch the node argument and just use this.)

In the future (ECMAScript Fifth Edition), there will be a quicker and more efficient way to say this:

for (var i=0; i<3; i++) {
    var changer= new TextChanger(i);
    changer.textNode.addEventListener('click', changer.change.bind(changer), false);
    changer.doneButton.addEventListener('click', changer.changeBack.bind(changer), false);
}

but in the meantime, since most browsers don't support function.bind yet, you can hack that in like this:

if (!Object.bind) {
    Function.prototype.bind= function(owner) {
        var that= this;
        var args= Array.prototype.slice.call(arguments, 1);
        return function() {
            return that.apply(owner,
                args.length===0? arguments : arguments.length===0? args :
                args.concat(Array.prototype.slice.call(arguments, 0))
            );
        };
    };
}
bobince
Makes sense, thanks a lot!
alex