Improve quality setting handling in Compression settings #978

ineagu · 2025-10-01T12:44:45Z

Refactored the Compression.js component to better handle toggling between auto and manual quality settings while fixing https://github.com/Codeinwp/optimole-service/issues/1516#issuecomment-3355204589. Updated the minimum allowed quality value in settings.php from 1 to 50 to stay consistent with the front-end.

Changed default compression setting to 80, which is more likely to be used vs 90, which is not recommended.

All Submissions:

Have you followed the Optimole Contributing guideline?
Does your code follow the WordPress' coding standards?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Refactored the Compression.js component to better handle toggling between auto and manual quality settings while fixing https://github.com/Codeinwp/optimole-service/issues/1516#issuecomment-3355204589. Updated the minimum allowed quality value in settings.php from 1 to 50 to stay consistent with the front-end.

Changed default compression setting to 80, which is more likely to be used vs 90, which is not recommended.

Closes # .

How to test the changes in this Pull Request:

Disable machine learning compression & save
Ensure the default value is used on the frontend.
Make sure going back to mauto or migrating from an older version works as expected.

Other information:

[x ] Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes, as applicable?
[ x] Have you successfully ran tests with your changes locally?

pirate-bot · 2025-10-01T12:46:41Z

Plugin build for 124434d is ready 🛎️!

Download build
Download dev

Copilot

Pull Request Overview

Refines compression quality handling: introduces state to preserve manual quality when toggling auto mode, adjusts default/auto mapping to 80 (from 90), and raises the minimum backend quality bound to align with the UI.

Added ref (manualQualityRef) plus toggle handler to restore prior manual quality after auto mode.
Changed quality mapping defaults (auto now resolves to 80) and updated backend minimum bound from 1 to 50.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
inc/settings.php	Raised lower quality bound to 50 (was 1) when sanitizing settings.
assets/src/dashboard/parts/connected/settings/Compression.js	Refactored quality handling: added getQuality(), manual quality preservation via ref, new auto toggle handler, changed default mappings.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-01T12:53:23Z

assets/src/dashboard/parts/connected/settings/Compression.js

+	const getQuality = value => {
+		if ( 'number' === typeof value ) {
+			return value;
+		}
+
+		if ( 'auto' === value ) {
+			return 80;
+		}
+
+		if ( 'high_c' === value ) {
+			return 90;
+		}
+
+		if ( 'medium_c' === value ) {
+			return 75;
+		}
+
+		if ( 'low_c' === value ) {
+			return 55;
+		}
+
+		return 80;
+	};


The toggle handler (lines 106-117) writes 'mauto' into data.quality, but getQuality does not recognize 'mauto', only 'auto', so 'mauto' will fall through to the default branch (80) and any logic expecting a distinct auto sentinel may break or become ambiguous. Either change 'mauto' to 'auto' everywhere or add a condition for 'mauto' in getQuality (and any other downstream logic) to keep behavior consistent.

Copilot · 2025-10-01T12:53:23Z

assets/src/dashboard/parts/connected/settings/Compression.js

+	const handleAutoQualityToggle = value => {
+		setCanSave( true );
+		const data = { ...settings };
+		data.autoquality = value ? 'enabled' : 'disabled';
+		if ( value ) {
+			manualQualityRef.current = getQuality( settings.quality );
+			data.quality = 'mauto';
+		} else {
+			const manualQuality = manualQualityRef.current ?? getQuality( settings.quality );
+			manualQualityRef.current = manualQuality;
+			data.quality = manualQuality;
+		}
+		setSettings( data );
+	};


When returning from auto mode (line 116), data.quality may receive a numeric value (e.g. 90) even if the original pre-toggle setting was an enumerated string like 'high_c'; this changes the type/semantic form of the stored value and could affect UI branches or persistence expecting symbolic presets. Preserve and restore the original raw quality token instead (store original settings.quality before converting) or store both the raw token and numeric resolution separately.

Copilot · 2025-10-01T12:53:23Z

