-
Notifications
You must be signed in to change notification settings - Fork 3
feat: Internationlization #120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Utilized for script requests.
Standardized logging within the code.
The editor no longer implicitly starts.
Simplify configuration with examples.
Consistently name utility files as objects.
Consistently name utility files as objects.
Required to ensure imported files have the correct locale set.
Ensure all project-specific strings are translated.
Two benefits: 1. Align with Gutenberg 2. Use the existing translation
Required to ensure imported files have the correct locale set.
Clarify the utility's purpose.
In order to set the locale before critical `@wordpress` packages load, we must load i18n packages separately from the rest of the `@wordpress` packages. Otherwise, the locale will not be set in time and/or we will encounter collisions from loading packages more than once.
These feel like two separate concerns. Separating them hopefully reduces complexity.
Two, distinct levels provided little-to-no value.
Prefixing with the log level duplicates browser log level styles. Prefixing the project name or acronym at least provides additional context.
1d43f80
to
948dc37
Compare
It's unclear why this manual chunk configuration is required, but the editor fails to load without it. The dynamic import of the module never resolves.
There is no reason to attempt loading the default locale. There will never be a translation file.
Using static imports means the packages are loaded before the locale is set, leading to incorrect translation strings in various places.
If the editor fails early in the load process, these styles need to be present for presenting error message UI. Therefore, we hoist them to the entry file.
Limit scope of file responsibilities.
Simplify initializing the local editor.
The locale must be set before most `@wordpress` modules load, as they rely upon globals for retrieving translation strings. However, the manual chunks configuration seemingly led to preloading of all `@wordpress` modules even before they were dynamically imported. vitejs/vite#8617 (comment)
A circular dependency causes the dynamic import to never resolve. Using a Promise circumvents the issue, as it appears to function differently than top-level await. Ideally, we resolve the circular dependency. However, using Rollup's `manualChunks` configuration does not appear to be an option, as it results in preloading of `@wordpress` modules. The preloading results in untranslated strings as `@wordpress` modules load before the globals they rely upon for localization, which are set by `setLocaleData`.
Usage was removed in a earlier commit.
Allow for avoiding unnecessary network requests when translation strings are already present.
Avoid unnecessary nesting.
Communicate the cache is ignored when using the `force` flag.
Simplify resetting build caches.
This is only used within the module.
Because we load internationalization modules before the rest of the `@wordpress` modules, we must strategically chunk modules to avoid circular dependencies. Additionally, Vite injects its own helper utilities seem to create their own circular dependencies, at times. Lastly, the correct chunk configuration for the `index` entry is a bit complicated and allusive. Instead, we rely upon a `Promise` rather than `async`/`await`, as the latter will timeout when a circular dependency exists.
Because of the latest dynamic import strategy and Vite managing externalizing `@wordpress` modules for the remote editor, we no longer need to use DI for these functions. For the local editor, the imported modules will be bundled and imported. For the remote editor, the imported modules will be replaced with global `window.wp` references.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the files changed count; the bulk of them are build output for iOS, necessary until we finish #45.
import { info, error, debug } from '../src/utils/logger.js'; | ||
|
||
const TRANSLATIONS_DIR = path.join( process.cwd(), 'src/translations' ); | ||
const SUPPORTED_LOCALES = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the same translations supported by Gutenberg Mobile.
<EnvironmentVariable | ||
key = "GUTENBERG_EDITOR_REMOTE_URL" | ||
value = "http://localhost:5174/remote.html" | ||
isEnabled = "NO"> | ||
</EnvironmentVariable> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disabled by default. Added to simplify testing the remote editor.
@@ -77,6 +77,7 @@ private struct _EditorView: UIViewControllerRepresentable { | |||
if #available(iOS 16.4, *) { | |||
viewController.webView.isInspectable = true | |||
} | |||
viewController.startEditorSetup() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this required start call to the demo app was overlooked during #114.
@@ -33,7 +33,7 @@ export default function DefaultBlockAppender() { | |||
|
|||
return ( | |||
<button | |||
aria-label={ __( 'Add paragraph block' ) } | |||
aria-label={ __( 'Add paragraph block', 'gutenberg-kit' ) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The addition of the gutenberg-kit
domain properly groups the project-specific string.
@@ -76,7 +76,7 @@ const EditorToolbar = ( { className } ) => { | |||
{ isSelected && ( | |||
<ToolbarGroup> | |||
<ToolbarButton | |||
title={ __( 'Open Settings' ) } | |||
title={ __( 'Block Settings' ) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to reuse the existing, translated Gutenberg project string.
rollupOptions: { | ||
output: { | ||
manualChunks: { | ||
// Chunk to avoid circular dependency | ||
bridge: [ 'src/utils/bridge.js' ], | ||
}, | ||
}, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Configuring manualChunks
results in preloading of @wordpress
modules—this appears to be a Vite feature. However, it disrupts internationalization (i18n) within the @wordpress
modules. The @wordpress
modules rely upon globals for retrieving locale strings. So, if the loading order is incorrect, certain strings are unavailable and missing.
Long term, we may be able to address this by expressly disabling preloading. For now, I opted to remove the manual chunk altogether.
The referenced circular dependency is address within the entry file by using a Promise
rather than async
/await
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only change is wrapping all selectors with a .gutenberg-kit
selector to increase specificity. The increased specificity is required as the styles are now loaded before styles from @wordpress
modules, which impacts how CSS prioritizes style application.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separated from src/utils/editor.js
to limit responsibilities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added to standardize logging within the project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This existing code from relocated from src/remote.jsx
to limit responsibilities of files and reuse editor initialization logic between src/index.jsx
and src/remote.jsx
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Existing logic abstracted to a new location for sharing editor initialization logic between src/index.jsx
and src/remote.jsx
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
I tested one of the translations (Russian) and haven't noticed any issues in the implementation.
What?
Add internationalization (i18n) support.
Why?
Ref CMM-280. Close #110. I18n improves the editor usability.
How?
Download and utilize translation strings from translate.wordpress.org.
Testing Instructions
Tip
Use the prototype builds for testing:
Important
1: Verify translations in the local editor
2: Verify translations in the remote editor
Experimental block editor plugins
(iOS) orgutenberg_kit_plugins
(Android) debug flag.Accessibility Testing Instructions
N/A, no navigation changes.
Screenshots or screencast
N/A, no visual changes.