UI, Table: an OrderingTable to change position of records

[nhaagen]

This introduces the Ordering Table. The Ordering Table is very close to the DataTable, but provides an input for each row to change the order of records (you may also drag rows around ;)). It is wrapped in a form and writes back the order of rows to its data binding.

[klees]

@thibsy @Amstutz AFAIC we could take this to the JF. Would you be so kind and have a look into the interface?

[tfamula]

Hi @nhaagen, thank you for the PR, I'm really looking forward to this table :)

While checking my legacy tables, I stumbled over the following use case (and I guess I will not be the only one): Depending on permissions, a user must see the "Position"-column and the "Save Order"-button – or not. As far as I can see, it will not be possible to hide/disable the column and the button within the OrderingTable. So the only option would be to create two tables (Ordering and Data) in my code and show one of them in the GUI, depending on whether the user is allowed to change the order or not. Is this the way to go or are you planning to make the ordering in the OrderingTable optionally deactivatable?

Thanks Thomas

[klees]

Hi @tfamula,

thanks for bringing that use case. Seems to make sense, so I guess no one will object to an according addition. Doesn't seem to be very complicated as well. Could you propose that addition once the ordering table is included?

Kind regards!

[oliversamoila]

Hello @nhaagen Thank you for this table variant.

• I have two more comments: [Save Order] should not be a primary action button in my opinion. The name of the table suggests that this is the central purpose, but there will also be use cases in which the ordering table is used, but the ordering is not primary, but simply functional or appropriate. Examples are the configuration of the main menu (since ILIAS 5.4) and the dashboard (since ILIAS 9).
• Could it be useful to explicitly point out in the description of the ordering table that the controls for selection (checkboxes) per line and actions per line are an COULD condition?

[klees]

Hi @thibsy, hi @Amstutz,

thanks for your time to discuss your feedback. I'll try to document our results here, please feel free to correct me if required.

I will put some general point regarding withRequest here first, so this comment can be referenced and people can stop reading afterwards: It is intentional, that withRequest is neither injected via the constructor nor injected via the Renderer. The reason that we do not inject it via constructor is as such: There will be components, that produce components to be rendered by other components. Membership, e.g., could produce a member table to be rendered in the Course. To allow Membership to pass a request via constructor, it either would need to pull it out of thin air somehow (think: $DIC, static call) or the request would need to be passed down by the Course. The request would mostly become available in the controller of the page that is currently viewed and would need to be passed down, maybe through various layers, if we would require it in the constructor. Also, the table would then somehow be tainted with that contextual info from the request and we would not be able to use it in some other context. This is also the reason, why we do not insert the request in the renderer. Imagine that we use the table to create some export. The original request would, most likely, not be correct in that case. So the request is something the consumer should be able to control easily and also something where the consumer should be reminded that he indeed should handle that issue with care. Also, the request should, mostly stay in the top layers, close to the endpoint that a user called. The same argumentation works for the inputs and also, most likely, for other locations where we want to include the request.

Hence, regarding HasRequest and HasActions, creating interfaces is not the way to proceed, as we currently do not see that we will need to write code that can act on all components that have a request or actions.

Regarding the item Submissions: We discussed that there is another problem, besides the ones mentioned in the item. When the interactions is mediated via a form with POST where the result is rendered immediately, a reload of the page will lead to that irritating "form resubmission" message from the browser. This is a well known problem, which is solved by processing the POST and than redirecting, which does not happen for the ordering table currently. We did agree though, that the proposed solutions both are not what we want. The first solution would turn an implementation detail to something essential: While we currently use a form to submit the desired order to the server, this is not essential to the ordering table. Other implementations are well possible: async, some custom json protocol, .... So, an ordering table is not an Input Container, but something on its own. Async, on the other hand would require us to somehow actually reorder the table or refresh the table with some content for the server, which both is something we currently cannot do very well in a principled way. A good solution to this might be to have a second URL to receive the POST and a slightly updated version of OrderingBinding with something like setOrder(): ?MessageBox\Failure instead. We will look into this.

Regarding OrderingBinding we will stick to that name for the moment, as the class indeed expresses a two way binding between a storage and the table and hence is more then the one way retrieval for the table. We'll first look to the activities regarding the submissions will turn out.

That's it, I think @nhaagen can go on here now =)

Best wishes!

[nhaagen]

Thank you all for discussing that, I rebased and added some changes.

[Save Order] should not be a primary action button in my opinion. The name of the table suggests that this is the central purpose, but there will also be use cases in which the ordering table is used, but the ordering is not primary, but simply functional or appropriate. Examples are the configuration of the main menu (since ILIAS 5.4) and the dashboard (since ILIAS 9).

It's a StandardButton, now.

• Could it be useful to explicitly point out in the description of the ordering table that the controls for selection (checkboxes) per line and actions per line are an COULD condition?

I added a comment in the example; the actions have to be added explicitly, so I think this is rather obvious (and a MAY, if). Actions have a full doc-entry on their own, too.

it will not be possible to hide/disable the column and the button within the OrderingTable

There is now Ordering::withOrderingDisabled (see example)

C\Row: this interface suggests, it holds commonalities between all possible table-rows. However, this is not the case for PresentationRow. Now, since this interface is essentially a C\DataRow, please revert this change and make C\OrderingRow extend C\DataRow instead, so we can remove this interface.

Changed back to OrderingRow extending DataRow; however, an OrderingRow now is a DataRow, too, so the Renderer has to distinguish by negative exclusion (or, worse, by the order of if-clauses). Ultimately, the PresentationRow should be a Row as well...

static return-type: C\OrderingBinding::withOrder() C\OrderingRow::withPosition()

Those are endemic to the ordering; I'd always prefer self over static. Same goes for the others; withRequest/withActions returns static because of the implementation in AbstractTable, only.

A good solution to this might be to have a second URL to receive the POST

I added Ordering::withTargetURL - although not too refined, we might take it as a base for another iteration?

Best, Nils

[klees]

Hi @thibsy and @Amstutz,

I think we have addressed all items of your review. Would you be so kind and move this forward?

Kind regards!

[klees]

Maybe we can take this to JF on monday...

[nhaagen]

Hi @thibsy , I think I addressed the main suggestions and issues;

[ ] DataTable.#tmpDragRow: I don't think we need this property.

I did not find a vanilla way to pass a var through events

[ ] Row builders:

Having but one RowBuilder would enable the consumer to return mixed row types from getRows; I really would not like to lose the current strictness.

[ ] AbstractTable: please rename this class to Table.

Sadly, the PresentationTable is still in the way. I'd really like to move everthing from AbstractTable to Table, once the data binding and View Controls are aligned

Kind regards, Nils

[thibsy]

Thx a lot @nhaagen for implementing the changes! I will add the JF label so we can discuss the interface on Monday.

[matthiaskunkel]

Jour Fixe, 13 MAY 2024: Richard presented us the current implementation of the UI Ordering Table. We highly appreciate this suggestion and accept the PR for trunk.