[JS/CSS] Inklapbare fieldsets werken niet goed in Fx

[cyberstalker]

Ik heb een functie geschreven die het mogelijk maakt fieldsets in en uit te klappen in een pagina. Om onverklaarbare redenen gaat dit fout in Firefox (in bijvoorbeeld opera werkt het perfect). Wanneer je een fieldset in/uitklapt verdwijnt bij die fieldset het legend element en verschijnt het weer voor de andere fieldsets. Ik heb de volgende code geschreven:

JavaScript:

window.addEventListener('load', init, false);

function init()
{
        var fieldsets   =       document.getElementsByTagName('fieldset');
        var legends     =       document.getElementsByTagName('legend');
        var i;

        for (i = 0; i < fieldsets.length; i++)
        {
                fieldsets[i].setAttribute('class', 'expanded');
        }

        for (i = 0; i < legends.length; i++)
        {
                legends[i].addEventListener('click', expand_collapse, false);
        }
}

function expand_collapse(event)
{
        var legend      =       event.target;
        var fieldset    =       getParentNode(legend, 'fieldset');

        var class       =       fieldset.getAttribute('class') == 'expanded'    ?       'collapsed'     :       'expanded';
        fieldset.setAttribute('class', class);
}

function getParentNode(src_el, node_type)
{
        while (src_el.nodeName != node_type)
        {
                src_el  =       src_el.parentNode;
        }

        return src_el;
}

Cascading Stylesheet:

fieldset.collapsed *
{
        display: none !important;
}

fieldset.collapsed legend
{
        display: block !important;
}

fieldset.collapsed legend:before
{
        content: '+ ';
}

fieldset.expanded legend:before
{
        content: '- ';
        font-weight: bold;
}

Testcase is te bekijken op http://www.xs4all.nl/~zeep10/got/index.xhtml

[Victor]

Cascading Stylesheet:

fieldset.collapsed
{
    height: 40px;
}

fieldset.collapsed *
{
    display: none;
}

fieldset.collapsed legend
{
    display: block !important;
}

fieldset.collapsed legend:before
{
    content: '+ ';
}

fieldset.expanded legend:before
{
    content: '- ';
    font-weight: bold;
}

Even de height van de fieldset afstemmen op de height van je legend en je bent er.

[cyberstalker]

Dat is nog eens een snelle service. Bedankt . Ik vind het overigens nog steeds vreemd.

[Victor]

cyberstalker schreef op zondag 11 maart 2007 @ 21:53:
Dat is nog eens een snelle service. Bedankt . Ik vind het overigens nog steeds vreemd.

Oe, een nog betere oplossing:

Cascading Stylesheet:

fieldset.collapsed
{
    padding-top: 0;
    padding-bottom: 0;
}

I.p.v. de height.

En al is het een quirk, laat Fx ook eens, IE mag het ook

[crisp]

Even wat algemene opmerkingen over je code. Hoe netjes het er ook uitziet, toch denk ik dat je het met wat kleine aanpassingen een stuk robuuster en dus meer universeel toepasbaar kan maken (IE even terzijde uiteraard ):

JavaScript:

window.addEventListener('load', init, false);

addEventListener is by spec ongedefinieerd voor window, gebruik dus document hier.

JavaScript:

var fieldsets   =       document.getElementsByTagName('fieldset');
var legends     =       document.getElementsByTagName('legend');

Hoewel de specificaties voorschrijven dat een fieldset altijd een legend moet bevatten, en een legend enkel gebruikt kan worden binnen een fieldset maak je op deze manier wel de aanname dat je (X)HTML altijd valid is. Ik zou persoonlijk toch puur eerst alle fieldsets pakken, kijken of die een legend-element bevat en dan pas je class en event toekennen.

JavaScript:

fieldsets[i].setAttribute('class', 'expanded');

oeh, setAttribute gebruiken voor een extension; vziw is dat ook undefined behavior...
En wat als je fieldset al een class heeft? Dan overschrijf je 'm hier dus vrolijk...

JavaScript:

var legend      =       event.target;

event.target verwijst niet noodzakelijk naar je legend-element. Het is veiliger om gewoon het 'this'-keyword te gebruiken.

En last but not least: je getParentNode functie: wat als je hier nu eens een element instopt dat geen parent heeft met de betreffende nodeName (ook zeer verwarrend dat je hiervoor een var gebruikt met als naam node_type). Note ook dat nodeName in HTML uppercased is.

[cyberstalker]

crisp schreef op zondag 11 maart 2007 @ 23:17:
Even wat algemene opmerkingen over je code. Hoe netjes het er ook uitziet, toch denk ik dat je het met wat kleine aanpassingen een stuk robuuster en dus meer universeel toepasbaar kan maken (IE even terzijde uiteraard ):

JavaScript:

window.addEventListener('load', init, false);

addEventListener is by spec ongedefinieerd voor window, gebruik dus document hier.

Dat wist ik niet. Bij deze aangepast . Wat ik overigens wel komisch vind is dat googlen naar window.addEventListener bijna twee keer zoveel resultaten oplevert als zoeken naar document.addEventListener

JavaScript:

var fieldsets   =       document.getElementsByTagName('fieldset');
var legends     =       document.getElementsByTagName('legend');

