JavaScript für Gallery - bitte kommentieren

[Alex]

Hallo!

Zur Zeit verwende ich ein Script[1], das für eine Gallery gedacht ist. Leider bin ich nicht so viel der ultimative JavaScript Guru. Deshalb folgende Bitte:

Kann sich das jemand anschaun und Verbesserungsvorschläge machen? Eventuell sind da noch Denkfehler oder tote Leichen drinnen. Vielleicht mag das der eine oder andere Browser sogar aus irgendeinem Grund überhaupt nicht. Wer weiß...

Herzliches Danke und natürlich frohes Fest!
Grüße aus Österreich.

[1]http://www.schandblatt.at/gallery/gallery.js

[Klawischnigg]

Hi there,

Eventuell sind da noch Denkfehler oder tote Leichen drinnen.

Dieser Pleonasmus war unter Garantie ein Denkfehler.

Vielleicht mag das der eine oder andere Browser sogar aus irgendeinem Grund überhaupt nicht. Wer weiß...

Deine try - catch Routine wird von älteren Browsern vermutlich nicht verstanden werden und einen Fehler verursachen. Sonst sieht die ganze Geschichte zu einfach aus, um wesentliche Fehler enthalten zu können.

[Der Dicki]

Hallihallo!

Eventuell sind da noch Denkfehler oder tote Leichen drinnen.

Dieser Pleonasmus war unter Garantie ein Denkfehler.

Oder ein Hinweis für die Suche?
Ich zumindest habe zwei "tote Leichen" gefunden:

Und zwar in Zeile 37 und 75, in denen es heisst:

  
if ( preCount >= (imgArray.length-1) )  

~~~, wobei jedoch imgArray.length auf jeden Fall gleich "imgnum" sein muss (Zeile 23).  
  
Ausserdem ist mir aufgefallen, daß in Zeile 50 und 51 ein neues Bildobjekt angelegt wird, anschliessend aber nur dessen src-Attribut benutzt wird. Wenn mich nicht Alles täuscht, kann man die src- Zuweisung auch in Zeile 58 unterbringen:  
~~~javascript
  
window.document.images[imgname].src = imgArray[count];  

Leider habe ich im Moment nicht die Möglichkeit, an dem Skript rumzutesten (nur einen IE 6 zur Hand), aber als Schuss ins Blaue: Kann man evtl. den try-catch- Block weglassen, wenn man statt "window.document.images" einfach nur "document.images" schreibt?

Viele liebe Grüße,
Der Dicki

[Alex]

Ich zumindest habe zwei "tote Leichen" gefunden:

Danke für's Suchen.

Und zwar in Zeile 37 und 75, in denen es heisst:

if ( preCount >= (imgArray.length-1) )

Stimmt. count/preCount können nie drüber gehen, also sollte "preCount == (imgArray.length-1)" eher stimmen. Am ehesten logisch wäre aber "count == (imgnum-1)".  
Ansonsten passt das schon so, da count ja bei null anfängt und das erste Bild aber Bild1 und nicht Bild0 ist. Ich finde das so logischer, als bei Bild0 anzufangen. Eigentlich könnte ich den counter aber bei eins anfangen lassen. Oder den Array erst bei 1 füllen. Das wäre dann am aller logischten....ok ich hör' schon auf.  
  
Hab das jetzt ein bisschen angepasst. Es gibt nun ein erstes und ein letztes Bild. Damit ist der Array zwar nicht mehr optimal ausgelastet, aber das ist in der heutigen Zeit nun wirklich egal, denke ich.  
  
  

> Ausserdem ist mir aufgefallen, daß in Zeile 50 und 51 ein neues Bildobjekt angelegt wird, anschliessend aber nur dessen src-Attribut benutzt wird.  

Ja, das ist ein dummer Hack, den ich mir zu allem Überfluss auch noch irgendwie zerstört hatte. Der Wrapper div soll sich an die Größe des Bildes anpassen, ohne dass ich diese Werte zuerst händisch eingebe.  
  
Was passiert aber, wenn ich ein Bild gar nicht laden kann? Dann wird's kompliziert, es sollte ja ein Fehler angezeigt werden. Vielleicht klappt das mit der neuen Version. Zumindest der Firefox versteht das und setzt die Attribute des Bildes zurück. Der IE zeigt aber ein Fehlerbild an und das hat die Breite 28.  
  
