Skip to content

Conversation

dariiing
Copy link
Contributor

@dariiing dariiing commented Feb 18, 2025

@dariiing dariiing added the Accessibility Make software available to everyone label Feb 18, 2025
@dariiing dariiing added this to the Spring Release (2025.3.0) milestone Feb 18, 2025
@dariiing dariiing self-assigned this Feb 18, 2025
Copy link

@akbarjorayev akbarjorayev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better limit_error message

@@ -118,6 +118,10 @@ define(req, function(AppConfig, Default, Language) {

Messages._languages = map;
Messages._languageUsed = language;
Messages.admin_logoSize_error = "The logo size must be smaller than 200KB"; // XXX
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value "200KB" should be passed from the code as a value, with the placeholder "{0}" in the translated string

@@ -884,6 +884,11 @@ define([
UI.warn(Messages.error);
return;
}
if (files[0].size > 200 * 1024) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use a variable here, same one that should be passed to the message

Copy link

@akbarjorayev akbarjorayev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

max file size variable added @davidbenque

Comment on lines 118 to 121

Messages._languages = map;
Messages._languageUsed = language;
Messages.admin_logoSize_error = "The logo size must be smaller than 200KB"; // XXX

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Messages._languages = map;
Messages._languageUsed = language;
Messages.admin_logoSize_error = "The logo size must be smaller than 200KB"; // XXX
let maxLogoSize = 200;
Messages._languages = map;
Messages._languageUsed = language;
Messages.admin_logoSize_error = `The logo size must be smaller than ${maxLogoSize}KB`; // XXX

@@ -884,6 +884,11 @@ define([
UI.warn(Messages.error);
return;
}
if (files[0].size > 200 * 1024) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (files[0].size > 200 * 1024) {
let maxLogoSizeMb = 200 * 1024;
if (files[0].size > maxLogoSizeMb) {

@dariiing dariiing marked this pull request as ready for review March 5, 2025 15:02
@dariiing dariiing added the Ready to Review This PR is ready to be checked by another team member label Mar 5, 2025
@davidbenque davidbenque removed the Accessibility Make software available to everyone label Mar 12, 2025
@yflory yflory added Ready to Test This PR is ready to be tested and removed Ready to Review This PR is ready to be checked by another team member labels Mar 12, 2025
@yflory yflory merged commit 88186d8 into staging Mar 17, 2025
@dariiing dariiing deleted the error-messages branch March 25, 2025 08:55
@davidbenque davidbenque removed the Ready to Test This PR is ready to be tested label Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants