[FEATURE] UI: `Field\TreeSelect` and `Field\TreeMultiSelect` inputs

[thibsy]

Hi folks,

It's been a while, but here's the first proposal for the public interface of the resource selector input!

The factory description is derived from the last state of the prototype created in #6320. Since the look and feel has already been discussed there, I won't go over it in detail here. Instead, let's dive into the more logical aspects of the interface:

Following multiple discussions in the UI-Clinic and the implementation of the prototype, we have decided that a Menu\Drilldown menu is the most feasible approach for navigating the hierarchical dataset at the moment.

To maintain flexibility, I have kept the interface of the Field\ResourceSelector decoupled from the Menu\Drilldown menu, so we can easily swap out the implementation to a different approach. This also lays the groundwork for e.g. adding alternative views, tailored for different devices.

For handling large datasets and rendering sup-parts asynchronously, I implemented a monoid-ish approach for nesting Field\Resources: The Field\ResourceContainer represents a Field\Resource itself, while providing an operation to nest further Field\Resources. In order to indicate what sup-parts of the dataset need to be rendered asynchronously, I have introduced a withRenderChildrenAsync() method on the Field\ResourceContainer, as it appeared to be the most suitable location.

Using the current approach for rendering sub-parts will need the UI framework to have its own endpoint. This is because unlike other components, the Field\ResourceSelectors sub-parts cannot simply be rendered as "HTML over the wire", because this would mean that we need to expose the somewhat special kind of Menu\Drilldown menu on the public interface. Besides that, this would again tightly couple usages to the drilldown approach, making it hard to exchange and impossible to automatically handle device-specific views.

Please note the extension of Field\HasDynamicInputs in the Field\ResourceSelector. I added this as a fallback, because its usage depends on further implementation details and developments in how we want to handle asynchronously rendered sub-parts of a dataset. There is a possibility we want to render sub-parts on the client using a template.

Let me know if you have any adhoc questions, otherwise I am looking to your review.

Kind regards, @thibsy

[thibsy]

[thibsy]

Btw, this could also play along nicely with the proposed change to Menu\Drilldown components in #7024.

[yvseiler]

Thanks a lot for your work @thibsy , it will be very exciting to see the Resource Selector in action. I could follow everything so far except your section here:

Using the current approach for rendering sub-parts will need the UI framework to have its own endpoint. This is because unlike other components, the Field\ResourceSelectors sub-parts cannot simply be rendered as "HTML over the wire", because this would mean that we need to expose the somewhat special kind of Menu\Drilldown menu on the public interface.

What are the implications of this? I read in there that the Resource Selector component should not be coupled with the drilldown component (for reasons), but what does "will need the UI framework to have its own endpoint" mean? Thanks for clarification.

[thibsy]

Hi @yvseiler,

Asynchronous rendering currently relies on consumers providing their own endpoints in form of URLs, which are used by the UI framework to fetch HTML content ("HTML over the wire") that is used to modify/replace the existing UI. However, for the resource selector, following the same concept would tightly couple each endpoint to the concept of rendering drilldown menu entries (sub-parts of the dataset), and would also require these components to be available on the public interface - which is not something we want.

Another approach could be to follow the same concept, but instead of HTML the client could fetch sub-parts in form of JSON data, which can be used to render the drilldown menu entries using a template. Since our framework relies on server-side-rendering, I suggested yet another approach, by introducing a dedicated endpoint that could look something like this: https://domain.com/ui.php?foo=bar. Having its own endpoint is something we ultimately want, so I suggested to make it happen with this component :). Doing so would also ensure, that the JSON response coming from our endpoint will always be structured the way we want it to be. Using consumer endpoints will never guarantee this.

Hope this helps!

Kind regards, @thibsy

[Amstutz]

short update here: @klees reviewed it - ill will give this another look and we will ship one review together. In short, we are on it and make progress.

[Amstutz]

Hi @thibsy!

Thank you so much for this nice UI element. Looks solid in general, but we could work on some aspects, as most often =) Especially the naming and wording seems to be something that we could really improve. It could be worth a live discussion. Note the @klees and @Amstutz also had some discussion around one and the other question and we are looking forward to your opinion.

Please answer the following questions:

• [x] Drilldown a: Is the "Text Input" rival actually correct? The Drilldown Menu recently gained a search functionality. Maybe leave out, since it is a legacy component anyway.
• [x] Drilldown b: If we understand correctly, the Drilldown will be used in the implementation behind the public interface, correct?
• [x] container: The description of container is really hard to understand. How does this abstractly represent an object? What even is an object in this context? Maybe this would also benefit from a rewording to tree (see first item in next paragraph) as well. A resource then would be a "leaf" and a container would be a "node" and we could just drop this "represents" wording etc. all together.
• [x] ResourceRetrieval::__construct: Having a constructor on an interface is really weird because a constructor can never be called on an abstract interface but only on concrete classes. Still, the reason why we would need this in the current implementation is comprehensible. Did you try to get rid of that constructor by leveraging the new possibilities of the component revision? This looks as if there should be a way to contribute retrieval objects in some way to move that construction back to the component it belongs to.

Please consider the following suggestions. You do not need to follow those, but but please indicate shortly why you prefer to do otherwise:

