Skip to content

Commit 27d020a

Browse files
sukhwinder33445sukhwinder33445
authored
CompatForm: Add missing Errors decorator (#322)
If the form validation failed, the form could not be submitted and no errors were displayed because the error decorator is missing. This is why there was no indication to the user about why the submit button was not working.
1 parent dec20c2 commit 27d020a

File tree

2 files changed

+210
-26
lines changed

2 files changed

+210
-26
lines changed

‎src/Compat/CompatForm.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,12 @@
77
use ipl\Html\Contract\FormSubmitElement;
88
use ipl\Html\Contract\HtmlElementInterface;
99
use ipl\Html\Form;
10-
use ipl\Html\FormDecoration\DecoratorChain;
1110
use ipl\Html\FormElement\SubmitButtonElement;
1211
use ipl\Html\FormElement\SubmitElement;
1312
use ipl\Html\HtmlDocument;
1413
use ipl\Html\HtmlString;
1514
use ipl\I18n\Translation;
1615
use ipl\Web\FormDecorator\IcingaFormDecorator;
17-
use ipl\Html\Contract\FormDecoration;
1816
use ipl\Web\Compat\FormDecorator\LabelDecorator;
1917

2018
class CompatForm extends Form
@@ -42,6 +40,7 @@ public function applyDefaultElementDecorators(): static
4240
$this->addElementDecoratorLoaderPaths([
4341
['ipl\\Web\\Compat\\FormDecorator', 'Decorator']
4442
]);
43+
4544
$labelDecorator = new LabelDecorator();
4645
$this->setDefaultElementDecorators([
4746
'Label' => $labelDecorator,
@@ -60,6 +59,7 @@ public function applyDefaultElementDecorators(): static
6059
'Checkbox',
6160
'RenderElement',
6261
'Description',
62+
'Errors' => ['name' => 'Errors', 'options' => ['class' => 'errors']],
6363
'ControlGroup' => [
6464
'name' => 'HtmlTag',
6565
'options' => [
@@ -77,6 +77,7 @@ public function applyDefaultElementDecorators(): static
7777
]
7878
],
7979
]);
80+
8081
$this->getDecorators()->addDecorator('Required', $labelDecorator);
8182

8283
return $this;

‎tests/Compat/CompatFormTest.php‎

Lines changed: 207 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,16 @@
1010

1111
class CompatFormTest extends TestCase
1212
{
13-
/** @var CompatForm */
14-
private $form;
15-
1613
protected function setUp(): void
1714
{
18-
$this->form = new CompatForm();
1915
StaticTranslator::$instance = new NoopTranslator();
2016
}
2117

2218
public function testDuplicateSubmitButtonApplied(): void
2319
{
24-
$this->form->addElement('submit', 'submitCreate');
25-
$this->form->addElement('submit', 'submitDelete');
20+
$form = new CompatForm();
21+
$form->addElement('submit', 'submitCreate');
22+
$form->addElement('submit', 'submitDelete');
2623

2724
$expected = <<<'HTML'
2825
<form class="icinga-form icinga-controls" method="POST">
@@ -36,20 +33,21 @@ public function testDuplicateSubmitButtonApplied(): void
3633
</form>
3734
HTML;
3835

39-
$this->assertHtml($expected, $this->form);
36+
$this->assertHtml($expected, $form);
4037
}
4138

4239
public function testSubmitElementDuplication(): void
4340
{
44-
$this->form->addElement('submit', 'submit', [
41+
$form = new CompatForm();
42+
$form->addElement('submit', 'submit', [
4543
'label' => 'Submit label',
4644
'class' => 'btn-primary'
4745
]);
48-
$this->form->addElement('submit', 'delete', [
46+
$form->addElement('submit', 'delete', [
4947
'label' => 'Delete label',
5048
'class' => 'btn-danger'
5149
]);
52-
$this->form->setSubmitButton($this->form->getElement('submit'));
50+
$form->setSubmitButton($form->getElement('submit'));
5351

5452
$expected = <<<'HTML'
5553
<form class="icinga-form icinga-controls" method="POST">
@@ -63,22 +61,23 @@ public function testSubmitElementDuplication(): void
6361
</form>
6462
HTML;
6563

66-
$this->assertHtml($expected, $this->form);
64+
$this->assertHtml($expected, $form);
6765
}
6866

6967

7068
public function testSubmitButtonElementDuplication(): void
7169
{
72-
$this->form->addElement('submitButton', 'submit', [
70+
$form = new CompatForm();
71+
$form->addElement('submitButton', 'submit', [
7372
'label' => 'Submit label',
7473
'class' => 'btn-primary',
7574
'value' => 'submit_value'
7675
]);
77-
$this->form->addElement('submitButton', 'delete', [
76+
$form->addElement('submitButton', 'delete', [
7877
'label' => 'Delete label',
7978
'class' => 'btn-danger'
8079
]);
81-
$this->form->setSubmitButton($this->form->getElement('submit'));
80+
$form->setSubmitButton($form->getElement('submit'));
8281

8382
$expected = <<<'HTML'
8483
<form class="icinga-form icinga-controls" method="POST">
@@ -92,12 +91,13 @@ public function testSubmitButtonElementDuplication(): void
9291
</form>
9392
HTML;
9493

95-
$this->assertHtml($expected, $this->form);
94+
$this->assertHtml($expected, $form);
9695
}
9796

9897
public function testDuplicateSubmitButtonOmitted(): void
9998
{
100-
$this->form->addElement('submit', 'submitCreate');
99+
$form = new CompatForm();
100+
$form->addElement('submit', 'submitCreate');
101101

102102
$expected = <<<'HTML'
103103
<form class="icinga-form icinga-controls" method="POST">
@@ -107,13 +107,14 @@ public function testDuplicateSubmitButtonOmitted(): void
107107
</form>
108108
HTML;
109109

110-
$this->assertHtml($expected, $this->form);
110+
$this->assertHtml($expected, $form);
111111
}
112112

113113
public function testDuplicateSubmitButtonAddedOnlyOnce(): void
114114
{
115-
$this->form->addElement('submit', 'submitCreate', ['id' => 'submit_id']);
116-
$this->form->addElement('submit', 'submitDelete');
115+
$form = new CompatForm();
116+
$form->addElement('submit', 'submitCreate', ['id' => 'submit_id']);
117+
$form->addElement('submit', 'submitDelete');
117118

118119
$expected = <<<'HTML'
119120
<form class="icinga-form icinga-controls" method="POST">
@@ -128,8 +129,8 @@ public function testDuplicateSubmitButtonAddedOnlyOnce(): void
128129
HTML;
129130

130131
// Call render twice to ensure that the submit button is only prepended once.
131-
$this->form->render();
132-
$this->assertHtml($expected, $this->form);
132+
$form->render();
133+
$this->assertHtml($expected, $form);
133134
}
134135

135136
public function testDuplicateSubmitButtonRespectsOriginalAttributes(): void
@@ -139,7 +140,7 @@ public function testDuplicateSubmitButtonRespectsOriginalAttributes(): void
139140
'formnovalidate' => true
140141
]);
141142

142-
$prefixButton = $this->form->duplicateSubmitButton($submitButton);
143+
$prefixButton = (new CompatForm())->duplicateSubmitButton($submitButton);
143144

144145
// Name should stay the same
145146
$this->assertSame($submitButton->getName(), 'test_submit');
@@ -156,7 +157,8 @@ public function testDuplicateSubmitButtonRespectsOriginalAttributes(): void
156157

157158
public function testLabelDecoration(): void
158159
{
159-
$this->form->applyDefaultElementDecorators()
160+
$form = new CompatForm();
161+
$form->applyDefaultElementDecorators()
160162
->addElement(
161163
'text',
162164
'test_text_non_required',
@@ -202,6 +204,187 @@ public function testLabelDecoration(): void
202204
</form>
203205
HTML;
204206

205-
$this->assertHtml($expected, $this->form);
207+
$this->assertHtml($expected, $form);
208+
}
209+
210+
public function testFieldsetDecoration(): void
211+
{
212+
$form = new CompatForm();
213+
$form
214+
->applyDefaultElementDecorators()
215+
->addElement('fieldset', 'foo', [
216+
'label' => 'Legend here',
217+
'description' => 'Description here',
218+
'id' => 'foo-id'
219+
]);
220+
221+
$expected = <<<'HTML'
222+
<form class="icinga-form icinga-controls" method="POST">
223+
<div class="control-group">
224+
<fieldset name="foo" id="foo-id" aria-describedby="desc_foo-id">
225+
<legend>Legend here</legend>
226+
<p id="desc_foo-id">Description here</p>
227+
</fieldset>
228+
</div>
229+
</form>
230+
HTML;
231+
232+
$this->assertHtml($expected, $form);
233+
}
234+
235+
public function testCheckboxDecoration(): void
236+
{
237+
$form = new CompatForm();
238+
$form
239+
->applyDefaultElementDecorators()
240+
->addElement('checkbox', 'foo', ['label' => 'Label here', 'id' => 'foo-id']);
241+
242+
$expected = <<<'HTML'
243+
<form class="icinga-form icinga-controls" method="POST">
244+
<div class="control-group">
245+
<div class="control-label-group">
246+
<label class="form-element-label" for="foo-id">Label here</label>
247+
</div>
248+
<input name="foo" type="hidden" value="n"/>
249+
<input class="sr-only" id="foo-id" name="foo" type="checkbox" value="y"/>
250+
<label class="toggle-switch" aria-hidden="true" for="foo-id">
251+
<span class="toggle-slider"></span>
252+
</label>
253+
</div>
254+
</form>
255+
HTML;
256+
257+
$this->assertHtml($expected, $form);
258+
}
259+
260+
public function testDescriptionDecoration(): void
261+
{
262+
$form = new CompatForm();
263+
$form
264+
->applyDefaultElementDecorators()
265+
->addElement('text', 'foo', ['description' => 'Description here', 'id' => 'foo-id']);
266+
267+
$expected = <<<'HTML'
268+
<form class="icinga-form icinga-controls" method="POST">
269+
<div class="control-group">
270+
<div class="control-label-group">&nbsp;</div>
271+
<input name="foo" type="text" id="foo-id" aria-describedby="desc_foo-id"/>
272+
<i aria-hidden="true" class="icon fa-info-circle control-info fa" role="img" title="Description here"/>
273+
<span class="sr-only" id="desc_foo-id">Description here</span>
274+
</div>
275+
</form>
276+
HTML;
277+
278+
$this->assertHtml($expected, $form);
279+
}
280+
281+
public function testErrorsDecoration(): void
282+
{
283+
$form = new CompatForm();
284+
$form
285+
->applyDefaultElementDecorators()
286+
->addElement('text', 'foo');
287+
288+
$el = $form->getElement('foo');
289+
$el->addMessage('First error');
290+
$el->addMessage('Second error');
291+
292+
$expected = <<<'HTML'
293+
<form class="icinga-form icinga-controls" method="POST">
294+
<div class="control-group">
295+
<div class="control-label-group">&nbsp;</div>
296+
<input name="foo" type="text"/>
297+
<ul class="errors">
298+
<li>First error</li>
299+
<li>Second error</li>
300+
</ul>
301+
</div>
302+
</form>
303+
HTML;
304+
305+
$this->assertHtml($expected, $form);
306+
}
307+
308+
public function testFormControlsDecoration(): void
309+
{
310+
$form = new CompatForm();
311+
$form
312+
->applyDefaultElementDecorators()
313+
->addElement('submit', 'foo', ['label' => 'Submit Form']);
314+
315+
$expected = <<<'HTML'
316+
<form class="icinga-form icinga-controls" method="POST">
317+
<div class="control-group form-controls">
318+
<input name="foo" type="submit" value="Submit Form"/>
319+
</div>
320+
</form>
321+
HTML;
322+
323+
$this->assertHtml($expected, $form);
324+
}
325+
326+
public function testMethodApplyDefaultElementDecorators(): void
327+
{
328+
// A fieldset, a text element, a required checkbox, and a submit button should cover
329+
// all default element decorators.
330+
$form = new CompatForm();
331+
$form->applyDefaultElementDecorators();
332+
333+
$fieldset = $form->createElement('fieldset', 'foo', [
334+
'label' => 'Fieldset Label',
335+
'description' => 'Fieldset Description',
336+
'id' => 'foo-id'
337+
]);
338+
339+
$fieldset->addElement('text', 'bar', [
340+
'label' => 'Legend here',
341+
'description' => 'Description here',
342+
'id' => 'bar-id'
343+
]);
344+
345+
$form
346+
->addElement($fieldset)
347+
->addElement('checkbox', 'fooBar', [
348+
'label' => 'Fieldset Label',
349+
'description' => 'Fieldset Description',
350+
'id' => 'fooBar-id'
351+
])
352+
->addElement('submit', 'submit_form', ['label' => 'Submit Form']);
353+
354+
$expected = <<<'HTML'
355+
<form class="icinga-form icinga-controls" method="POST">
356+
<div class="control-group">
357+
<fieldset aria-describedby="desc_foo-id" id="foo-id" name="foo">
358+
<legend>Fieldset Label</legend>
359+
<p id="desc_foo-id">Fieldset Description</p>
360+
<div class="control-group">
361+
<div class="control-label-group">
362+
<label class="form-element-label" for="bar-id">Legend here</label>
363+
</div>
364+
<input aria-describedby="desc_bar-id" id="bar-id" name="foo[bar]" type="text"/>
365+
<i aria-hidden="true" class="icon fa-info-circle control-info fa" role="img" title="Description here"/>
366+
<span class="sr-only" id="desc_bar-id">Description here</span>
367+
</div>
368+
</fieldset>
369+
</div>
370+
<div class="control-group">
371+
<div class="control-label-group">
372+
<label class="form-element-label" for="fooBar-id">Fieldset Label</label>
373+
</div>
374+
<input name="fooBar" type="hidden" value="n"/>
375+
<input aria-describedby="desc_fooBar-id" class="sr-only" id="fooBar-id" name="fooBar" type="checkbox" value="y"/>
376+
<label aria-hidden="true" class="toggle-switch" for="fooBar-id">
377+
<span class="toggle-slider"/>
378+
</label>
379+
<i aria-hidden="true" class="icon fa-info-circle control-info fa" role="img" title="Fieldset Description"/>
380+
<span class="sr-only" id="desc_fooBar-id">Fieldset Description</span>
381+
</div>
382+
<div class="control-group form-controls">
383+
<input name="submit_form" type="submit" value="Submit Form"/>
384+
</div>
385+
</form>
386+
HTML;
387+
388+
$this->assertHtml($expected, $form);
206389
}
207390
}

0 commit comments

Comments
 (0)