Fix circular logic in percent value restoration with flag guard

Co-authored-by: yeelam-gordon <73506701+yeelam-gordon@users.noreply.github.com>

src/settings-ui/Settings.UI.Library/ImageSize.cs

@@ -32,12 +32,19 @@ public partial class ImageSize : INotifyPropertyChanged, IHasId
 
     public ImageSize(int id = 0, string name = "", ResizeFit fit = ResizeFit.Fit, double width = 0, double height = 0, ResizeUnit unit = ResizeUnit.Pixel)
     {
-        Id = id;
-        Name = name;
-        Fit = fit;
-        Width = width;
-        Height = height;
-        Unit = unit;
+        _id = id;
+        _name = name;
+        _fit = fit;
+        _width = width < 0 || double.IsNaN(width) ? 0 : width;
+        _height = height < 0 || double.IsNaN(height) ? 0 : height;
+        _unit = unit;
+        
+        // If constructed with Percent unit, store these as the last percent values
+        if (unit == ResizeUnit.Percent)
+        {
+            _lastPercentWidth = _width;
+            _lastPercentHeight = _height;
+        }
     }
 
     private int _id;
@@ -46,6 +53,13 @@ public partial class ImageSize : INotifyPropertyChanged, IHasId
     private double _height;
     private double _width;
     private ResizeUnit _unit;
+    
+    // Store last percent values to restore when switching back to Percent
+    private double _lastPercentWidth = 100.0;
+    private double _lastPercentHeight = 100.0;
+    
+    // Flag to prevent updating stored percent values during restoration
+    private bool _isRestoringPercentValues = false;
 
     public int Id
     {
@@ -89,14 +103,36 @@ public partial class ImageSize : INotifyPropertyChanged, IHasId
     public double Width
     {
         get => _width;
-        set => SetProperty(ref _width, value < 0 || double.IsNaN(value) ? 0 : value);
+        set
+        {
+            var newValue = value < 0 || double.IsNaN(value) ? 0 : value;
+            if (SetProperty(ref _width, newValue))
+            {
+                // Store the value if we're currently in Percent unit (but not during restoration)
+                if (_unit == ResizeUnit.Percent && !_isRestoringPercentValues)
+                {
+                    _lastPercentWidth = newValue;
+                }
+            }
+        }
     }
 
     [JsonPropertyName("height")]
     public double Height
     {
         get => _height;
-        set => SetProperty(ref _height, value < 0 || double.IsNaN(value) ? 0 : value);
+        set
+        {
+            var newValue = value < 0 || double.IsNaN(value) ? 0 : value;
+            if (SetProperty(ref _height, newValue))
+            {
+                // Store the value if we're currently in Percent unit (but not during restoration)
+                if (_unit == ResizeUnit.Percent && !_isRestoringPercentValues)
+                {
+                    _lastPercentHeight = newValue;
+                }
+            }
+        }
     }
 
     [JsonPropertyName("unit")]
@@ -105,8 +141,19 @@ public partial class ImageSize : INotifyPropertyChanged, IHasId
         get => _unit;
         set
         {
+            var previousUnit = _unit;
             if (SetProperty(ref _unit, value))
             {
+                // When switching to Percent unit from another unit, restore last percent values
+                // or default to 100% if this is the first time switching to Percent
+                if (value == ResizeUnit.Percent && previousUnit != ResizeUnit.Percent)
+                {
+                    _isRestoringPercentValues = true;
+                    Width = _lastPercentWidth;
+                    Height = _lastPercentHeight;
+                    _isRestoringPercentValues = false;
+                }
+                
                 PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(IsHeightUsed)));
             }
         }

src/settings-ui/Settings.UI.UnitTests/ModelsTests/ImageSizeTests.cs

