diff --git a/src/settings-ui/Settings.UI.Library/ImageSize.cs b/src/settings-ui/Settings.UI.Library/ImageSize.cs index c902294f2c..4125e3fa31 100644 --- a/src/settings-ui/Settings.UI.Library/ImageSize.cs +++ b/src/settings-ui/Settings.UI.Library/ImageSize.cs @@ -129,5 +129,11 @@ public partial class ImageSize : INotifyPropertyChanged, IHasId [JsonIgnore] public ImageSize AccessibleTextHelper => this; + /// + /// Creates a copy of this . Used to build a working copy for editing + /// so changes can be discarded (on cancel) without touching the original preset. + /// + public ImageSize Clone() => new ImageSize(_id, _name, _fit, _width, _height, _unit); + public string ToJsonString() => JsonSerializer.Serialize(this); } diff --git a/src/settings-ui/Settings.UI.UnitTests/ViewModelTests/ImageResizer.cs b/src/settings-ui/Settings.UI.UnitTests/ViewModelTests/ImageResizer.cs index 5d30cc433c..7b51b26353 100644 --- a/src/settings-ui/Settings.UI.UnitTests/ViewModelTests/ImageResizer.cs +++ b/src/settings-ui/Settings.UI.UnitTests/ViewModelTests/ImageResizer.cs @@ -240,6 +240,71 @@ namespace ViewModelTests Assert.AreEqual(ResizeUnit.Pixel, newTestSize.Unit); } + [TestMethod] + public void CreateNewImageSizeModelShouldNotAddToCollection() + { + // arrange + var mockSettingsUtils = ISettingsUtilsMocks.GetStubSettingsUtils(); + Func sendMockIPCConfigMSG = msg => { return 0; }; + ImageResizerViewModel viewModel = new ImageResizerViewModel(mockSettingsUtils.Object, SettingsRepository.GetInstance(_mockGeneralSettingsUtils.Object), sendMockIPCConfigMSG, (string name) => name); + int sizeOfOriginalArray = viewModel.Sizes.Count; + + // act + ImageSize workingCopy = viewModel.CreateNewImageSizeModel("New size"); + + // Assert - the working copy is populated but not committed to the collection + Assert.IsNotNull(workingCopy); + Assert.AreEqual("New size 1", workingCopy.Name); + Assert.AreEqual(sizeOfOriginalArray, viewModel.Sizes.Count); + } + + [TestMethod] + public void AddImageSizeWithModelShouldCommitPreparedSize() + { + // arrange + var mockSettingsUtils = ISettingsUtilsMocks.GetStubSettingsUtils(); + Func sendMockIPCConfigMSG = msg => { return 0; }; + ImageResizerViewModel viewModel = new ImageResizerViewModel(mockSettingsUtils.Object, SettingsRepository.GetInstance(_mockGeneralSettingsUtils.Object), sendMockIPCConfigMSG, (string name) => name); + int sizeOfOriginalArray = viewModel.Sizes.Count; + ImageSize workingCopy = viewModel.CreateNewImageSizeModel("New size"); + + // act + viewModel.AddImageSize(workingCopy); + + // Assert + Assert.AreEqual(sizeOfOriginalArray + 1, viewModel.Sizes.Count); + Assert.IsTrue(viewModel.Sizes.Contains(workingCopy)); + } + + [TestMethod] + public void UpdateImageSizeShouldApplyWorkingCopyValuesToOriginal() + { + // arrange + var mockSettingsUtils = ISettingsUtilsMocks.GetStubSettingsUtils(); + Func sendMockIPCConfigMSG = msg => { return 0; }; + ImageResizerViewModel viewModel = new ImageResizerViewModel(mockSettingsUtils.Object, SettingsRepository.GetInstance(_mockGeneralSettingsUtils.Object), sendMockIPCConfigMSG, (string name) => name); + viewModel.AddImageSize("Original"); + ImageSize original = viewModel.Sizes.First(x => x.Id == 0); + + ImageSize edited = original.Clone(); + edited.Name = "Edited"; + edited.Fit = ResizeFit.Stretch; + edited.Width = 320; + edited.Height = 240; + edited.Unit = ResizeUnit.Percent; + + // act + viewModel.UpdateImageSize(original, edited); + + // Assert - the edits are applied to the original preset in place + Assert.AreEqual("Edited", original.Name); + Assert.AreEqual(ResizeFit.Stretch, original.Fit); + Assert.AreEqual(320, original.Width); + Assert.AreEqual(240, original.Height); + Assert.AreEqual(ResizeUnit.Percent, original.Unit); + Assert.AreSame(original, viewModel.Sizes.First(x => x.Id == 0)); + } + [TestMethod] public void DeleteImageSizeShouldDeleteImageSizeWhenSuccessful() { diff --git a/src/settings-ui/Settings.UI/Converters/ImageResizerSizeToAccessibleTextConverter.cs b/src/settings-ui/Settings.UI/Converters/ImageResizerSizeToAccessibleTextConverter.cs index 3fd03b08d6..41224fd498 100644 --- a/src/settings-ui/Settings.UI/Converters/ImageResizerSizeToAccessibleTextConverter.cs +++ b/src/settings-ui/Settings.UI/Converters/ImageResizerSizeToAccessibleTextConverter.cs @@ -26,7 +26,6 @@ public sealed partial class ImageResizerSizeToAccessibleTextConverter : IValueCo private static readonly Dictionary AccessibilityFormats = new() { { "Edit", Helpers.ResourceLoaderInstance.ResourceLoader.GetString("ImageResizer_EditButton_Accessibility_Name") }, - { "Remove", Helpers.ResourceLoaderInstance.ResourceLoader.GetString("ImageResizer_RemoveButton_Accessibility_Name") }, }; private readonly ImageResizerFitToStringConverter _fitConverter = new(); diff --git a/src/settings-ui/Settings.UI/SettingsXAML/Views/ImageResizerPage.xaml b/src/settings-ui/Settings.UI/SettingsXAML/Views/ImageResizerPage.xaml index d8fdda09b5..e6ce9f1712 100644 --- a/src/settings-ui/Settings.UI/SettingsXAML/Views/ImageResizerPage.xaml +++ b/src/settings-ui/Settings.UI/SettingsXAML/Views/ImageResizerPage.xaml @@ -59,9 +59,16 @@ SelectionMode="None"> - + 42 + 0 - - - - - + Content="" + FontFamily="{ThemeResource SymbolThemeFontFamily}" + Style="{StaticResource SubtleButtonStyle}"> + + + + + + + + + @@ -283,6 +228,60 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/settings-ui/Settings.UI/SettingsXAML/Views/ImageResizerPage.xaml.cs b/src/settings-ui/Settings.UI/SettingsXAML/Views/ImageResizerPage.xaml.cs index 6a2068d5a8..9c5e0bf351 100644 --- a/src/settings-ui/Settings.UI/SettingsXAML/Views/ImageResizerPage.xaml.cs +++ b/src/settings-ui/Settings.UI/SettingsXAML/Views/ImageResizerPage.xaml.cs @@ -3,26 +3,58 @@ // See the LICENSE file in the project root for more information. using System; -using System.Globalization; +using System.ComponentModel; +using System.Windows.Input; +using CommunityToolkit.WinUI.Controls; using ManagedCommon; using Microsoft.PowerToys.Settings.UI.Helpers; using Microsoft.PowerToys.Settings.UI.Library; using Microsoft.PowerToys.Settings.UI.ViewModels; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; +using Microsoft.Windows.ApplicationModel.Resources; namespace Microsoft.PowerToys.Settings.UI.Views { - public sealed partial class ImageResizerPage : NavigablePage, IRefreshablePage + public sealed partial class ImageResizerPage : NavigablePage, IRefreshablePage, INotifyPropertyChanged { + public event PropertyChangedEventHandler PropertyChanged; + public ImageResizerViewModel ViewModel { get; set; } + public ICommand AddCommand => new RelayCommand(Add); + + public ICommand UpdateCommand => new RelayCommand(Update); + + private readonly ResourceLoader resourceLoader = ResourceLoaderInstance.ResourceLoader; + + // Working copy shown in the edit dialog, bound via x:Bind. Edits happen on this copy so a + // cancel simply discards it; for edits it is a clone of the original, for adds a new model. + private ImageSize _editingSize = new ImageSize(); + + // The original preset being edited, or null when adding a new one. + private ImageSize _editOriginal; + + public ImageSize EditingSize + { + get => _editingSize; + set + { + _editingSize = value; + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(EditingSize))); + } + } + + // Header for the dimensions field. When Height isn't used (e.g. percentage scaling that keeps + // the aspect ratio) the single value scales the whole image, so "Width" would be misleading. + public string GetDimensionHeader(bool isHeightUsed) => + resourceLoader.GetString(isHeightUsed ? "ImageResizer_Dimensions_Width" : "ImageResizer_Dimensions_Size"); + public ImageResizerPage() { InitializeComponent(); var settingsUtils = SettingsUtils.Default; - var resourceLoader = ResourceLoaderInstance.ResourceLoader; Func loader = resourceLoader.GetString; ViewModel = new ImageResizerViewModel(settingsUtils, SettingsRepository.GetInstance(settingsUtils), ShellPage.SendDefaultIPCMessage, loader); @@ -31,49 +63,81 @@ namespace Microsoft.PowerToys.Settings.UI.Views public async void DeleteCustomSize(object sender, RoutedEventArgs e) { - Button deleteRowButton = (Button)sender; - - if (deleteRowButton != null) + if (sender is not MenuFlyoutItem menuItem || menuItem.CommandParameter is not ImageSize size) { - ImageSize x = (ImageSize)deleteRowButton.DataContext; - var resourceLoader = Helpers.ResourceLoaderInstance.ResourceLoader; - - ContentDialog dialog = new ContentDialog(); - dialog.XamlRoot = RootPage.XamlRoot; - dialog.Title = x.Name; - dialog.PrimaryButtonText = resourceLoader.GetString("Yes"); - dialog.CloseButtonText = resourceLoader.GetString("No"); - dialog.DefaultButton = ContentDialogButton.Primary; - dialog.Content = new TextBlock() { Text = resourceLoader.GetString("Delete_Dialog_Description") }; - dialog.PrimaryButtonClick += (s, args) => - { - // Using InvariantCulture since this is internal and expected to be numerical - bool success = int.TryParse(deleteRowButton?.CommandParameter?.ToString(), NumberStyles.Integer, CultureInfo.InvariantCulture, out int rowNum); - if (success) - { - ViewModel.DeleteImageSize(rowNum); - } - else - { - Logger.LogError("Failed to delete custom image size."); - } - }; - var result = await dialog.ShowAsync(); + return; } + + ContentDialog dialog = new ContentDialog(); + dialog.XamlRoot = RootPage.XamlRoot; + dialog.Title = size.Name; + dialog.PrimaryButtonText = resourceLoader.GetString("Yes"); + dialog.CloseButtonText = resourceLoader.GetString("No"); + dialog.DefaultButton = ContentDialogButton.Primary; + dialog.Content = new TextBlock() { Text = resourceLoader.GetString("Delete_Dialog_Description") }; + dialog.PrimaryButtonClick += (s, args) => + { + ViewModel.DeleteImageSize(size.Id); + }; + await dialog.ShowAsync(); } - private void AddSizeButton_Click(object sender, RoutedEventArgs e) + private async void AddSizeButton_Click(object sender, RoutedEventArgs e) { try { - ViewModel.AddImageSize(); + _editOriginal = null; + EditingSize = ViewModel.CreateNewImageSizeModel(); + EditSizeDialog.Title = resourceLoader.GetString("ImageResizer_EditSizeDialog_AddTitle"); + EditSizeDialog.PrimaryButtonText = resourceLoader.GetString("ImageResizer_EditSizeDialog_Save"); + EditSizeDialog.PrimaryButtonCommand = AddCommand; + await EditSizeDialog.ShowAsync(); } catch (Exception ex) { - Logger.LogError("Exception encountered when adding a new image size.", ex); + Logger.LogError("Exception encountered when opening the add image size dialog.", ex); } } + private async void EditSize_Click(object sender, RoutedEventArgs e) + { + if (sender is not SettingsCard card || card.DataContext is not ImageSize original) + { + return; + } + + try + { + // Edit a working copy so changes can be discarded on cancel without touching the original. + _editOriginal = original; + EditingSize = original.Clone(); + EditSizeDialog.Title = resourceLoader.GetString("ImageResizer_EditSizeDialog_EditTitle"); + EditSizeDialog.PrimaryButtonText = resourceLoader.GetString("ImageResizer_EditSizeDialog_Update"); + EditSizeDialog.PrimaryButtonCommand = UpdateCommand; + await EditSizeDialog.ShowAsync(); + } + catch (Exception ex) + { + Logger.LogError("Exception encountered when opening the edit image size dialog.", ex); + } + } + + private void Add() + { + ViewModel.AddImageSize(EditingSize); + EditSizeDialog.Hide(); + } + + private void Update() + { + if (_editOriginal != null) + { + ViewModel.UpdateImageSize(_editOriginal, EditingSize); + } + + EditSizeDialog.Hide(); + } + private void ImagesSizesListView_ContainerContentChanging(ListViewBase sender, ContainerContentChangingEventArgs args) { if (ViewModel.IsListViewFocusRequested) diff --git a/src/settings-ui/Settings.UI/Strings/en-us/Resources.resw b/src/settings-ui/Settings.UI/Strings/en-us/Resources.resw index 2c1c0734d0..e4904b53c9 100644 --- a/src/settings-ui/Settings.UI/Strings/en-us/Resources.resw +++ b/src/settings-ui/Settings.UI/Strings/en-us/Resources.resw @@ -1218,9 +1218,6 @@ opera.exe Fit - - Width - Height @@ -1233,6 +1230,29 @@ opera.exe Add new size + + Cancel + + + Add new size + + + Edit size + + + Save + + + Update + + + Width + Header for the width input field in the Image Resizer edit size dialog + + + Size + Header shown instead of "Width" when a single value scales the whole image (e.g. percentage), in the Image Resizer edit size dialog + JPEG quality level (%) {Locked="JPEG"} @@ -2379,29 +2399,16 @@ From there, simply click on one of the supported files in the File Explorer and Pixels - - Edit - - - Edit size - ImageResizer presets Add a new preset - - Remove - Edit the {0} preset Expands to the AutomationProperties.Name value for the Edit button. Example: "Edit the Small preset". - - Remove the {0} preset - Expands to the AutomationProperties.Name value for the Remove button. Example: "Remove the Large preset". - No Label of a cancel button @@ -2642,9 +2649,6 @@ From there, simply click on one of the supported files in the File Explorer and Edit - - Remove - Cancel diff --git a/src/settings-ui/Settings.UI/ViewModels/ImageResizerViewModel.cs b/src/settings-ui/Settings.UI/ViewModels/ImageResizerViewModel.cs index 44a435271d..0fbfc38c98 100644 --- a/src/settings-ui/Settings.UI/ViewModels/ImageResizerViewModel.cs +++ b/src/settings-ui/Settings.UI/ViewModels/ImageResizerViewModel.cs @@ -318,19 +318,41 @@ public partial class ImageResizerViewModel : Observable } public void AddImageSize(string namePrefix = "") + { + AddImageSize(CreateNewImageSizeModel(namePrefix)); + } + + /// + /// Creates a new preset populated with default values and a generated unique name, without + /// adding it to the collection. Used as the working copy for the add dialog + /// (so nothing is committed until the user confirms) and as the source for . + /// + public ImageSize CreateNewImageSizeModel(string namePrefix = "") { if (string.IsNullOrEmpty(namePrefix)) { namePrefix = DefaultPresetNamePrefix; } - Sizes.Add(new ImageSize( + return new ImageSize( _nextId, GenerateNameForNewSize(namePrefix), _customSize.Fit, _customSize.Width, _customSize.Height, - _customSize.Unit)); + _customSize.Unit); + } + + /// + /// Commits a preset created via to the + /// collection, assigning it the next available unique ID. + /// + public void AddImageSize(ImageSize size) + { + ArgumentNullException.ThrowIfNull(size); + + size.Id = _nextId; + Sizes.Add(size); _nextId++; @@ -338,6 +360,29 @@ public partial class ImageResizerViewModel : Observable IsListViewFocusRequested = true; } + /// + /// Applies the values from an edited working copy back onto the original preset, saving once. + /// + public void UpdateImageSize(ImageSize original, ImageSize updated) + { + ArgumentNullException.ThrowIfNull(original); + ArgumentNullException.ThrowIfNull(updated); + + // Temporarily detach the per-item save handler so the individual property updates below + // don't each trigger a save; persist once at the end instead. + original.PropertyChanged -= SizePropertyChanged; + + original.Name = updated.Name; + original.Fit = updated.Fit; + original.Width = updated.Width; + original.Height = updated.Height; + original.Unit = updated.Unit; + + original.PropertyChanged += SizePropertyChanged; + + SaveImageSizes(); + } + public void DeleteImageSize(int id) { ImageSize size = _sizes.First(x => x.Id == id);