Hoewel de specificaties voorschrijven dat een fieldset altijd een legend moet bevatten, en een legend enkel gebruikt kan worden binnen een fieldset maak je op deze manier wel de aanname dat je (X)HTML altijd valid is. Ik zou persoonlijk toch puur eerst alle fieldsets pakken, kijken of die een legend-element bevat en dan pas je class en event toekennen.

Daar heb je gelijk in. Ik heb deze code ook maar even snel geschreven, en in mijn eigen XHTML-pagina's werken ze prima (ik heb het W3C hoog in het vaandel staan ).

JavaScript:

fieldsets[i].setAttribute('class', 'expanded');

oeh, setAttribute gebruiken voor een extension; vziw is dat ook undefined behavior...

Wat bedoel je precies met extension? 'class' is toch een attribuut van dat element? Is dat niet precies waar de setAttribute functie voor is bedoelt?

En wat als je fieldset al een class heeft? Dan overschrijf je 'm hier dus vrolijk...

Daar heb je inderdaad een punt. Ik gebruik in mijn code eigenlijk bijna nooit classes, daar ik de stijl uit de context laat afhangen. Dit ga ik wel aanpassen nog.

Waarschijnlijk gewoon de huidige class splitten op ' ', checken of daar 'collapsed' of 'expanded' in voorkomt, dat element wijzigen en dan weer imploden, daar kom ik wel uit denk ik .

JavaScript:

var legend      =       event.target;

event.target verwijst niet noodzakelijk naar je legend-element. Het is veiliger om gewoon het 'this'-keyword te gebruiken.

Hoe bedoel je dit precies? Bedoel je wanneer het legend element nog andere childnodes heeft dan de enkele verwachte textNode? Een andere reden hiervoor zou ik namelijk niet kunnen bedenken. En treedt hetzelfde probleem dan niet op bij het gebruik van 'this' wanneer er meerdere elementen in de legend staan?

En last but not least: je getParentNode functie: wat als je hier nu eens een element instopt dat geen parent heeft met de betreffende nodeName

Dan gaat het mis inderdaad. Een extra checkje zou hier gewenst zijn.

(ook zeer verwarrend dat je hiervoor een var gebruikt met als naam node_type).

node_name zou inderdaad logischer zijn.

Note ook dat nodeName in HTML uppercased is.

Is ook bekend. Ik zou hier natuurlijk een toLowerCase() omheen kunnen zetten, dan werkt het altijd. Ik gebruik echter altijd XHTML, waarin dit prima werkt, en een toLowerCase() zou de boel enkel langzamer maken.

[ Voor 3% gewijzigd door cyberstalker op 12-03-2007 14:27 ]

[crisp]

cyberstalker schreef op maandag 12 maart 2007 @ 14:24:
[...]
Dat wist ik niet. Bij deze aangepast . Wat ik overigens wel komisch vind is dat googlen naar window.addEventListener bijna twee keer zoveel resultaten oplevert als zoeken naar document.addEventListener

Implementaties volgen niet altijd specificaties, en de aanname dat het zo hoort is snel gemaakt. Ik zie het overigens wel gebeuren dat de specificatie op dit punt nog eens aangepast gaat worden en deze methods extenden naar window voor ECMAScript bindings

Wat bedoel je precies met extension? 'class' is toch een attribuut van dat element? Is dat niet precies waar de setAttribute functie voor is bedoelt?

My bad; class is inderdaad gewoon CDATA en een 'normaal' attribuut dat niet puur bedoelt is als extension naar CSS. Punt is wel dat setAttribute('class', 'foo') weer niet werkt in IE omdat die wil dat je daar className gebruikt (wat weer niet volgens standaard is en dus niet werkt in echte browsers).

Daar heb je inderdaad een punt. Ik gebruik in mijn code eigenlijk bijna nooit classes, daar ik de stijl uit de context laat afhangen. Dit ga ik wel aanpassen nog.

Waarschijnlijk gewoon de huidige class splitten op ' ', checken of daar 'collapsed' of 'expanded' in voorkomt, dat element wijzigen en dan weer imploden, daar kom ik wel uit denk ik .

Niet puur splitten op spaties, maar op whitespace in het algemeen
Ik gebruik tegenwoordig dit:

JavaScript:

function trim(string)
{
    return string.replace(/^\s+|\s+$/, '');
}

function hasClass(el, classname)
{
    return new RegExp('(^|\\s)' + classname + '(\\s|$)').test(el.className);
}

function addClass(el, classname)
{
    el.className = trim(el.className + ' ' + classname);
}

function removeClass(el, classname)
{
    el.className = trim(el.className.replace(new RegExp('(^|\\s)' + classname + '(\\s|$)'), ' '));
}

Maar ik heb ook nog een meer generieke classdealer library. Ik moet ooit nog maar eens kijken welke qua performance beter is...

Hoe bedoel je dit precies? Bedoel je wanneer het legend element nog andere childnodes heeft dan de enkele verwachte textNode? Een andere reden hiervoor zou ik namelijk niet kunnen bedenken. En treedt hetzelfde probleem dan niet op bij het gebruik van 'this' wanneer er meerdere elementen in de legend staan?

Niet enkel element childNodes (wat in het geval van een legend-element natuurlijk niet van toepassing kan of mag zijn) maar ook bijvoorbeeld een absoluut gepositioneerd element boven de legend.
'this' verwijst altijd naar het element dat in scope is - oftewel het element dat de handler gedefinieerd heeft. Itt event.target wat verwijst naar het element dat het event triggerde.