Skip to content
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

Fix missing translations for Customizer #4184

Merged
merged 3 commits into from
Jan 26, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions inc/customizer/loader.php
Copy link
Contributor

Choose a reason for hiding this comment

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

While this might work, I have some concerns about the code.

  1. Is that using glob() might cause issues depending on the server.
  2. Each script chunk is enqueued.

Can't this be fixed by modifying the webpack config? I'm asking this just based on your findings.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, that this will introduce a performance penalty due to the IO reads for unnecessary work since we don't need to read the folder at each load and it can be saved on build when the release is made since those filenames are never changed and are static. The only time when they are changed is on a new release.

Copy link
Contributor Author

@Soare-Robert-Daniel Soare-Robert-Daniel Jan 10, 2024

Choose a reason for hiding this comment

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

Another solution is to tweak webpack to make a single file, but this will change the .pot file, and I am not sure what the impact is in wp org. Does WP regenerate the .po file along with .json files based on the new references in .pot file?

Can the ranking in store might be affected by this?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Soare-Robert-Daniel wordpress.org generates the translation list based on the files, by extracting the texts from the files, not on the po/pot/json files, those are redundant, and they are not used on translation.wordpress.org.

Copy link
Contributor

Choose a reason for hiding this comment

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

one idea is similar how wp-scripts generates the assets.php file with the deps, generate a PHP file with the chunks list by webpack and require it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks guys for the feedback. I implement the idea of changing assets.php to include a special option for chunks. This was done via a small custom plugin for Webpack that modifies the output of the wp-scripts.

A message will be displayed in the terminal:
Screenshot 2024-01-11 at 14 40 06

Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,24 @@ public function enqueue_customizer_controls() {
wp_style_add_data( 'react-controls', 'rtl', 'replace' );
wp_enqueue_style( 'react-controls' );

// Automatically detect, register, and enqueue js chunk scripts with translations.
$build_path = get_template_directory() . '/assets/apps/customizer-controls/build/';
$js_chunk_files = glob( $build_path . '*.js' );

foreach ( $js_chunk_files as $chunk_file ) {
if ( 'controls.js' === basename( $chunk_file ) ) {
continue;
}

$chunk_handle = 'neve-customizer-chunk-' . basename( $chunk_file, '.js' );
wp_register_script( $chunk_handle, $bundle_path . basename( $chunk_file ), [], $dependencies['version'], true );
wp_enqueue_script( $chunk_handle );

if ( function_exists( 'wp_set_script_translations' ) ) {
wp_set_script_translations( $chunk_handle, 'neve' );
}
}

$fonts = neve_get_google_fonts();
$chunks = array_chunk( $fonts, absint( count( $fonts ) / 5 ) );

Expand Down
Loading