• [x] purpose: Please go into a little bit more detail what a "resource" is supposed to be. The selector seems to be the right choice when "resources" are structured via a tree that can be traversed to discover "resources". A resource could e.g. be an object in the repository, a file in the filesystems directory structure or a term in a taxonomy. This input would not work well for cases where the "resources" are a longish flat list, where each "resource" is "tagged" in some way. So e.g. selecting a user from a member list would not be a very good use case, right? We think this should be elaborated in the "purpose" description. We are missing "tree" there and an explanation of the vague term "resource".
• [x] accessibility: We would suggest to be a little less explicit regarding the "effect" of selecting a resource. Darkening color to indicate "disabled" could be a problem for accessibilty due to contrast issues. Maybe something like "the actual drilldown menu entry will indicated that it cannot be selected anymore" is enough and leaves some wiggle space for a accessible solution.
• [x] usage.1/2: We are not convinced that these rules are actually helpful. usage.1 uses the vague term "discoverability", thus we won't be able to decide anything based on that rule. usage.2 intends to mirror the "Text Input" rival, right? With regards to the first item in this list, rule 1 might be better phrased
• [x] factory params: Please put the ResourceRetrieval as last parameter. We use that order for the Data Table as well and some consistency is always nice.
• [x] ResourceRetrieval1: Could we name this DataRetrieval as well? We will likely be using this pattern and should form some convention here. The current convention would be to use generic Data, while this seems to to suggest that the Retrieval is named after the thingy it returns. This should be one or the other, but not both.
• [x] Resource::getId: Could we use string|int as return type?
• [x] Resource::getIcon: This method indicates that we would get an Icon but we would get a Glyph instead. Something is off here, either the name or the return type. Seems to be the return type, but we are not 100% sure. The same thing is in the factory.
• [x] Async: Please promote the split between sync and async containers to the factory, and maybe even to the level of the interfaces that are involved. Currently, I would be asked to provide resources to an async container via the fourth parameter in ResourceFactory::container, just to basically discard them by marking the container as async via mutator later on. In that case, if we understand the interface correctly, that resources would need to be loaded again in another process once the front end decides to load them via the ResourceRetrieval, right?
• [x] MultiSelect: Please consider splitting single and multiselect selectors via the factory (and classes etc...). For one, I could imagine that we might want to have drastically different components for both cases, since requirements for the user interface seem considerable different. Also, we change the original content of the input via a mutator. So if we have some transformations attached to the input, these could become invalid when using the mutator.

Please implement the following changes:

• [x] context: There is some copy pasta there that we should remove. This is not the Rating Input =)
• [x] composition: In its collapsed form, where we only represent the input as a Shy Button, the Input should indicate which resources are currently selected without requiring the user to open the modal to do the selection. This is missing in the "composition" and "effect".
• [x] title: Please rename Resource::getTitle to Resource::getName (and in other places accordingly). According to [previous research](https://github.com/ILIAS-eLearning/ILIAS/pull/6107

Shall we discuss the current state, our review and the course of action?

@klees and @Amstutz

[thibsy]

Hi @Amstutz, Hi @klees

Thanks a lot for reviewing this! A lot of good inputs in there :).

I have changed the overall wording to address several of your inputs, so the tree data structure is better reflected now. I tried to stick with a wording we are already familiar with for the inputs themselves, which are now called Field\TreeSelect and Field\MultiTreeSelect. These names kind of mirror the already existing Field\Select and Field\MultiSelect inputs. Please note that we also have a Field\ColorPicker input, which IMO could also benefit from a rename to Field\ColorSelect someday.

I have put the node-related interfaces into a new namespace, because I did not want to crowd the Input\Field namespace too much and it made interface names a bit simpler. Similar things have also been done for e.g. interruptive items for modals. However, in this case I am not sure if Field\Node is the right place. Maybe we could also pull this into Input\Node or similar. I would have liked to use the existing Tree\Node interfaces, but this would have meant a larger refactoring of the tree component as well, which would make this project even bigger than it already is - so I sticked with implementing some new interfaces.

Let me answer some of your questions/remarks now:

• Drilldown a: the text input rival could be outdated, I merely added this because I remembered a discussion we had in one of the workshops about this component, that a tree select inputs like proposed here should not be used if the user exactly knows what they are searching for. However, since we have gained this functionality in the drilldown menu as well, I think we can just leave it out now.
• Drilldown b: yes and no, some version of the drilldown menu will be used to implement the tree select inputs, which should not be on the public interface. There will be a lot of unique features, like breadcrumb navigation and additional select buttons that will require some new internal drilldown component - but most of it can and will be reused.
• ResourceRetrieval::__construct(): I think this can be implemented using the new component system, I therefore removed this declaration from the public interface.
• factory params: this is not possible because the byline is an optional argument. I would also like to keep the label and byline argument together, so I sticked with NodeRetrieval as a first argument. The same is already done for the file input.

Let me know what you think about this approach.

Kind regards, @thibsy

[matthiaskunkel]

Jour Fixe, 10 JUN 2024: We highly appreciate this suggestion and accept the PR for trunk / ILIAS 10.

[Amstutz]

Hi @thibsy

thx, ill pass this back to you. Feel free to re-assign me/us as soon as implemented.

lg Timon