assets/src/dashboard/parts/connected/settings/Compression.js

 	const updateQuality = value => {
 		setCanSave( true );
 		const data = { ...settings };
 		data.quality = value;
+		manualQualityRef.current = value;


[nitpick] manualQualityRef.current is set to value without normalization; depending on caller, this could be a preset token or a number, increasing downstream branching complexity. Consider consistently storing the raw selection token plus (optionally) a derived numeric form to avoid mixed-type state and simplify later restoration logic.

Copilot · 2025-10-01T12:53:23Z

inc/settings.php

 				case 'limit_width':
 					$sanitized_value = $this->to_bound_integer( $value, 100, 5000 );
 					break;
 				case 'quality':


Raising the minimum allowed quality from 1 to 50 is a silent tightening of accepted input that will coerce previously valid stored values (<50) up to 50 on next save, potentially surprising users who intentionally chose lower quality for stronger compression. Consider documenting this change inline (comment) and/or providing a migration note or backward-compat handling (e.g., warning, transitional deprecation) to make the behavioral change explicit.

Suggested change

case 'quality':

case 'quality':

// Minimum allowed quality was raised from 1 to 50.

// Any previously stored values <50 will be coerced to 50 on next save.

// Consider informing users who intentionally chose lower quality for stronger compression.

if (intval($value) < 50) {

// Optional: Log a warning or trigger a deprecation notice for legacy values.

trigger_error(

'The minimum allowed image quality has been raised from 1 to 50. Values below 50 will be set to 50. Please review your settings.',

E_USER_DEPRECATED

);

}

Soare-Robert-Daniel · 2025-10-01T16:04:05Z

inc/settings.php

 					break;
 				case 'quality':
-					$sanitized_value = $this->to_bound_integer( $value, 1, 100 );
+					$sanitized_value = $this->to_bound_integer( $value, 50, 100 );


I think here is the most important thing, the initial issue with value being saved as one is because quality can be a string, auto, or number, to which the sanization does not consider the string case, thus the value will be saved as 1, it is left untouched.

We should handle this case.

Soare-Robert-Daniel

I left some comments.

Soare-Robert-Daniel · 2025-10-01T16:07:28Z

assets/src/dashboard/parts/connected/settings/Compression.js

+	const handleAutoQualityToggle = value => {
+		setCanSave( true );
+		const data = { ...settings };
+		data.autoquality = value ? 'enabled' : 'disabled';
+		if ( value ) {
+			manualQualityRef.current = getQuality( settings.quality );
+			data.quality = 'mauto';
+		} else {
+			const manualQuality = manualQualityRef.current ?? getQuality( settings.quality );
+			manualQualityRef.current = manualQuality;
+			data.quality = manualQuality;
+		}
+		setSettings( data );
+	};


I think this made something a little complicated. If you want to remember the slide value, separated from the settings, you can use another state to remember it and use it when you toggle off. useRef is used for more tricky things when you manually manage the reference, but I do not think this case to much tricky.

so you suggest handling this exclusively in the backend? when "auto", set the default value?

Refactored the Compression.js component to better handle toggling between auto and manual quality settings, preserving the user's manual quality value when switching modes. Updated the minimum allowed quality value in settings.php from 1 to 50 for better image quality control.

ineagu · 2025-10-08T16:33:09Z

works as expected for me

pirate-bot · 2025-10-09T13:03:00Z

🎉 This PR is included in version 4.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

selul requested review from Soare-Robert-Daniel and Copilot October 1, 2025 12:51

Copilot AI reviewed Oct 1, 2025

View reviewed changes

Soare-Robert-Daniel reviewed Oct 1, 2025

View reviewed changes

Soare-Robert-Daniel requested changes Oct 1, 2025

View reviewed changes

ineagu and others added 4 commits October 8, 2025 17:21

fixed linting

be3acff

Update settings.php

b0c2099

fix: handle mauto for quality value on saving

1fe91ed

Soare-Robert-Daniel force-pushed the fix/1516 branch from 33ce46b to 1fe91ed Compare October 8, 2025 14:41

chore: phpstan

124434d

selul merged commit d08e039 into development Oct 9, 2025
11 of 12 checks passed

selul deleted the fix/1516 branch October 9, 2025 07:56

pirate-bot added the released Indicate that an issue has been resolved and released in a particular version of the product. label Oct 9, 2025

-				case 'quality':
+				case 'quality':
+					// Minimum allowed quality was raised from 1 to 50.
+					// Any previously stored values <50 will be coerced to 50 on next save.
+					// Consider informing users who intentionally chose lower quality for stronger compression.
+					if (intval($value) < 50) {
+						// Optional: Log a warning or trigger a deprecation notice for legacy values.
+						trigger_error(
+							'The minimum allowed image quality has been raised from 1 to 50. Values below 50 will be set to 50. Please review your settings.',
+							E_USER_DEPRECATED
+						);
+					}

Improve quality setting handling in Compression settings #978

Improve quality setting handling in Compression settings #978

Uh oh!

Conversation

ineagu commented Oct 1, 2025

All Submissions:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Other information:

Uh oh!

pirate-bot commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Soare-Robert-Daniel Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Soare-Robert-Daniel left a comment

Choose a reason for hiding this comment

Uh oh!

Soare-Robert-Daniel Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

ineagu Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

ineagu commented Oct 8, 2025

Uh oh!

Uh oh!

pirate-bot commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pirate-bot commented Oct 1, 2025 •

edited

Loading