-
Notifications
You must be signed in to change notification settings - Fork 663
FileUploader: revert maxWidth calculation update #31659
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR reverts a previous fix that addressed a layout issue in FileUploader where the size element would move to the next row when both size and title were long. The revert restores more width allocation for the file name element by removing a 10px gap from the width calculation. The team decided to revert this fix and will consider a CSS-only approach if more real cases emerge in the future.
Key changes:
- Removed
GAPandREFERENCE_TEXTconstants used in the previous fix - Changed reference text from constant to inline
'1000 Mb'string - Removed gap subtraction from maxWidth calculation to allocate more space for file names
|
|
||
| const prevFileSize = $fileSize?.text(); | ||
| $fileSize?.text(REFERENCE_TEXT); | ||
| $fileSize?.text('1000 Mb'); |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded reference text '1000 Mb' has two issues:
-
Incorrect capitalization: Should be
'1000 MB'(uppercase 'B') to match the localized unit format injs/localization/messages/en.json. -
Localization concern: The reference text is used to measure maximum width, but different locales have different unit lengths. For example, Arabic uses "ميغا بايت" which is much longer than "Mb", while most languages use "MB". This could result in inaccurate width calculations for some locales, potentially causing layout issues.
Consider using a properly formatted localized reference string, e.g., messageLocalization.format('dxFileUploader-Mb') to construct the reference text: \1000 ${messageLocalization.format('dxFileUploader-Mb')}``.
| $fileSize?.text('1000 Mb'); | |
| $fileSize?.text(`1000 ${messageLocalization.format('dxFileUploader-Mb')}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 2 out of 5 changed files in this pull request and generated 1 comment.
|
|
||
| const prevFileSize = $fileSize?.text(); | ||
| $fileSize?.text(REFERENCE_TEXT); | ||
| $fileSize?.text('1000 Mb'); |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded string '1000 Mb' is a magic value that should be extracted to a constant for better maintainability. Consider defining it at the module level (e.g., const MAX_FILE_SIZE_TEXT = '1000 Mb';) to make it clear this is a reference text used for width calculation purposes.
Previously, we fixed a layout issue where the size element moved to the next row when both the size and the title were long. However, that change reduced the width allocated for the name in general. We decided to revert the fix for now. If we encounter more real cases in the future, we will rework this part completely, remove the JavaScript width calculations, and solve it using a CSS-only approach.