mform icon indicating copy to clipboard operation
mform copied to clipboard

Methoden "setLabelColClass" und "setFormItemColClass" ohne Funktion

Open dpf-dd opened this issue 1 year ago • 3 comments

Folgende Anweisungen funktionieren noch mit der letzten Version von MForm 7, seit 8.0.3. werden die Anweisungen aber komplett ignoriert...

MForm 7 => korrekt

INPUT:
$form->setLabelColClass('col-sm-12 text-center');
$form->setFormItemColClass('col-sm-12 text-center');

OUTPUT:
<div class="form-group ">
    <div class="col-sm-12 text-center">...</div>
    <div class="col-sm-12 text-center">...</div>    
</div>

MForm 8 => nicht korrekt

INPUT:
$form->setLabelColClass('col-sm-12 text-center');
$form->setFormItemColClass('col-sm-12 text-center');

OUTPUT:
<div class="form-group">
    <div class="col-sm-2 control-label">...</div>
    <div class="col-sm-10">...</div>    
</div>

Affected versions / Verwendete Versionen REDAXO: 5.17.1 PHP: 8.3.8 Database: MariaDB 10.6.18 Browser: alle AddOns: MForm 8.0.3

dpf-dd avatar Aug 05 '24 11:08 dpf-dd

Irgendwie kam mir das Problem bekannt vor... :D

https://github.com/FriendsOfREDAXO/mform/issues/316

Hier ist es! Der Fehler wurde mit MForm 7.4.3 "eingeführt". Die Fragment-Datei "mform_default.php" hat sich in Version 8 etwas verändert und der Fix aus 7.x fehlt...

Ich würde hier auch gerne einen PR beisteuern, aber ich verstehe den Code nicht zu 100%... Mir erschließt sich das Dasein des Switches nicht wirklich. setLabelColClass und setFormItemColClass werden direkt übergeben, aber von $this->type gelesen und ich weiß nicht wie tief dieses $this->type in der Programmlogik verbaut ist... Wo bzw. wie kann man denn die drei Values "default, default_full, default_custom_full" einstellen/ansteuern/bearbeiten?

switch ($this->getVar('type')) {
    default:
    case 'default':
        $this->setVar('labelColClass', 'col-sm-2 control-label');
        $this->setVar('formItemColClass', 'col-sm-10');
        break;
    case 'default_full':
    case 'default_custom_full':
        $this->setVar('labelColClass', 'col-sm-12');
        $this->setVar('formItemColClass', 'col-sm-12');
        break;
}

dpf-dd avatar Aug 06 '24 08:08 dpf-dd

Es gibt eine ->setFull() Methode mit der oder mit dem Attribute 'full' => true kann man den Type auf "default_full" setzen.

joachimdoerr avatar Aug 07 '24 18:08 joachimdoerr

Dann wäre die Frage, was hat Vorrang bei folgender Konstellation bzw. wie soll dann so eine "Doppelanweisung" behandelt werden?

$form->addTextField("$id.0.meinfeld");
$form->setLabel('Mein Feld');
$form->setFull();
$form->setLabelColClass('col-sm-12 text-center');
$form->setFormItemColClass('col-sm-12 text-center');

dpf-dd avatar Aug 08 '24 06:08 dpf-dd

Und täglich grüßt das Murmeltier :(

Es funktioniert schon wieder nicht / MForm 8.0.4:

$form->setLabelColClass('col-sm-6 text-center');
$form->setFormItemColClass('col-sm-6 text-center');

wird komplett ignoriert. Der Hotfix von @skerbis funktioniert, ist aber leider nicht für 8.x... https://github.com/FriendsOfREDAXO/mform/pull/317

Problem: $this->getVar('labelColClass') enthält nicht das, was ich mit $form->setLabelColClass('col-sm-6 text-center'); mitgebe. Der Aufruf führt dazu, dass MForm $this->getVar('type') auf "default_custom_full" setzt (statt "default"). Deswegen biegt mform_default.php an folgender Stelle technisch richtig, aber logisch falsch ab.

switch ($this->getVar('type')) {
    default:
    case 'default':
        $this->setVar('labelColClass', 'col-sm-2 control-label');
        $this->setVar('formItemColClass', 'col-sm-10');
        break;
    case 'default_full':
    case 'default_custom_full':
        $this->setVar('labelColClass', 'col-sm-12');
        $this->setVar('formItemColClass', 'col-sm-12');
        break;
}

Ich würde sagen, rein vom Quelltext her liefert "setLabelColClass" und "setFormItemColClass" aktuell NIEMALS den mitgelieferten Wert sondern immer das hart codierte "col-sm-12".

Lösung wäre, die switch-Anweisung in der mform_default.php entsprechend zu erweitern...

switch ($this->getVar('type')) {
    default:
    case 'default':
        $this->setVar('labelColClass', 'col-sm-2 control-label');
        $this->setVar('formItemColClass', 'col-sm-10');
        break;
    case 'default_full':
    case 'default_custom_full':
        $this->setVar('labelColClass', 'col-sm-12');
        $this->setVar('formItemColClass', 'col-sm-12');
        break;
    case 'default_custom':
        ...
        break;
}

Für mich ist das eindeutig ein Bug!?

EDIT: Bis zur finalen Lösung funktioniert nun folgender Fix in der mform_default.php:

<?php
/** @var rex_fragment $this */

switch ($this->getVar('type')) {
    default:
    case 'default':
        $this->setVar('labelColClass', 'col-sm-2 control-label');
        $this->setVar('formItemColClass', 'col-sm-10');
        break;
    case 'default_full':
    case 'default_custom_full':
        $this->setVar('labelColClass', 'col-sm-12');
        $this->setVar('formItemColClass', 'col-sm-12');
        break;
    case 'default_custom':
        ### => kein Override nötig, es wird automatisch der mitgegebene Wert verwendet
        ### $this->getVar('labelColClass') enthält bereits den korrekten Wert und muss hier nicht neu geschrieben werden;
        break;
    
}

dpf-dd avatar Jan 28 '25 10:01 dpf-dd

der fix wurde ja gemergt, deswegen close.

joachimdoerr avatar Mar 17 '25 07:03 joachimdoerr