Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI, ViewControls/DataTable: 41310, add nullControl to reserve input name #7488

Merged
merged 2 commits into from
May 29, 2024

Conversation

nhaagen
Copy link
Contributor

@nhaagen nhaagen commented May 7, 2024

@klees
Copy link
Member

klees commented May 15, 2024

@thibsy / @Amstutz: CaT internal QS is done, this seems to be good for your review.

@Amstutz
Copy link
Contributor

Amstutz commented May 15, 2024

Hi @nhaagen

Thx a lot for this enhancement. It was not immediately clear to me, why we would need this instead of living with null. However, I believe to understand the advantage (thx @klees).

Therefore, please change or indicate why not:

  • Enhance the purpose description: Maybe drop a phrase that we use this where we otherwise would need to pass null? It might be obvious to most, but could help some.

Please change:

  • Add a rendering test: Making sure nothing is rendered.

One last thing: Thx for the code style improvements. However, they make PRs sometimes difficult to read. Maybe put them in separate PRs in the future.

Thx a lot
@thibsy, @klees and @Amstutz

@matthiaskunkel
Copy link
Member

Jour Fixe, 27 MAY 2024: We highly appreciate the suggestion and accept the PR for all maintained versions (incl. 8).

@Amstutz
Copy link
Contributor

Amstutz commented May 29, 2024

Thx a lot for the refinements!

@Amstutz Amstutz merged commit 4abc47f into ILIAS-eLearning:release_9 May 29, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants