Improve handling of invalid icon IDs

Fixes #32, #33

OfficeRibbonXEditor/ViewModels/IconViewModel.cs

@@ -9,20 +9,30 @@
 
 namespace OfficeRibbonXEditor.ViewModels
 {
+    using System.Collections.Generic;
     using System.IO;
+    using System.Windows;
     using System.Windows.Media.Imaging;
+    using System.Xml;
+
+    using CommonServiceLocator;
+
+    using OfficeRibbonXEditor.Services;
 
     public class IconViewModel : TreeViewItemViewModel
     {
         private string id;
 
+        private string newId;
+
         private bool isEditingId;
 
         public IconViewModel(string id, BitmapImage image, OfficePartViewModel parent)
             : base(parent, false, false)
         {
             this.Image = image;
             this.id = id;
+            this.newId = id;
         }
 
         public IconViewModel(string id, string filePath, OfficePartViewModel parent) 
@@ -32,31 +42,104 @@ public IconViewModel(string id, string filePath, OfficePartViewModel parent)
 
         public BitmapImage Image { get; }
 
+        /// <summary>
+        /// Gets or sets the Id of the icon, applying the changes directly to the underlying model
+        /// </summary>
         public string Id
         {
             get => this.id;
             set
             {
-                // Make sure this.ChangeId() is called with the previous ID and not the new one already. Otherwise,
-                // the icon will actually not be updated inside the part
+                if (!IsValidId(value))
+                {
+                    return;
+                }
+
+                // Make sure this.ChangeId() is later called with the previous ID and not the new one
+                // already. Otherwise, the icon will actually not be updated inside the part
                 var previousId = this.id;
-                if (this.Set(ref this.id, value))
+
+                if (!this.Set(ref this.id, value))
                 {
-                    this.ChangeId(previousId, value);
+                    return;
                 }
+
+                this.ChangeId(previousId, value);
             }
         }
 
+        /// <summary>
+        /// Gets or sets the potentially new ID to be used for the icon. This is used, for example, in
+        /// editing mode before committing to use the new ID (in case the user discard the changes)
+        /// </summary>
+        public string NewId
+        {
+            get => this.newId;
+            set => this.Set(ref this.newId, value);
+        }
+
         public bool IsEditingId
         {
             get => this.isEditingId;
             set => this.Set(ref this.isEditingId, value);
         }
-
+
+        /// <summary>
+        /// Attempts to apply the NewId property to the Id one, but cancels the action if the ID is invalid
+        /// </summary>
+        public void CommitIdChange()
+        {
+            this.IsEditingId = false;
+
+            if (!IsValidId(this.NewId, out var errorMessage))
+            {
+                // Revert back the change
+                this.NewId = this.Id;
+                ServiceLocator.Current.GetInstance<IMessageBoxService>()?.Show(
+                    errorMessage, 
+                    "Error Changing Icon ID", 
+                    MessageBoxButton.OK, 
+                    MessageBoxImage.Error);
+                return;
+            }
+
+            this.Id = this.NewId;
+        }
+
+        /// <summary>
+        /// Ignores the current value of NewId, setting it back to be equal to Id
+        /// </summary>
+        public void DiscardIdChange()
+        {
+            this.NewId = this.Id;
+            this.IsEditingId = false;
+        }
+
         protected override void SelectionLost()
         {
             this.IsEditingId = false;
         }
+
+        private static bool IsValidId(string id)
+        {
+            return IsValidId(id, out _);
+        }
+
+        private static bool IsValidId(string id, out string errorMessage)
+        {
+            try
+            {
+                XmlConvert.VerifyName(id);
+            }
+            catch (XmlException e)
+            {
+                errorMessage = e.Message;
+                return false;
+            }
+
+            errorMessage = string.Empty;
+            return true;
+        }
 
         private static BitmapImage LoadImageFromPath(string filePath)
         {

OfficeRibbonXEditor/ViewModels/MainWindowViewModel.cs

@@ -171,7 +171,7 @@ public TreeViewItemViewModel SelectedItem
                 {
                     // Stop showing the editing textbox when the focus changes to something else.
                     // See: https://github.com/fernandreu/office-ribbonx-editor/issues/32
-                    icon.IsEditingId = false;
+                    icon.CommitIdChange();
                 }
 
                 if (this.SelectedItem != null)

OfficeRibbonXEditor/Views/MainWindow.xaml

@@ -404,7 +404,7 @@
                         <StackPanel Orientation="Horizontal">
                             <Image Width="16" Height="16" Margin="3,0" Source="{Binding Image}"/>
                             <TextBlock Text="{Binding Id}" Visibility="{Binding IsEditingId, Converter={StaticResource InverseBooleanToVisibilityConverter}}"/>
-                            <TextBox Text="{Binding Id}" Visibility="{Binding IsEditingId, Converter={StaticResource BooleanToVisibilityConverter}}" KeyDown="OnChangeIdTextDown" IsVisibleChanged="OnIdTextVisible"/>
+                            <TextBox Text="{Binding NewId, UpdateSourceTrigger=PropertyChanged}" Visibility="{Binding IsEditingId, Converter={StaticResource BooleanToVisibilityConverter}}" KeyDown="OnChangeIdTextDown" IsVisibleChanged="OnIdTextVisible"/>
                             <StackPanel.ContextMenu>
                                 <ContextMenu>
                                     <MenuItem Header="Change ID" InputGestureText="F2" Command="{Binding Main.ChangeIconIdCommand, Source={StaticResource Locator}}"/>

OfficeRibbonXEditor/Views/MainWindow.xaml.cs

@@ -198,17 +198,24 @@ private void OnShowAboutDialog(object sender, RoutedEventArgs e)
 
         private void OnChangeIdTextDown(object sender, KeyEventArgs e)
         {
-            if (e.Key != Key.Enter)
+            if (!(sender is TextBox textBox))
             {
                 return;
             }
 
-            if (!(sender is TextBox textBox))
+            if (!(textBox.DataContext is IconViewModel icon))
             {
                 return;
             }
-
-            textBox.MoveFocus(new TraversalRequest(FocusNavigationDirection.Next));
+
+            if (e.Key == Key.Enter)
+            {
+                icon.CommitIdChange();
+            }
+            else if (e.Key == Key.Escape)
+            {
+                icon.DiscardIdChange();
+            }
         }
 
         private void OnIdTextVisible(object sender, DependencyPropertyChangedEventArgs e)