Skip to content

Add descriptive error messages - #1789

Merged
yflory merged 10 commits into
stagingfrom
error-messages
Mar 17, 2025
Merged

Add descriptive error messages#1789
yflory merged 10 commits into
stagingfrom
error-messages

Conversation

@dariiing

@dariiing dariiing commented Feb 18, 2025

Copy link
Copy Markdown
Contributor

@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

@akbarjorayev akbarjorayev left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Better limit_error message

Comment thread www/admin/inner.js Outdated
Comment thread www/settings/inner.js Outdated
Comment thread customize.dist/messages.js Outdated

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

Copy link
Copy Markdown
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

Comment thread www/admin/inner.js Outdated
UI.warn(Messages.error);
return;
}
if (files[0].size > 200 * 1024) {

Copy link
Copy Markdown
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

@akbarjorayev akbarjorayev left a comment

Copy link
Copy Markdown

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 thread customize.dist/messages.js Outdated
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

Copy link
Copy Markdown

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

Comment thread www/admin/inner.js Outdated
UI.warn(Messages.error);
return;
}
if (files[0].size > 200 * 1024) {

Copy link
Copy Markdown

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