UI Inputs rebased #19
Conversation
There was a problem hiding this comment.
Hi Timon,
I think I came to some conclusions. Reviewing this was really hard, so I might have missed something. I couldn't resist to comment on the documentation as well, so there is a lot food for discussion included here.
I'm convinced that this implementation would work, but I feel that there are some conceptual problems we should solve. This basically revolves around the question how input from POST, initial values and error are handled. I'm not too sure I see the problems clearly atm.
I still would suggest the following course of action, as it should help us in any case:
- Conceptualize Validation in an interface, move it somewhere else (-> this PR gets smaller)
- Get to a shared understanding that parallel validation is a special case we should not consider here (and maybe convince FS)
- Conceptualize Transformation in an interface, move it somewhere else (-> this PR gets smaller)
- Conceptualize Result in a class, move it somewhere else (-> I'm about 100% convinced that this will make everything easier afterwards)
- Get too a shared understanding that we basically need to create two APIs, one for the end user dev who just want to create forms, one for the form element creating dev who, well, creates new form elements. These can and imho should have different shapes.
I think that we then could get to something like:
- A formlet-like API for the innarts of Forms and Filter-Forms, modeled like
name_source -> (name_source, $POST -> Result, Result -> (ID, primitive value, error_message)), where thename_sourceis updated, the second item extracts a result from the input array and the third item contains the information for the renderer. - An API above that that encodes form-like structures and wraps the underlying formlet API by supplying the name_source and pouring some sugar over the formlets stuff.
The formlets then would be the internal API for creating new form elements and the other API would be for public use. This would however mean that there would be elements provided by the UI-factory that could not be rendered on their own and that there would be renderers for elements that are not provided (directly) by the UI-factory. The latter could be provided publicly, but users would then need to fill the names in, which imho we do not want. This deviation from the current pattern is arguably ok, since we deal with new stuff (input) here and the suggested approach allows a nice separation of concerns.
I'm looking forward to our discussion about this!
| * purpose: > | ||
| * Filters enable to add and remove criteria for displaying or hiding items in a list or table. | ||
| * composition: > | ||
| * Filters are composed of the input selectors and fields for composing the individual criteria for the filter along with an “Apply Filter” Button to apply Filter Button. |
| namespace ILIAS\UI\Component\Input\Container; | ||
|
|
||
| /** | ||
| * Common interface shared by all containers. |
There was a problem hiding this comment.
What is a container? That word seems to be very generic, so maybe we describe it a little in this context? Like "A container for input elements contains controls to define one submittable unit of input."? The point here is, that a container basically provides the <form>, right? So what is the semantic commonality?
| * | ||
| * rules: | ||
| * accessibility: | ||
| * 1: Every Input Element in every Input Collection MUST be accessible by keyboard. |
There was a problem hiding this comment.
Move this up one layer to Input\Factory::container.
| * purpose: > | ||
| * Forms are used for creating content of sub-items or for configuring objects or services. | ||
| * composition: > | ||
| * A form is divided into areas, which are further divided into sections containing settings with selection or input field controls. |
There was a problem hiding this comment.
Area is missing in the Form\Factory.
| * --- | ||
| * description: | ||
| * purpose: > | ||
| * Forms are used for creating content of sub-items or for configuring objects or services. |
There was a problem hiding this comment.
I think there should be more use cases then creating sub-items or configuring objects/services, e.g. the registration form. Or is this another type of container?
| * Interface Equals | ||
| * @package ILIAS\UI\Component\Input\Validation | ||
| */ | ||
| interface Equals extends Validation { |
There was a problem hiding this comment.
I think we could use concrete classes for the concrete validations. As a user I would want to say that I want my input to be validated with this exact notion of equality/not emptyness/regex and not with any notion of these.
| * | ||
| * @return Validation | ||
| */ | ||
| public function invert(); |
There was a problem hiding this comment.
not()? Or expose not on the factory?
| */ | ||
| public function section($title, $items) { | ||
| return new Section($title, $items); | ||
| } |
There was a problem hiding this comment.
Please don't mess with indentation =)
| */ | ||
| class Factory { | ||
| /** | ||
| * Todo: This static function is poisson for testability and proper DI |
| /** | ||
| * @inheritdoc | ||
| */ | ||
| public function extractToView(){ |
There was a problem hiding this comment.
This needs to be kept in sync manually with withInputFromView, right? I think this happens because the name is not really attached to the input item. If one would e.g. determine it when creating the form, this would fall in one place.
…nto 'trunk' Resolve "Migration: Einbau auf Ansicht der Zertifikatsliste" Closes #19 See merge request ntheen/ilias-certificates!146
Please do not rage on this too much ;-). I think at this point we should focus on clarifying the bigger picture and concentrate on large issues. In a second iteration we can go into the details.