Weiß da jemand Rat?  
  
  

> Kann man evtl. den try-catch- Block weglassen, wenn man statt  
> "window.document.images" einfach nur "document.images" schreibt?  

Geht hier nicht. Der ganze Block ist aber mittlerweile sowieso weg. Hat ja eh nicht funktioniert.  
  
  
Das Script ist jetzt eine Spur schöner, wenn man bei Code überhaupt von "schön" sprechen kann.  
  
  
lg, und frohe Feiertage,  
Alex

[Der Dicki]

Hallihallo!

Ich zumindest habe zwei "tote Leichen" gefunden:
Danke für's Suchen.

Gern geschehen ;)

Und zwar in Zeile 37 und 75, in denen es heisst:

if ( preCount >= (imgArray.length-1) )

> Stimmt. count/preCount können nie drüber gehen, also sollte "preCount == (imgArray.length-1)" eher stimmen. Am ehesten logisch wäre aber "count == (imgnum-1)".  

Eigentlich meinte ich nur, daß man hier das "imgArray.length" durch "imgnum" ersetzen kann, weil es genau dadurch definiert wurde: ( imgArray = new array(imgnum); )  
  
  

> Hab das jetzt ein bisschen angepasst. Es gibt nun ein erstes und ein letztes Bild. Damit ist der Array zwar nicht mehr optimal ausgelastet, aber das ist in der heutigen Zeit nun wirklich egal, denke ich.  

Da komme ich jetzt grade nicht mit...  
  
  

> > Ausserdem ist mir aufgefallen, daß in Zeile 50 und 51 ein neues Bildobjekt angelegt wird, anschliessend aber nur dessen src-Attribut benutzt wird.  
> Ja, das ist ein dummer Hack, den ich mir zu allem Überfluss auch noch irgendwie zerstört hatte. Der Wrapper div soll sich an die Größe des Bildes anpassen, ohne dass ich diese Werte zuerst händisch eingebe.  

hmm... Das Anpassen des divs an die Größe des Bildes sollte doch eigentlich, falls Du nicht mit absoluten Positionierungen hantierst, automatisch erfolgen, oder?  
  
  

> Was passiert aber, wenn ich ein Bild gar nicht laden kann? Dann wird's kompliziert, es sollte ja ein Fehler angezeigt werden. Vielleicht klappt das mit der neuen Version. Zumindest der Firefox versteht das und setzt die Attribute des Bildes zurück. Der IE zeigt aber ein Fehlerbild an und das hat die Breite 28.  

