UIComponent: Render File Upload: Show Max nr of files and upload size #7306
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
klees
requested changes
Apr 24, 2024
There was a problem hiding this comment.
Hi @lscharmer,
thanks for looking into this.
Please implement the following changes:
- Output: Please do not construct strings containing meaningful parts in the renderer but instead via the mechanism we have for that: the template. Introduce separate placeholders for the new labels and values, we will also be able to address possible accessibility or styling changes easily then.
lscharmer
force-pushed
the
file-upload-info
branch
2 times, most recently
from
79f130c to
6f041be
Compare
thibsy
requested changes
Jun 6, 2024
There was a problem hiding this comment.
Hi @lscharmer
Thx a lot for incorporating @klees requested changes. I just looked over this again, and there are still some questions/remarks about this, before we can merge this:
- this is not yet a mandatory, but we will need to limit our
JavaScriptBindablecomponents to one top-level HTML element in the future. Is it therefore possible, to move the additional info into the current top-level element? - you are escaping two values of which we certainly know there will be no special characters inside them. Please remove both
htmlspecialchars()calls.
Kind regards,
@thibsy
lscharmer
force-pushed
the
file-upload-info
branch
from
6f041be to
f34f2b2
Compare
thibsy
approved these changes
Aug 20, 2024
There was a problem hiding this comment.
Hi @lscharmer,
Thx for implementing the requested changes.
We noticed you are using the | character as a separator, which may not be accessibility-friendly and could be flagged in an audit.
Otherwise this LGTM now, so I'll merge this for the time being.
Kind regards,
@thibsy
klees
pushed a commit
to klees/ILIAS
that referenced
this pull request
Aug 22, 2024
oliversamoila
pushed a commit
to oliversamoila/ILIAS
that referenced
this pull request
Sep 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses one part of Mantis Issue: https://mantis.ilias.de/view.php?id=39689
This PR displays the max nr of files and the max upload size below the file input field.
The info list may be extended to show the accepted file extensions in a future PR.