Skip to content

Make the uploads-busy validation message configurable#1099

Open
romansklenar wants to merge 2 commits into
basecamp:mainfrom
romansklenar:config-uploads-busy-message
Open

Make the uploads-busy validation message configurable#1099
romansklenar wants to merge 2 commits into
basecamp:mainfrom
romansklenar:config-uploads-busy-message

Conversation

@romansklenar

Copy link
Copy Markdown

Problem

While an attachment is uploading, the editor reports itself form-invalid (the upload-lock validity added in 0.9.16) and the browser shows a native validation bubble. The message is hardcoded English:

const UPLOADS_BUSY_MESSAGE = "Please wait for all files to upload"

So a non-English app shows an untranslated bubble while an inline image uploads, with no way to override it.

Fix

Promote the string to an editor config option, uploadsBusyMessage, registered in the default preset and defaulting to the current English text — no behavior change for existing users.

Because it lives in the default preset, Lexxy recognizes it both ways, the same mechanism used for permittedAttachmentTypes and friends:

Lexxy.configure({ default: { uploadsBusyMessage: "…" } })
<lexxy-editor uploads-busy-message=""></lexxy-editor>

#setUploadsValidity now reads this.editorConfig.get("uploadsBusyMessage").

Note: Lexxy has no general i18n layer yet (toolbar title="Bold" etc. are hardcoded too). That is a much larger change and out of scope here — this PR adds the single config key. Happy to follow a maintainer's steer toward a general messages map instead.

Test

Adds a regression test to test/browser/tests/attachments/form_validity.test.js (plus a fixture with a non-ASCII custom message): configure uploads-busy-message, start an upload, and assert the editor's internals.validationMessage equals the custom string. Fails on main (shows the hardcoded English) and passes with the change.

Also documents the option in docs/configuration.md.

Full chromium browser suite: 484 passed.

Context: a downstream consumer currently carries a temporary monkey-patch wrapping setElementValidity to translate the English string. This PR lets them drop it.

While an attachment is uploading the editor reports itself form-invalid
with a native validation bubble. The message was hardcoded English
("Please wait for all files to upload"), so non-English apps showed an
untranslated bubble.

Promote the string to an editor config option, uploadsBusyMessage,
registered in the default preset and defaulting to the current English
text — no behavior change for existing users. Because it lives in the
default preset, Lexxy recognizes it both via
configure({ default: { uploadsBusyMessage: "…" } }) and the dasherised
attribute <lexxy-editor uploads-busy-message="…">, the same mechanism
used for permittedAttachmentTypes and friends.
@romansklenar romansklenar marked this pull request as ready for review June 9, 2026 19:54
Copilot AI review requested due to automatic review settings June 9, 2026 19:54

Copilot AI left a comment

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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a configurable “uploads busy” form validation message for the attachments workflow, allowing apps to localize the message shown while uploads are in progress.

Changes:

  • Introduces uploadsBusyMessage configuration (with a default) and wires it into attachments validity handling.
  • Adds a browser fixture + Playwright test to verify a custom message is surfaced during uploads.
  • Documents the new configuration option.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/browser/tests/attachments/form_validity.test.js Adds an end-to-end test asserting a custom validity message during uploads.
test/browser/fixtures/attachments-uploads-busy-message.html Adds a fixture page configuring uploads-busy-message on <lexxy-editor>.
src/extensions/attachments_extension.js Uses configured uploadsBusyMessage instead of a local constant.
src/config/lexxy.js Adds default uploadsBusyMessage to presets.
docs/configuration.md Documents the new configuration option and attribute example.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +52 to +53
await calls.releaseDirectUploadResponses()
})

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch — fixed in cf0a87f. After releaseDirectUploadResponses() the test now waits for the rendered blob image and for checkValidity() to return true, so the upload fully settles before teardown. This mirrors the existing default test in the same file.

@@ -0,0 +1,30 @@
<!DOCTYPE html>
<html lang="en">

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'll keep lang="en" here. The fixture's actual document content is English (the placeholder "Write something…", the "Post title" label), so lang="en" correctly describes the rendered page. The Czech string is only a config value passed via the uploads-busy-message attribute to exercise the localization plumbing — it surfaces as a native browser validation bubble (whose language the UA derives separately), not as document content, so it doesn't drive the document lang. Switching to lang="cs" would mislabel the English UI text. This also stays consistent with the other fixtures in test/browser/fixtures/, which all use lang="en".

Addresses review comment on test/browser/tests/attachments/form_validity.test.js:53

After releasing the mocked direct-upload responses the test ended without
waiting for the upload to finish, so the page could be torn down while
async completion handlers were still running. Wait for the rendered blob
image and for the editor to return to a valid state, matching the existing
default test in this file.
@romansklenar

Copy link
Copy Markdown
Author

RDY for review.

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.

2 participants