CSS - JS делает ссылки бесполезными - PullRequest
Новая
       17

CSS - JS делает ссылки бесполезными

0 голосов
/ 12 января 2010

Хорошо, хорошо, благодаря людям здесь, я выяснил, какая часть моего js-кода делает мои ссылки бесполезными, это то, что вставлено ниже ... Все еще пытаюсь понять, почему, но если у кого-то есть понимание, это будь великим ... 

function initJumpMenus() {
    // Turns all <select> elements with the 'jumpmenu' class into jump menus
    var selectElements = document.getElementsByTagName("select");
    for( i = 0; i < selectElements.length; i++ ) {
        // Check for the class and make sure the element has an ID
        if( selectElements[i].className == "jumpmenu" &&
            document.getElementById(selectElements[i].id) != ""
        ) {
            jumpmenu = document.getElementById(selectElements[i].id);
            jumpmenu.onchange = function() {
                if( this.options[this.selectedIndex].value != '' ) {
                    // Redirect
            location.href=this.options[this.selectedIndex].value;
                }
            }
        }
    }
}

window.onload = function() {
    initJumpMenus();
}

Ответы [ 2 ]

3 голосов
/ 12 января 2010

Это: 

document.getElementById(selectElements[i].id) != ""

неправильно. Вы хотите проверить, имеет ли элемент идентификатор, поэтому просто выполните: 

selectElements[i].id != ""

Я хотел бы отметить, что вы можете улучшить свой код здесь и там:

1 Не записывать for циклов, которые получают свойство length для каждой итерации,

вместо этого: 

for( i = 0, num = selectElements.length; i < num; i++ ) {
    ...
}

Только если вы ожидаете, что selectElements будет расти или уменьшаться, имеет смысл запросить значение свойства, но в этом случае вам, вероятно, все равно не следует писать цикл for.

2: не писать nodelist[index]

Для нодлистов, таких как возвращаемые getElementsByTagName(), вы не должны писать nodelist[index] (даже если большинство браузеров поддерживают это). Стандартный метод DOM - item, поэтому вместо него пишите nodelist.item(index).

3 выборки предметов из списка только один раз

Если вам нужен элемент из списка более одного раза, сохраните его в локальной переменной. ваш цикл станет: 

for( i = 0, selectElement; i < selectElements.length; i++ ) {
    selectElement = selectElements.item(i);
    ...more code using selectElement...
}

Обратите внимание на объявление переменной selectElement в цикле for. так как вы не используете его вне цикла, его объявление избегает беспорядка и гарантирует, что если вы переместите цикл, вы переместите объявление.

4. сначала самые дешевые сравнения

Вы писали: 

selectElement.className == "jumpmenu" &&
selectElement.id != ""

это можно было бы немного улучшить, если поменять ноги: 

selectElement.id != "" &&
selectElement.className == "jumpmenu"

Это будет быстрее, так как проверять, пуста ли строка, меньше работы, а в тех случаях, когда строка пуста, мы даже не проверяем className

5 Не используйте document.getElementById(), если у вас уже есть элемент

Внутри цикла у вас есть это: 

jumpmenu = document.getElementById(selectElements[i].id);

В основном вы получаете идентификатор из selectElement и используете его для запроса документа, чтобы найти .... элемент, имеющий и id равный значению текущего selectElement. Поскольку id уникальны в документе (или должны быть), вы в основном пишете совершенно ненужное предложение. jumpmenu и selectElement относятся к одному и тому же объекту.

6. Улучшения обработчика onchange

внутри цикла вы назначаете обработчик onchange. Вы делаете это, создавая новую функцию для каждой итерации цикла. Это код обработчика: 

function() {
    if( this.options[this.selectedIndex].value != '' ) {
        // Redirect
        location.href=this.options[this.selectedIndex].value;
    }
}

Здесь следует отметить три вещи: во-первых, код обработчика onchange содержит это location.href = ..., которое, вероятно, должно быть document.location.href = ....

Во-вторых, вы дважды ссылаетесь на this.options[this.selectedIndex].value. Опять же, поместите это в локальную переменную.

В-третьих, this относится к элементу, который испытал событие onchange к моменту выполнения этой функции. Кроме this и свойств this, в этом обработчике нет переменных, которые бы исходили из цикла или внешней функции initJumpMenus. Вы должны просто создать его один раз вне цикла и назначать его для каждой итерации: 

var onchange_handler = function() {
    if( this.options[this.selectedIndex].value != "" ) {
        // Redirect
        location.href=this.options[this.selectedIndex].value;
    }
}
for (...) {
    if (...) {
        selectElement.onchange = onchange_handler;
    }
}

7 Резюме

Сложив все это вместе, я бы так написал: 

function initJumpMenus() {
    var handler = function() {
        var value = this.options.item(this.selectedIndex).value;
        if( value != "" ) {
            // Redirect
            document.location.href = value;
        }
    }
    var selectElements = document.getElementsByTagName("select");
    for(var i = 0, num = selectElements.length, selectElement; i < num; i++ ) {
        selectElement = selectElements.item(i);
        // Check for the class and make sure the element has an ID
        if( selectElement.id != "" &&
            selectElement.className == "jumpmenu"
        ) {
            selectElement.onchange = handler;
        }
    }
}
0 голосов
/ 12 января 2010

Проверьте консоль ошибок;любое исключение в JS остановит реакцию формы ссылки на клик.

...