@@ -0,0 +1,119 @@
+// Copyright (c) Microsoft Corporation
+// The Microsoft Corporation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using Microsoft.PowerToys.Settings.UI.Library;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace Microsoft.PowerToys.Settings.UI.UnitTests.ModelsTests
+{
+    [TestClass]
+    public class ImageSizeTests
+    {
+        [TestMethod]
+        public void WhenUnitChangesToPercentFirstTime_WidthShouldBeSetTo100()
+        {
+            // Arrange
+            var imageSize = new ImageSize(1, "Test", ResizeFit.Fit, 854, 480, ResizeUnit.Pixel);
+            
+            // Act
+            imageSize.Unit = ResizeUnit.Percent;
+            
+            // Assert
+            Assert.AreEqual(100.0, imageSize.Width, "Width should be set to 100 when switching to Percent unit for the first time");
+        }
+        
+        [TestMethod]
+        public void WhenUnitChangesToPercentFirstTime_HeightShouldBeSetTo100()
+        {
+            // Arrange
+            var imageSize = new ImageSize(1, "Test", ResizeFit.Stretch, 854, 480, ResizeUnit.Pixel);
+            
+            // Act
+            imageSize.Unit = ResizeUnit.Percent;
+            
+            // Assert
+            Assert.AreEqual(100.0, imageSize.Height, "Height should be set to 100 when switching to Percent unit for the first time");
+        }
+        
+        [TestMethod]
+        public void WhenUnitChangesFromPercentToPixel_ValuesShouldNotChange()
+        {
+            // Arrange
+            var imageSize = new ImageSize(1, "Test", ResizeFit.Fit, 50, 75, ResizeUnit.Percent);
+            
+            // Act
+            imageSize.Unit = ResizeUnit.Pixel;
+            
+            // Assert
+            Assert.AreEqual(50.0, imageSize.Width, "Width should remain unchanged when switching from Percent to other units");
+            Assert.AreEqual(75.0, imageSize.Height, "Height should remain unchanged when switching from Percent to other units");
+        }
+        
+        [TestMethod]
+        public void WhenUnitRemainsPercent_ValuesShouldNotChange()
+        {
+            // Arrange
+            var imageSize = new ImageSize(1, "Test", ResizeFit.Fit, 75, 60, ResizeUnit.Percent);
+            
+            // Act
+            imageSize.Unit = ResizeUnit.Percent;
+            
+            // Assert
+            Assert.AreEqual(75.0, imageSize.Width, "Width should remain unchanged when unit stays as Percent");
+            Assert.AreEqual(60.0, imageSize.Height, "Height should remain unchanged when unit stays as Percent");
+        }
+        
+        [TestMethod]
+        public void WhenSwitchingBackToPercent_PreviousPercentValuesShouldBeRestored()
+        {
+            // Arrange - Start with 50% width, 75% height
+            var imageSize = new ImageSize(1, "Test", ResizeFit.Stretch, 50, 75, ResizeUnit.Percent);
+            
+            // Act - Switch to Pixel (values become 50 and 75 pixels)
+            imageSize.Unit = ResizeUnit.Pixel;
+            
+            // Change the pixel values
+            imageSize.Width = 1920;
+            imageSize.Height = 1080;
+            
+            // Switch back to Percent
+            imageSize.Unit = ResizeUnit.Percent;
+            
+            // Assert - Should restore the previous percent values (50, 75), not the pixel values
+            Assert.AreEqual(50.0, imageSize.Width, "Width should be restored to previous percent value (50) when switching back to Percent");
+            Assert.AreEqual(75.0, imageSize.Height, "Height should be restored to previous percent value (75) when switching back to Percent");
+        }
+        
+        [TestMethod]
+        public void WhenModifyingPercentValues_NewValuesShouldBeRemembered()
+        {
+            // Arrange - Start with default 100%
+            var imageSize = new ImageSize(1, "Test", ResizeFit.Stretch, 1920, 1080, ResizeUnit.Pixel);
+            imageSize.Unit = ResizeUnit.Percent;
+            
+            // Modify to 60% width, 80% height
+            imageSize.Width = 60;
+            imageSize.Height = 80;
+            
+            // Act - Switch to Pixel and back
+            imageSize.Unit = ResizeUnit.Pixel;
+            imageSize.Unit = ResizeUnit.Percent;
+            
+            // Assert - Should remember the modified values (60, 80)
+            Assert.AreEqual(60.0, imageSize.Width, "Modified percent width (60) should be restored");
+            Assert.AreEqual(80.0, imageSize.Height, "Modified percent height (80) should be restored");
+        }
+        
+        [TestMethod]
+        public void WhenConstructedWithPercent_ValuesShouldBePreserved()
+        {
+            // Arrange & Act
+            var imageSize = new ImageSize(1, "Test", ResizeFit.Fit, 50, 75, ResizeUnit.Percent);
+            
+            // Assert
+            Assert.AreEqual(50.0, imageSize.Width, "Width should be preserved when constructed with Percent unit");
+            Assert.AreEqual(75.0, imageSize.Height, "Height should be preserved when constructed with Percent unit");
+        }
+    }
+}