Antwort an „Rolf B“ verfassen

Hallo MudGuard,

da ist noch mehr zu viel. Dafür fehlt anderes.

  • den input-Elementen fehlt das zwingend erforderliche Label. Es ist zwar theoretisch da, aber nicht zugänglich.

  • align für input - gab es das überhaupt einmal? Aber auch auf dem td wäre es missbilligt und sollte per CSS (text-align) festgelegt werden.

  • bgcolor für tr – diese Eigenschaft ist missbilligt. Statt dessen soll man
    style="background-color: <?= $valuerangeArray[0]->bgcolor ?>"
    schreiben. Sofern es denn sinnvoll ist, die Farben individuell pro Row oder Zelle zu vergeben. Aber das kann man aus dem gezeigten Code schlecht beurteilen und es hängt von der Variabilität der Farben ab, ob man es mit Klassen versuchen sollte.

  • cellspacing="2" border="2" cellpadding="3" align="center" frame="box" für das table-Element: Muss alles weg, ist alles HTML 3 Relikt oder so, das gehört ins CSS. Man gebe der Table eine Klasse und ordne dann im CSS für Tables mit dieser Klasse zu:

    • align="center" zentriert die ganze Tabelle, das sollte sich als margin-inline:auto für das table-Element abbilden lassen
    • cellspacing="2" ist border-spacing: 2px auf dem table-Element
    • cellpadding="3" möchte padding: 3px auf den th/td Elementen sein
    • frame="box" und border="2" wird als "border: 2px solid gray" auf dem table-Element und als "border: 1px solid black" auf de td Elementen festgelegt (Farbe nach Belieben - bei Chrome ist die border-color für Tables per Default gray)
  • Das Innere der For-Schleife sollte besser wieder HTML-Template sein. So wie auch für das table Element. Dito für </table>.

  • Mich deucht, das tr vor der Schleife und die tr in der Schleife unterscheiden sich lediglich im checked Attribut. Um Redundanz zu vermeiden, würde ich dieses tr in die Schleife ziehen und auf $i==0 testen. Hier bietet sich der Auswahloperator ?: an (auch ternärer Operator genannt).

  • Ich würde der Übersichtlichkeit wegen auch versuchen, das Innere des if ($switch) Blocks in eine Funktion auszulagern.

if ($switch) {
   renderValueRanges($valuerangeArray);
}

...

function renderValueRanges($valueRangeArray) {
?>
  <table class="value_ranges">
<?php
  for ($i=0; $i < count($valuerangeArray); $i++):
    $id = $valuerangeArray[0]->varname;
    $text = $valuerangeArray[0]->text;
?>
    <tr style="background-color:<?= $valuerangeArray[0]->bgcolor ?>">
      <td>
        <input type='radio' id="<?= $id ?>" value="0" <?= $i==0 ? "checked" : "" ?>>
      </td>
      <td>
        <label for="<?= $id ?>"><?= $text ?></label>
      </td>
		</tr>
<?php
   endfor;
?>
  </table>
<?php
}

Die Variablen $id und $text sind nicht unbedingt nötig und Puristen würden sagen, die braucht man nicht. Ich finde, sie verbessern die Lesbarkeit und verhindern kilometerlange Codezeilen.

Die Klasse beschriftung_radio_button, die ja ohnehin schon "LABEL" geschrieen hat, sollte im CSS mit einem geeigneten Selektor ersetzt werden. Möglichkeiten wären:

  • .value_ranges td:nth-of-type(2)
  • .value_ranges td:has(label)
  • oder direkt das label formatieren: .value_ranges label

Je nach Bedarf und Belieben.

Zu prüfen wäre auch noch, ob die Radiobuttons außer der ID auch noch ein name-Attribut brauchen. Nur mit id werden sie in einem Form nicht gepostet, wenn mich die Erinnerung nicht trügt.

Rolf

--
sumpsi - posui - obstruxi
freiwillig, öffentlich sichtbar
freiwillig, öffentlich sichtbar
freiwillig, öffentlich sichtbar

Ihre Identität in einem Cookie zu speichern erlaubt es Ihnen, Ihre Beiträge zu editieren. Außerdem müssen Sie dann bei neuen Beiträgen nicht mehr die Felder Name, E-Mail und Homepage ausfüllen.

abbrechen