RFC – Container::setDefaults() opravit chybu + BaseControl::setDisabled() vylepšit funkci

Ahoj,

mám návrh na vylepšení funkce Nette formulářů ve dvou místech, které spolu úzce souvisí.

Mizející $value v disablovaném prvku po submitu formuláře

Problém řeším cca 2 roky v tomto vlákně zde: https://forum.nette.org/…ba-framework

Dnes se mě podařilo chybu přesně lokalizovat a ve zdrojovém kódu Nette opravit a opravu vyzkoušet.

Chyba spočívá v tom, že pokud do formulářového prvku který je disablován setDisabled() pošleme defaultní data setDefaults() po anchor události formuláře, tak po submitu formuláře data zmizí, což je nežádoucí chování. Pokud použijeme metodu setDefaultValue() data nemizí a je to OK.

Ukázka testu ve formuláři:

$form->addText('disabled', 'Disablovaný')
    ->setDisabled()->setOmitted(false);

$form->addSubmit('send', 'Odeslat')
    ->onClick[] = $form->handleSend(...);

$form['disabled']->setDefaultValue('abcd');         //  OK
$form->setDefaults(['disabled' => 'abcd']);         //    OK

$form->onRender[] = fn() => $form['disabled']->setDefaultValue('abcd');     // OK
$form->onRender[] = fn() => $form->setDefaults(['disabled' => 'abcd']);       //  NOT OK

$form->onValidate[] = fn() => $form->addError('is_submitted');

return $form;

Při pohledu do zdrojového kódu je chyba zřejmá. Zatímco metoda BaseControl:setDefaultValue() používá pro nastavení dat podmínku:

$this->isDisabled() || !$form || !$form->isAnchored() || !$form->isSubmitted()

metoda Container::setDefaults používá jinou podmínku:

!$form || !$form->isAnchored() || !$form->isSubmitted()

PR je zde: https://github.com/…rms/pull/326

Nevhodné mazání dat z prvku při volání setDisabled()

V dokumentaci k disablování prvku je uvedeno https://doc.nette.org/…rms/controls#…, že pokud chceme zobrazit v disablovaném prvku data, musíme dodržet předepsané pořadí volání setDisabled() a setDefaultValue(), jinak se předaná data smažou. Tento požadavek je nepříjemný a matoucí, protože není úplně logický a na pořadí volání by záležet nemělo.

Proč setDisabed() maže data v prvku ? Protože to tak je ve zdrojovém kódu napsáno a v době, kdy Nette formuláře vznikaly k tomu byl nějaký důvod.

Dnes jsem se na otázku zda by se data v disablovaném prvku mazat měly nebo neměly podíval detailně. Norma HTML definuje input s html atributem disabled jako prvek, který nelze editovat, data která obsahuje zobrazuje, ale neodesílá je zpět. Tedy žádné mazání již vložených dat, data, která v prvku jsou se nemažou a zobrazí. To je žádoucí chování.

OK tak tedy mazání dat z metody setDisabled() odstraníme. Potom můžeme metody setDisabled() a setDefaultValue() volat v libovolném pořadí a funkce se nezmění. A nevznikne nám nějaký problém tím, že jsou v prvku nějaká data, která se nesmažou ? Protože když setDisabled() vznikalo tak tam nějaký důvod byl!! Data se do prvku mohou dostat dvěmi cestami:

a) setDefaults() + setDefaultValue()
b) po submitu formuláře od uživatele

V případě a) data do prvku posílá vývojář a ví proč, protože data chce zobrazit – tyto data by se mazat neměly, naopak jejich smazání vadí.

V případě b) data odesílá uživatel z formuláře – to znamená, že prvek ve fázi vykreslení formuláře (nesubmitnutý) nebyl disablován, jinak by uživatel data nemohl poslat a vývojář prvek disabluje až po submitu s použitím nějaké podmínky. Pokud vývojář nechal v první fázi prvek nedisablovaný a nechal uživatele vyplnit nějaká data, která po submitu chce dodatečně vymazat a prvek disablovat nedává to moc smysl. V takovém případě je přece logické prvek disablovat od počátku a nenechat tam data vyplnit. Pokud tato data metoda setDisabled() nesmaže ale ponechá, žádný problém tím nevznikne.

Myslím, že odstraněním mazání dat po zavolání setDisabled() žádný problém nevznikne, byť je to BC break. A použití této metody bude bez záludností a funkce 100%. Dnes je totiž stav takový, že metodu setDisabled() můžeme použít jak pro zobrazení needitovatelných dat, tak pro smazání dat z prvku – kterou funkci framework použije záleží na zmíněném pořadí volání metod. To není dobře navržené API, to by se mělo opravit.

PR je zde https://github.com/…rms/pull/327

Prosím o komentáře a připomínky k navrženým úpravám.

Editoval m.brecher (21. 4. 2:10)