1

enter code hereI am trying to make something editable online with a function like this

function toggle_editable (div, cssclass) {

var classToEdit = document.getElementsByClassName(cssclass)

    for (i = 0;classToEdit.length; i++) {

        if (classToEdit[i].contentEditable == false) {
            classToEdit[i].contentEditable = true ;
        }
        if (classToEdit[i].contentEditable == true) {
            classToEdit[i].contentEditable = false ;
        }
    }
}

classToEdit is a collection of HTML elements with the same class name or whatever document.getElementsByClassName(cssclass) returns

when going through the debugger it jumps over the line

classToEdit[i].contentEditable == true

as well as over the line

 classToEdit[i].contentEditable == true

and does not execute the code in the braces following the if statements

this works however - meaning it sets the contenteditable property without hesitation

classToEdit.contenteditable = true;

as well as this

classToEdit.contenteditable = false;

(well obviously)

also this seemed to have no effect

classToEdit.contenteditable = !classToEdit.contenteditable 

ideas anyone?

ps why is the loop

  • In addition to the first answer, you also need `else if` instead of just `if`… – Matijs Dec 26 '14 at 22:32
  • Again, if `classToEdit` is a HTMLCollection, setting `classToEdit.contenteditable = true` definitely doesn't change the `contentEditable` value of an element. – Teemu Dec 26 '14 at 22:35
  • Also, if you want to toggle the value of contentEditable, you could do so using `element.contentEditable = !(element.contentEditable);`. No need for an `if` at all. – Matijs Dec 26 '14 at 22:40

2 Answers2

0

You've created an infinite loop here:

for (i = 0;classToEdit.length; i++) {

Should be:

for (var i = 0; i < classToEdit.length; i++) {

But, if you say classToEdit.contenteditable = true "works", you've to define "not working/is working" since the snippet doesn't definitely do what you expect it to do, if classToEdit is a HTMLCollection.

It looks like you'd want to toggle contentEditable values, you can do it like this:

for (var i = 0; i < classToEdit.length; i++) {
    if (classToEdit[i].contentEditable == false) {
        classToEdit[i].contentEditable = true ;
    } else { // Notice else here, no need for another check
        classToEdit[i].contentEditable = false;
    }
}

Or simply without ifs in the loop:

classToEdit[i].contentEditable = !classToEdit[i].contentEditable;

Your current code will switch the value back to it's original in a case the value was false.

Teemu
  • 22,918
  • 7
  • 53
  • 106
  • Worked - thanks for your support and to teemu for the infinite loop note. – Kiril Iliev Dec 26 '14 at 22:40
  • @KirilIliev Well, that might have been a typo in the post only, but I mentioned it just in case it isn't a typo. An answer to the "ps" question you can find from the documentation of [getElementsByClassName](https://developer.mozilla.org/en-US/docs/Web/API/document.getElementsByClassName) – Teemu Dec 26 '14 at 22:53
  • 1
    Also note `i` should be declared. – Oriol Dec 26 '14 at 23:05
0

HTMLElement.contentEditable returns a string and not a boolean.

Hence, what you want to identify the state of your editable field is:

// Incorrect
classToEdit[i].contentEditable == true
// Coorect
classToEdit[i].contentEditable === 'true'

What's even better if you want to know the state of your fields is to use HTMLElement.isContentEditable which returns a boolean:

classToEdit[i].contentEditable = !element.isContentEditable

Another way to refactor the above:

function toggleContentEdit() {
    var editableFields = document.getElementsByClassName('editable');

    [].forEach.call(editableFields, function(field){
        var isEditable = field.isContentEditable;
        field.setAttribute('contenteditable', !isEditable);
    });
};

JSFiddle: http://jsfiddle.net/6qz3aotv/

Hady
  • 2,597
  • 2
  • 29
  • 34
  • `Element.contentEditable` is the same as using `get/setAttribute('contenteditable')`, see [MDN](https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement.contentEditable) – Matijs Dec 26 '14 at 22:36
  • That's interesting, booleans have always worked for me. – Teemu Dec 26 '14 at 23:05
  • Yeah - I always thought javascript was a C++ with loose rules - it turns out it has stricter rules and not so straightforward typecasting :) – Kiril Iliev Dec 27 '14 at 09:06