Das ist ja mal ein Luxus, daß der Feuerfuchs das src- Attribut wieder verwirft :)  
Für das Problemkind Internet Explorer und vielleicht auch ein paar andere hilft Dir vielleicht [die Eigenschaft complete](http://de.selfhtml.org/javascript/objekte/images.htm#complete) weiter. Ist einen Versuch wert, denke ich...  
  

> lg, und frohe Feiertage,  

Viele liebe Grüße, und Dir auch schöne Feiertage,  
Der Dicki

[Alex]

Da komme ich jetzt grade nicht mit...

Es gibt in der neuen Version 2 Variablen. Eine für die Nummer, mit der die Bilder beginnen (also meistens 0 oder 1) und eine für die Nummer des letzten Bildes.

hmm... Das Anpassen des divs an die Größe des Bildes sollte doch eigentlich, falls Du nicht mit absoluten Positionierungen hantierst, automatisch erfolgen, oder?

Da muss ich nochmal reinschaun, wie ich das Designtechnisch gelöst habe....

Für das Problemkind Internet Explorer und vielleicht auch ein paar andere hilft Dir vielleicht die Eigenschaft complete weiter.

Die ist leider bei meinen Tests beim IE immer "true" gewesen. Also gibt's offensichtlich keine Möglichkeit, das zu überprüfen, oder ich bin einfach nur zu dumm dazu.

[Struppi]

Für das Problemkind Internet Explorer und vielleicht auch ein paar andere hilft Dir vielleicht die Eigenschaft complete weiter.
Die ist leider bei meinen Tests beim IE immer "true" gewesen. Also gibt's offensichtlich keine Möglichkeit, das zu überprüfen, oder ich bin einfach nur zu dumm dazu.

Doch gibt es, onload und onerror, wie ich bereits schrieb.

Struppi.

[Struppi]

Kann sich das jemand anschaun und Verbesserungsvorschläge machen? Eventuell sind da noch Denkfehler oder tote Leichen drinnen. Vielleicht mag das der eine oder andere Browser sogar aus irgendeinem Grund überhaupt nicht. Wer weiß...

Einmal halte ich das für nicht sinnvoll eigene Arraygrenzen zu definieren, ist auch überflüssig.

so bekommst du ein sauberes Array mit deinen Bildern:

do  
{  
    imgArray[imgArray.length] = prefix + count + suffix;  
}while(imgLast > count++ )  

Dann musst du den onbload bzw. onerror Handler benutzen um das Bild anzuzeigen. Auch wenn der nicht dokumentiert ist funktioniert der (bis ein kleines Problem im IE, der den Event nicht feuert, wenn das Bild im Cache ist.

function changeImage(dir)  
{  
    var count = getCounter( dir, count);  
  
    var newImg = new Image();  
    newImg.onload = function()  
    {  
        document.getElementById(imgdesc).firstChild.nodeValue = imgPrefix + count + imgSuffix;  
        document.getElementById(imgname).style.display =  "";  
        document.getElementById(imghold).style.height = this.height + 2 + 18 + "px";  
        document.getElementById(imghold).style.width = this.width + 2 + "px";  
    };  
    newImg.onerror = function()  
    {  
        document.getElementById(imgdesc).firstChild.nodeValue = imgError;  
        document.getElementById(imgname).style.display =  "none";  
        document.getElementById(imghold).style.height = "18px";  
    };  
    newImg.src = imgArray[count];  
    if(document.all && newImg.complete) newImg.onload();  
  
    preImg = new Image();  
    count = getCounter( dir, count);  
    preImg.src = imgArray[count];  
}  

Der Counter kann dann so weiter gezählt werden:

function getCounter(dir, count)  
{  
    count += dir ? 1 : -1;  
    if(count < 0 ) count = imgArray.length - 1;  
    else if( count >= imgArray.length) count = 0;  
    return count;  
}  

(ungetestet, da ich die Bilder nicht hab)

Struppi.

[Mathias Brodala]

Hallo Struppi.

Dann musst du den onbload […] Handler benutzen […]

Ziemlich aufgeblasen.

Einen schönen Montag noch.

Gruß, Math*scnr*ias

[Alex]

Boah. Herzlichen Dank. Das waren genau die Anregungen, die ich gebraucht habe.

OnError funzt im Firefox bei mir hier nicht, dafür habe ich den alten Hack mit .width einfach wieder eingebaut. Vielleicht habe ich auch nur etwas falsch verstanden.

Egal. Super fand ich auch den Vorschlag zu getCounter. Da hab ich ja schon einen boolean für eine inlineIf und "vergesse" tatsächlich, sie zu benützen. Sieht jetzt viel besser aus.

Sollte jemand das Ding in Action sehen wollen, bitte hier kucken:
http://whf.at/rundgang.html

Soll aber wirklich keine Werbung sein.

[Struppi]

OnError funzt im Firefox bei mir hier nicht, dafür habe ich den alten Hack mit .width einfach wieder eingebaut. Vielleicht habe ich auch nur etwas falsch verstanden.

Ja, es heißt onerror JS unterscheidet zwischen Groß und Kleinschreibung.

Struppi.

[Alex]

Ja, es heißt onerror JS unterscheidet zwischen Groß und Kleinschreibung.

Und ich benütze auch nichts anderes in meinem Script. Aber egal, so, wie es jetzt ist, funzt es ja eh im IE und FF

[Struppi]

Und ich benütze auch nichts anderes in meinem Script. Aber egal, so, wie es jetzt ist, funzt es ja eh im IE und FF

würd ich nicht drauf wetten.
...
if(document.all && newImg.complete) newImg.onload();
if(newImg.width == 0) newImg.onerror();
...
Damit rufst immer die Error Funktion auf, ausser das Bild ist bereits im Cache. Ansonsten sollte es funktioniere.

Struppi.