Fix FileWidget i18n msgid breakage introduced in #7982 - #8334
Conversation
|
@stevepiercy I need your attention here. The same problem I had with the PR https://github.com/plone/volto/pull/8320 happened here, but the PR has been merged. So, I'm trying to do a rollback for those IDs. |
stevepiercy
left a comment
There was a problem hiding this comment.
Just a tweak on the news fragment. Otherwise LGTM.
Was there a release made since the inadvertant msgid change? If so, then there also needs to be a mention in the upgrade guide.
@stevepiercy Yes, https://github.com/plone/volto/releases/tag/19.1.3. What do I need to mention? |
Co-authored-by: Steve Piercy <web@stevepiercy.com>
|
@Wagner3UB good question, and I'm not exactly sure. I'd say that a note for 19.1.3, as well as another for the next released version, would be appropriate. I'm not sure of the surface area, that is, the group of developers who might be affected by the change. My guess is developers who relied on the those |
|
@Wagner3UB @stevepiercy If the text changes, shouldn't the id also change? Since ids are strings, I find it strange to have one text in the message and another in the id. It looks like an error. If I were using ids in my own project, I'd prefer the translations to "break." That would allow me to see the new message. |
|
@wesleybl think of it as a Python dict. A |
wesleybl
left a comment
There was a problem hiding this comment.
Review
Thanks for this fix! The core intent (restoring original msgid keys) is correct and important. A few issues to address:
1. DefaultMessage values are not unchanged (PR description is incorrect)
The PR description states "The defaultMessage values remain as introduced in #7982", but the diff shows FileWidget's defaultMessages are all being significantly changed to new accessibility-focused strings:
| Message key | Old defaultMessage (#7982) | New defaultMessage (this PR) |
|---|---|---|
editFile |
Drop a file here or click to replace the existing file |
File upload area. Press Enter or click to replace the existing file |
fileDrag |
Drop a file here or click to upload |
File upload area. Press Enter or click to open the file browser |
dragAndDropActionA11y |
File upload area. Press Enter to open the file browser |
File upload area. Press Enter or click to open the file browser |
The new strings are better for accessibility. Please update the PR description to accurately reflect this.
2. Inconsistent editFile defaultMessage between FileWidget and RegistryImageWidget
Both widgets now share the same msgid "Drop file here to replace the existing file", but have different defaultMessages:
- FileWidget.jsx:
'File upload area. Press Enter or click to replace the existing file'(new a11y text) - RegistryImageWidget.jsx:
'Drop file here to replace the existing file'(unchanged old text)
Is this intentional? If RegistryImageWidget should also show the new a11y-friendly text, its defaultMessage should be updated too.
3. visually-hidden → visually-hidden-volto change is good but should be mentioned
This is a correct fix (aligns with Volto's convention used in Toast.jsx and UrlWidget.jsx), but it's unrelated to the i18n msgid restoration. Please document it in the PR description.
Minor: duplicate type="file" on the input
Pre-existing issue, but line 218 explicitly sets type: 'file' which is already handled by getInputProps() on line 217-219. Not introduced by this PR though.
After addressing #1 (description update), this looks good to approve.
…into fix/file-widget-a11y-msgids
|
@wesleybl Done @stevepiercy can you run your check here, please? |
| #. Default: "File upload area. Press Enter or click to replace the existing file" | ||
| #: components/manage/Widgets/FileWidget | ||
| msgid "File upload area. Press Enter to open the file browser" | ||
| msgid "File upload area. Press Enter or click to replace the existing file" |
There was a problem hiding this comment.
@stevepiercy This msgid wasn't added in #7982, it existed before and had translations as well. #7982 renamed Press Enter to browse files from your computer. into it, which broke those. This PR just restores the original msgid and only changes the Default, as you suggested.
My bad for the mess with the IDs. It caused a huge chain reaction.
There was a problem hiding this comment.
I just double-checked. I stand by what I say. @Wagner3UB would you please double check?
| #. Default: "File upload area. Press Enter or click to replace the existing file" | ||
| #: components/manage/Widgets/FileWidget | ||
| msgid "File upload area. Press Enter to open the file browser" | ||
| msgid "File upload area. Press Enter or click to replace the existing file" |
There was a problem hiding this comment.
I just double-checked. I stand by what I say. @Wagner3UB would you please double check?
wesleybl
left a comment
There was a problem hiding this comment.
I still find it strange for the id to be something other than 'default', but okay.
…evert in upgrade guide
stevepiercy
left a comment
There was a problem hiding this comment.
Thanks for the upgrade guide entry. That's nice.
Co-authored-by: Steve Piercy <web@stevepiercy.com>
Co-authored-by: Steve Piercy <web@stevepiercy.com>
Co-authored-by: Steve Piercy <web@stevepiercy.com>
Co-authored-by: Steve Piercy <web@stevepiercy.com>
Summary
PR #7982 (FileWidget - a11y) inadvertently changed the
idfields of severaldefineMessagesentries inFileWidget.jsxandRegistryImageWidget.jsx. Since theidis the key used at runtime to look up translations in.pofiles, renaming them effectively broke all existing translations for those strings — any site that had already translated the old keys would silently fall back to the English default.This PR does two things:
idkeys, so existing.potranslations keep matching (backward compatibility).defaultMessagevalues to improve accessibility: the new strings describe both interaction methods ("Press Enter or click") instead of keyboard only, and a newdragAndDropReplaceA11ymessage distinguishes the "replace existing file" state from the initial "upload" state announced to assistive technology.id(backward-compat)defaultMessage(a11y)editFileDrop file here to replace the existing fileFile upload area. Press Enter or click to replace the existing file.fileDragDrop file here to upload a new fileFile upload area. Press Enter or click to open the file browser.dragAndDropActionA11yPress Enter to browse files from your computer.File upload area. Press Enter or click to open the file browser.dragAndDropReplaceA11y(new)File upload area. Press Enter or click to replace the existing file.The same
defaultMessageupdates are applied toRegistryImageWidget.jsxso both widgets share identicalidanddefaultMessage, avoiding an i18n extraction conflict on the shared keys.