FormElement: Add Multiselect element #65
Conversation
sukhwinder33445
force-pushed
the
add-multiselect-element
branch
3 times, most recently
from
5fb3558 to
c51f7cb
Compare
sukhwinder33445
force-pushed
the
add-multiselect-element
branch
2 times, most recently
from
23e1f3b to
ca695ed
Compare
sukhwinder33445
force-pushed
the
add-multiselect-element
branch
3 times, most recently
from
7dd0883 to
41d50de
Compare
sukhwinder33445
force-pushed
the
add-multiselect-element
branch
5 times, most recently
from
1d04828 to
2e45745
Compare
sukhwinder33445
force-pushed
the
add-multiselect-element
branch
3 times, most recently
from
d520fbf to
cc22abe
Compare
sukhwinder33445
force-pushed
the
add-multiselect-element
branch
from
04340e1 to
b21a796
Compare
sukhwinder33445
force-pushed
the
add-multiselect-element
branch
from
b21a796 to
58846c9
Compare
sukhwinder33445
force-pushed
the
add-multiselect-element
branch
from
134831c to
6165b61
Compare
sukhwinder33445
changed the title
sukhwinder33445
force-pushed
the
add-multiselect-element
branch
from
6165b61 to
b3ae3d7
Compare
sukhwinder33445
force-pushed
the
add-multiselect-element
branch
from
b3ae3d7 to
9a2d691
Compare
| } | ||
| } | ||
|
|
||
| BaseFormElement::validate(); |
There was a problem hiding this comment.
Because the parent class checks its own value first, since I already have a loop above for this check, I skip calling the parent method.
There was a problem hiding this comment.
Exactly. In addition we have duplicated code. This shows that the code flow is flawed, and it raises the question of why we even have a separate element for this. Instead, specifying the multiple attribute for the select element should just work.
| $this->valid = false; | ||
| } else { | ||
| parent::validate(); | ||
| $this->addMessage(sprintf($this->translate("'%s' is not allowed here"), $value)); |
There was a problem hiding this comment.
Possible candidate for InArrayValidator or DeferredInArrayValidator.
There was a problem hiding this comment.
I have already created a new PR for this.
There was a problem hiding this comment.
But I don't see its usage here.
| } | ||
| } | ||
|
|
||
| BaseFormElement::validate(); |
There was a problem hiding this comment.
Exactly. In addition we have duplicated code. This shows that the code flow is flawed, and it raises the question of why we even have a separate element for this. Instead, specifying the multiple attribute for the select element should just work.
* Add multiple attribute in `$defaultAttrributes` and remove constructor * Don't validate in `setValue()`, the `validate()` method is resposible for this.
Form::validate() takes care of this.
sukhwinder33445
force-pushed
the
add-multiselect-element
branch
from
9a2d691 to
47ab4d5
Compare
|
Add |
No description provided.