UI: Introduce Is Nullable to Certain Column Types
Hi everybody
This PR is first to start a discussion. I would also implement this afterwards, if we think some implementation is needed, but I would then ask you to be a little bit patient with me (this is outside any contract).
While working on the table to show logs in the test I had the requirement to add a column with optional content (in my case a link). This is currently not possible. Right now it is possible to render the link and then insert it as a text column, but I don't believe this to be the right solution either.
This PR thus proposes to add a function Column::withIsNullable(bool $nullable): void to certain column types, but not all.
Open questions and explanations:
- I did not add
withIsNullableto all column-types, because I believe that certain types, e.g.StatusorBooleanprofit from being clear: In these cases the options are clear and should always be defined. Other types contain primarily text (TextandEMail) and here I think it is better to force the use of empty strings for empty values instead of offering two different options to achieve the same thing, although this argument is a little controversial, as there still is a difference between the empty value and a non-existent value.Numberin my eyes is different and a clear case of the latter, as we have a clear and visual difference between the neutral value, ie.0, and a non-existent value, ie.null. - I think the TimeSpan is another interesting case, where my current thinking is that
nullableshould mean that either the left or the right or both values can benull. I would argue that this avoids the need to introduce more of these "configuration functions" as all this scenarios are very well possible (and I would argue will come up). - Finally sorting: I would suggest to add empty values to the end of the list, as I this feels more logical to me (fuzzy argument, I know). The argument for putting it first: Our main database sorts them in first by default (I know: changeable). I have no clear-cut opinion on this.
If I get it right the functions on the interfaces of the table come with little in the way of doc-blocks and I think this should be self explanatory, too, but I could add some information to the Column\Factory if desired.
Thank you very much and best, @kergomard
This is now partially superseded by https://github.com/ILIAS-eLearning/ILIAS/pull/7566 . There are more column-types that might profit from some kind of "nullable" state (see above), but I don't have much of a horse in the race anymore. Depending on your decision, I'm still willing to further work on this.
@kergomard I have assigned this back to you to get this off my list, feel free to ping me and assign back if there is a next iteration.
Thank you very much for the Feedback @klees, @Amstutz , and @thibsy !
I realized something that I only figured out by a mistake: I don't believe anymore, that this is needed and I'm sorry to have caused you work!
There already is a mechanism for this: You just don't specify the corresponding key in the array passed to the DataRowBuilder. It just didn't cross my mind that this was an option. Am I missing something?
If this is the way to go, I would provide a PR to expand the examples correspondingly. If I'm wrong, and we would prefer to have this more verbose approach, I can go on working on this PR.
Sorry again and best, @kergomard
I'm currently not sure if we support this deliberately or just by accident. If there is no pointer to that functionality in the examples or descriptions, I would guess this is by accident. I will look into this some time soon and try to give a more informed opinion.
Kind regards!
Hello everyone, I stumbled across this and the related PR in https://mantis.ilias.de/view.php?id=39326 again. As a non-expert on the implementation details, can you give me some info on when we can expect a next step here? I'm just a little concerned that it's assumed that it's the other one's turn ;) Thank you very much for your work on this topic.