Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to fix a spacing issue within the file-uploader component by refactoring the HTML structure. The changes are generally well-implemented and focused. I have identified a couple of minor areas for improvement: one regarding redundant CSS classes which can affect maintainability, and another concerning a small violation of the project's UX writing guidelines. My review comments provide specific suggestions to address these points.
projects/element-ng/file-uploader/si-file-uploader.component.html
Outdated
Show resolved
Hide resolved
spike-rabbit
left a comment
spike-rabbit
left a comment
There was a problem hiding this comment.
Looks already better, just the alignment of the validation icon seem to be off.
I left some comments which should fix it.
Can you please also check if the AI comments are valid.
Please also choose a more descriptive commit message. Something about align with UX specs or so.
projects/element-ng/file-uploader/si-file-uploader.component.html
Outdated
Show resolved
Hide resolved
projects/element-ng/file-uploader/si-file-uploader.component.html
Outdated
Show resolved
Hide resolved
projects/element-ng/file-uploader/si-file-uploader.component.html
Outdated
Show resolved
Hide resolved
|
@hbxes can you please have a look here. |
robertwilde
force-pushed
the
fix/file-uploader/spacing
branch
3 times, most recently
from
fa15d8f to
8315667
Compare
|
I don't think it's a wise idea to place the unit, right next to the value, this change has to be reverted ... |
robertwilde
force-pushed
the
fix/file-uploader/spacing
branch
from
8315667 to
4fec865
Compare
@hbxes @spike-rabbit Thanks, I'll fix it. I take it we need to update the UX Writing/Punctuation documentation. |
robertwilde
force-pushed
the
fix/file-uploader/spacing
branch
from
4fec865 to
b40c6fb
Compare
4 participants
Related to #1412
Fix spacing between warning/error label and upper two labels as specified in the Figma source.
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: