Add multi-language authoring support (Web changes) - #2859
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## DEP-234-authoring-language-switching #2859 +/- ##
=======================================================================
Coverage ? 72.99%
=======================================================================
Files ? 522
Lines ? 20672
Branches ? 1854
=======================================================================
Hits ? 15090
Misses ? 5572
Partials ? 10
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
jareth-whitney
left a comment
jareth-whitney
left a comment
There was a problem hiding this comment.
Wow, this translation layer is basically its own piece of software. Great job implementing it, I'm excited to try it out once it's merged. Take a look at the following:
- Code optimization (if else/if to ternary, since it's a boolean condition)
- Consider setting default language as an env variable
- Consider disabling language select box if there are no options
- Verification that details tab label number will always match sequential tab number
- Language list comparison: should it work out of sequence?
- Separate reused code into its own function
- 'Complete section' vs 'Completed section'
- Some mapping ideas to make code less verbose
- Lowercasing a static value that is already lowercased 'en'
- Static english string 'Click here to register', is it falling back to this in all languages?
- Widget form function sharing
- timelineTranslation value safety
- Translating values that should stay the same (email and phone number)
- Any consequences of changing es target?
| UPDATE: `${AppConfig.apiUrl}/widgets/widget_id/events/event_id/item/item_id`, | ||
| DELETE: `${AppConfig.apiUrl}/widgets/widget_id/events/event_id`, | ||
| SORT: `${AppConfig.apiUrl}/widgets/widget_id/events/sort_index`, | ||
| GET_ITEM_TRANSLATION: `${AppConfig.apiUrl}/events/event_id/translations/item/event_item_id/language/language_id`, |
There was a problem hiding this comment.
It could be good to have a method that gets an entire list of file translations.
| if (isSingleLanguage === true) { | ||
| languageTypeValue = 'single'; | ||
| } else if (isSingleLanguage === false) { | ||
| languageTypeValue = 'multi'; |
There was a problem hiding this comment.
| if (isSingleLanguage === true) { | |
| languageTypeValue = 'single'; | |
| } else if (isSingleLanguage === false) { | |
| languageTypeValue = 'multi'; | |
| languageTypeValue = isSingleValue === true ? 'single' : 'multi'; |
| formState: { errors }, | ||
| } = useFormContext<EngagementUpdateData>(); | ||
| const { engagement } = useRouteLoaderData('single-engagement') as EngagementLoaderAdminData; | ||
| const { languageCode = 'en' } = useParams(); |
There was a problem hiding this comment.
English should be a safe fallback, otherwise we could define the default language in env variables.
|
|
||
| const handleSelectChange = (event: SelectChangeEvent<string>) => { | ||
| if (!isSubmitting) { | ||
| if (!isSubmitting && !isSwitchingLanguage && !isEnglishOnly) { |
There was a problem hiding this comment.
Preventing a change if English is the only language is a good move. I wonder if we could make that select box disabled when it's single-language, because it looks kind of clunky when you click on it and nothing is in there.
There was a problem hiding this comment.
The select box is already disabled when single-language in this commit (line 584) :)
| }, | ||
| }} | ||
| renderValue={() => { | ||
| const completed = false; // todo: Replace with real "completed" boolean value once it is available. |
There was a problem hiding this comment.
Yay, glad that todo can finally be removed.
| (translation) => translation.widget_image_id === imageWidget.id, | ||
| ); | ||
|
|
||
| const nextTranslations = existingTranslation |
There was a problem hiding this comment.
I'm noticing that we're doing similar things for different widget forms. If the functions aren't too different to consolidate, sharing any reused code could help keep things compact.
| const timelineTranslation = contentTranslations.timeline_widgets.find( | ||
| (t) => t.widget_timeline_id === currentTimelineWidget.id, | ||
| ); | ||
| translateTimelineWidget(currentTimelineWidget, timelineTranslation); |
There was a problem hiding this comment.
Should timelineTranslation be checked before it's used?
| language_id: number; | ||
| name?: string; | ||
| title?: string; | ||
| email?: string; |
There was a problem hiding this comment.
Shouldn't need to translate email and phone number, we should be able to store those on the main contact entry.
|
|
||
| /* Language and Environment */ | ||
| "target": "es2017" /* Set the JavaScript language version for emitted JavaScript and include compatible library declarations. */, | ||
| "target": "es2023" /* Set the JavaScript language version for emitted JavaScript and include compatible library declarations. */, |
There was a problem hiding this comment.
Are there any consequences to changing the es target from 2017 to 2023?
There was a problem hiding this comment.
No; this was to support the .toSorted method from es2023 syntax as suggested by SonarCloud
| - Added/expanded translation infrastructure for engagement and widget content (including timeline, image, contact, and related widget translation resources), plus supporting migrations and tests. | ||
| - Publishing behavior aligns with requirements: English required sections can be completed/published independently; incomplete non-English content does not block publishing, and incomplete languages are not exposed on the public view. | ||
| - English-only engagements now keep language controls effectively non-interactive/disabled in authoring contexts where no alternate language is configured. | ||
|
|
There was a problem hiding this comment.
Very cool stuff Nat, great job on this ticket.
|
3b73913
into
DEP-234-authoring-language-switching
3 participants
Issue #: 🎟️ DEP-234
Description of changes:
User Guide update ticket (if applicable):
Common component changes: