Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions src/admin/class-admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ public function register_settings() {
'show_in_rest' => [
'schema' => [
'type' => 'object',
'additionalProperties' => true, // If we upgrade or downgrade, the settings can possibly show as null. Prevent this.
'properties' => [
// General settings
'optimize_all_media' => [
Expand Down
5 changes: 5 additions & 0 deletions src/admin/class-script-loader.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ public static function enqueue_cimo_assets() {
// Get current settings
$settings = get_option( 'cimo_options', [] );

// Expose threshold for WP Core's big image scaling.
// Can be false when auto scaling is disabled.
$threshold = apply_filters( 'big_image_size_threshold', 2560 );

// Localize script with REST API URL, nonce, and settings
wp_localize_script(
'cimo-script',
Expand All @@ -100,6 +104,7 @@ public static function enqueue_cimo_assets() {
'svgUpload' => isset( $settings['svg_upload'] ) ? (int) $settings['svg_upload'] : 0,
'svgOptimizationEnabled' => isset( $settings['svg_optimization_enabled'] ) ? (int) $settings['svg_optimization_enabled'] : 1,
'stealthModeEnabled' => isset( $settings['stealth_mode_enabled'] ) ? (int) $settings['stealth_mode_enabled'] : 0,
'wpScalingThreshold' => $threshold,
]
);

Expand Down
9 changes: 8 additions & 1 deletion src/admin/js/page/admin-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -591,8 +591,15 @@ const AdminSettings = () => {
label={ __( 'Maximum Image Dimension', 'cimo-image-optimizer' ) }
type="number"
value={ settings.maxImageDimension }
placeholder={ settings.disableWpScaling === 1 ? '2560' : undefined }
onChange={ value => handleInputChange( 'maxImageDimension', value ) }
help={ __( 'Maximum width or height in pixels for uploaded images. Images exceeding this dimension will be automatically resized while preserving aspect ratio. Leave empty to disable resizing. We recommend a value of 1920px.', 'cimo-image-optimizer' ) }
help={ sprintf(
__( 'Maximum width or height in pixels for uploaded images. Images exceeding this dimension will be automatically resized while preserving aspect ratio. Leave empty to %s. We recommend a value of 1920px.', 'cimo-image-optimizer' ),
window.cimoSettings?.wpScalingThreshold
? sprintf( __( "use WordPress's default auto-scaling at %spx", 'cimo-image-optimizer' ), window.cimoSettings.wpScalingThreshold )
: __( 'disable auto-scaling', 'cimo-image-optimizer' )
) }
Comment on lines +594 to +601
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Placeholder should use wpScalingThreshold and avoid split translations.

Two issues in this block:

  1. Placeholder is hardcoded to '2560' while the help text uses the dynamic window.cimoSettings?.wpScalingThreshold. If a site filters big_image_size_threshold to a non-default value, the placeholder and help will disagree. Use the localized value for both (and fall back to hiding the placeholder when the threshold is falsy, consistent with the branching already done for the help text).

  2. Nested sprintf splits a sentence across two translatable strings. Translators see an isolated fragment ("use WordPress's default auto-scaling at %spx") with no surrounding context, and word order can't be rearranged for languages that need it. Prefer two complete, self-contained sentences selected by a single conditional.

✏️ Suggested adjustment
-								placeholder={ settings.disableWpScaling === 1 ? '2560' : undefined }
+								placeholder={ settings.disableWpScaling === 1 && window.cimoSettings?.wpScalingThreshold
+									? String( window.cimoSettings.wpScalingThreshold )
+									: undefined }
 								onChange={ value => handleInputChange( 'maxImageDimension', value ) }
-								help={ sprintf(
-									__( 'Maximum width or height in pixels for uploaded images. Images exceeding this dimension will be automatically resized while preserving aspect ratio. Leave empty to %s. We recommend a value of 1920px.', 'cimo-image-optimizer' ),
-									window.cimoSettings?.wpScalingThreshold
-										? sprintf( __( "use WordPress's default auto-scaling at %spx", 'cimo-image-optimizer' ), window.cimoSettings.wpScalingThreshold )
-										: __( 'disable auto-scaling', 'cimo-image-optimizer' )
-								) }
+								help={ window.cimoSettings?.wpScalingThreshold
+									? sprintf(
+										/* translators: %d is the WordPress big_image_size_threshold in pixels. */
+										__( "Maximum width or height in pixels for uploaded images. Images exceeding this dimension will be automatically resized while preserving aspect ratio. Leave empty to use WordPress's default auto-scaling at %dpx. We recommend a value of 1920px.", 'cimo-image-optimizer' ),
+										window.cimoSettings.wpScalingThreshold
+									)
+									: __( 'Maximum width or height in pixels for uploaded images. Images exceeding this dimension will be automatically resized while preserving aspect ratio. Leave empty to disable auto-scaling. We recommend a value of 1920px.', 'cimo-image-optimizer' )
+								}

Also note the stray blank line at 602 inside the JSX attributes — safe to remove.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/admin/js/page/admin-settings.js` around lines 594 - 601, The placeholder
currently uses a hardcoded '2560' and the help text nests sprintf, causing
inconsistent/unsplittable translations; update the input's placeholder prop to
use the dynamic window.cimoSettings?.wpScalingThreshold (or undefined when
falsy) so it matches the help text logic, and refactor the help prop to choose
between two complete, self-contained translated sentences (one when
wpScalingThreshold is truthy and one when falsy) instead of nesting sprintf
calls; adjust the JSX around settings.disableWpScaling and
handleInputChange('maxImageDimension', ...) accordingly, and remove the stray
blank line inside the JSX attributes.


__next40pxDefaultSize
/>
</div>
Expand Down
12 changes: 11 additions & 1 deletion src/shared/converters/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,16 @@ export const getFileConverter = _file => {
}

if ( file.type.startsWith( 'image/' ) ) {
const max = parseFloat( window.cimoSettings?.maxImageDimension || 0 )
const wp = parseFloat( window.cimoSettings?.wpScalingThreshold || 0 )

// Determine the final max dimension to use for conversion,
// prioritizing the lower of the two thresholds if both are set.
// 0 means no max dimension.
const finalMaxDimension = max && wp
? Math.min( max, wp )
: max || wp || 0

// If the browser doesn't support webp, then we can't convert it.
if ( ! isFormatSupported( 'webp' ) ) {
return new NullConverter( file )
Expand All @@ -46,7 +56,7 @@ export const getFileConverter = _file => {
return new ImageConverter( file, {
format: 'webp',
quality: window.cimoSettings?.webpQuality || 0.8,
maxDimension: window.cimoSettings?.maxImageDimension || 0,
maxDimension: finalMaxDimension,
} )
}
}
Expand Down
Loading