Add menu items for moving fields up and down in the interface (#127)

* Add menu items and handlers for move up/down for each field slice
* Also clean up some obsolete code

Co-authored-by: Jake Oliver <jeoliver97@gmail.com>

DistFiles/Language Explorer/Configuration/CommonDataTreeInclude.xml

@@ -5,6 +5,9 @@
 	<command id="CmdDataTree-Delete" label="Delete {0}" message="DataTreeDelete" icon="Delete"/>
 	<command id="CmdDataTree-WritingSystemMenu-ShowAllRightNow" label="Show all right now" message="DataTreeWritingSystemsShowAll"/>
 	<command id="CmdDataTree-WritingSystemMenu-Configure" label="Configure..." message="DataTreeWritingSystemsConfigureDlg"/>
+	<!-- For moving fields up and down in the detail view -->
+	<command id="CmdDataTree-MoveFieldUp" label="Move Up" message="MoveFieldUp" />
+	<command id="CmdDataTree-MoveFieldDown" label="Move Down" message="MoveFieldDown" />
   </commands>
   <contextMenus>
 	<menu id="mnuDataTree-Object">
@@ -15,6 +18,10 @@
 			<item command="CmdIfData"/>
 			<item command="CmdNormallyHidden"/>
 		</menu>
+		<menu id="mnuMove" label="Move">
+			<item command="CmdDataTree-MoveFieldUp"/>
+			<item command="CmdDataTree-MoveFieldDown"/>
+		</menu>
 		<item command="CmdDataTree-Help"/>
 	</menu>
 	<menu id="mnuDataTree-MultiStringSlice">
@@ -31,6 +38,10 @@
 				  behavior="singlePropertySequenceValue" property="SelectedWritingSystemHvosForCurrentContextMenu" defaultPropertyValue=""/>
 			<item command="CmdDataTree-WritingSystemMenu-Configure"/>
 		</menu>
+		<menu id="mnuMove" label="Move">
+			<item command="CmdDataTree-MoveFieldUp"/>
+			<item command="CmdDataTree-MoveFieldDown"/>
+		</menu>
 		<item command="CmdDataTree-Help"/>
 	 </menu>
 	  <!--A unique ShowHiddenFields menu is required for all tools so they can have their own boolProperty-->

Src/Common/Controls/DetailControls/ButtonLauncher.cs

@@ -47,12 +47,7 @@ protected Slice Slice
 		{
 			get
 			{
-				// Depending on compile switch for SLICE_IS_SPLITCONTAINER,
-				// grandParent will be both a Slice and a SplitContainer
-				// (Slice is a subclass of SplitContainer),
-				// or just a SplitContainer (SplitContainer is the only child Control of a Slice).
-				// If grandParent is not a Slice, then we have to move up to the great-grandparent
-				// to find the Slice.
+				// Return the Slice parent of this button, even if the button buried in other controls
 				Control parent = Parent;
 				while (!(parent is Slice))
 					parent = parent.Parent;

Src/Common/Controls/DetailControls/DataTree.cs

@@ -54,6 +54,11 @@ namespace SIL.FieldWorks.Common.Framework.DetailControls
 	/// System.Windows.Forms.UserControl
 	public class DataTree : UserControl, IVwNotifyChange, IxCoreColleague, IRefreshableRoot
 	{
+		/// <summary>
+		/// Part refs that don't represent actual data slices
+		/// </summary>
+		public static string[] SpecialPartRefs = { "ChangeHandler", "_CustomFieldPlaceholder" };
+
 		/// <summary>
 		/// Occurs when the current slice changes
 		/// </summary>
@@ -294,13 +299,9 @@ void slice_SplitterMoved(object sender, SplitterEventArgs e)
 			if (m_currentSlice == null)
 				return; // Too early to do much;
 
-			// Depending on compile switch for SLICE_IS_SPLITCONTAINER,
-			// the sender will be both a Slice and a SplitContainer
-			// (Slice is a subclass of SplitContainer),
-			// or just a SplitContainer (SplitContainer is the only child Control of a Slice).
-			Slice movedSlice = sender is Slice ? (Slice) sender
+			var movedSlice = sender is Slice slice ? slice
 				// sender is also a SplitContainer.
-				: (Slice) ((SplitContainer) sender).Parent; // Have to move up one parent notch to get to teh Slice.
+				: (Slice) ((SplitContainer) sender).Parent; // Review: This branch is probably obsolete.
 			if (m_currentSlice != movedSlice)
 				return; // Too early to do much;
 

Src/Common/Controls/DetailControls/Slice.cs

@@ -28,6 +28,11 @@
 
 namespace SIL.FieldWorks.Common.Framework.DetailControls
 {
+	enum Direction
+	{
+		Up, Down
+	}
+
 	/// <summary>
 	/// A Slice is essentially one row of a tree.
 	/// It contains both a SliceTreeNode on the left of the splitter line, and a
@@ -37,22 +42,8 @@ namespace SIL.FieldWorks.Common.Framework.DetailControls
 	/// within the tree for this item, knowing whether the item can be expanded,
 	/// and optionally drawing the part of the tree that is opposite the item, and
 	/// many other things.}
-#if SLICE_IS_SPLITCONTAINER
-	/// The problem I (RandyR) ran into with this is when the DataTree scrolled and reset the Top of the slice,
-	/// the internal SplitterRectangle ended up being non-0 in many cases,
-	/// which resulted in the splitter not be in the right place (visible)
-	/// The MS docs say in a vertical orientation like this, the 'Y"
-	/// value of SplitterRectangle will always be 0.
-	/// I don't know if it is a bug in the MS code or in our code that lets it be non-0,
-	/// but I worked with it quite a while without finding the true problem.
-	/// So, I went back to a Slice having a SplitContainer,
-	/// rather than the better option of it being a SplitContainer.
-	///</remarks>
-	public class Slice : SplitContainer, IxCoreColleague
-#else
 	///</remarks>
 	public class Slice : UserControl, IxCoreColleague
-#endif
 	{
 		#region Constants
 
@@ -225,11 +216,7 @@ protected internal SplitContainer SplitCont
 			{
 				CheckDisposed();
 
-#if SLICE_IS_SPLITCONTAINER
-				return this;
-#else
 				return Controls[0] as SplitContainer;
-#endif
 			}
 		}
 
@@ -468,17 +455,13 @@ public ImageCollection SmallImages
 		/// <summary></summary>
 		public Slice()
 		{
-#if SLICE_IS_SPLITCONTAINER
-			TabStop = false;
-#else
 			// Create a SplitContainer to hold the two (or one control.
 			m_splitter = new SplitContainer {TabStop = false, AccessibleName = "Slice.SplitContainer"};
 			// Do this once right away, mainly so child controls like check box that don't control
 			// their own height will get it right; then  after the controls get added to it, don't do it again
 			// until our own size is definitely established by SetWidthForDataTreeLayout.
 			m_splitter.Size = Size;
 			Controls.Add(m_splitter);
-#endif
 			// This is really important. Since some slices are invisible, all must be,
 			// or Show() will reorder them.
 			Visible = false;
@@ -528,7 +511,7 @@ public virtual void RegisterWithContextHelper()
 		{
 			CheckDisposed();
 
-			if (Control != null)//grouping nodes do not have a control
+			if (Control != null) //grouping nodes do not have a control
 			{
 				//It's OK to send null as an id
 				if (m_mediator != null) // helpful for robustness and testing.
@@ -2801,6 +2784,70 @@ internal protected virtual bool UpdateDisplayIfNeeded(int hvo, int tag)
 			return false;
 		}
 
+		private void MoveField(Direction dir)
+		{
+			CheckDisposed();
+			if (ContainingDataTree.ShowingAllFields)
+			{
+				XmlNode swapWith;
+				XmlNode fieldRef = FieldReferenceForSlice();
+
+				if (fieldRef == null)
+				{
+					Debug.Fail("Could not identify field to move on slice.");
+					return;
+				}
+
+				if (dir == Direction.Up)
+				{
+					swapWith = PrevPartSibling(fieldRef);
+				}
+				else
+				{
+					swapWith = NextPartSibling(fieldRef.NextSibling);
+				}
+
+				var parent = fieldRef.ParentNode;
+				// Reorder in the parent node in the xml
+				if (parent != null)
+				{
+					parent.RemoveChild(fieldRef);
+					if (dir == Direction.Up)
+						parent.InsertBefore(fieldRef, swapWith);
+					else
+						parent.InsertAfter(fieldRef, swapWith);
+				}
+
+				// Persist in the parent part (might not be the immediate parent node)
+				Inventory.GetInventory("layouts", m_cache.ProjectId.Name)
+					.PersistOverrideElement(PartParent(fieldRef));
+				ContainingDataTree.RefreshList(true);
+			}
+		}
+
+		/// <summary>
+		/// Find the last part ref in the Key which represents the part of the data and configuration for this slice.
+		/// This is built up in DataTree with the path to the part in the combined layout and parts configuration files.
+		/// There may be other part refs in the path if this slice represents a subfield.
+		/// </summary>
+		private XmlNode FieldReferenceForSlice()
+		{
+			XmlNode fieldRef = null;
+			foreach (object obj in Key)
+			{
+				var node = obj as XmlNode;
+				if (node == null || node.Name != "part" ||
+					XmlUtils.GetOptionalAttributeValue(node, "ref", null) == null)
+				{
+					continue;
+				}
+
+				fieldRef = node;
+			}
+
+			return fieldRef;
+		}
+
 		protected void SetFieldVisibility(string visibility)
 		{
 			CheckDisposed();
@@ -2907,15 +2954,90 @@ protected bool IsVisibilityItemChecked(string visibility)
 		{
 			CheckDisposed();
 
-			XmlNode lastPartRef = null;
-			foreach (object obj in Key)
+			var lastPartRef = FieldReferenceForSlice();
+
+			return lastPartRef != null &&
+				   XmlUtils.GetOptionalAttributeValue(lastPartRef, "visibility", "always") ==
+				   visibility;
+		}
+
+		private bool CheckValidMove(UIItemDisplayProperties display, Direction dir)
+		{
+			XmlNode lastPartRef = FieldReferenceForSlice();
+
+			if (lastPartRef == null)
+				return false;
+			return dir == Direction.Up
+				? PrevPartSibling(lastPartRef) != null
+				: NextPartSibling(lastPartRef) != null;
+		}
+
+		private XmlNode PrevPartSibling(XmlNode partRef)
+		{
+			XmlNode prev = partRef.PreviousSibling;
+			while (prev != null && (prev.NodeType != XmlNodeType.Element || prev.Name != "part" ||
+				XmlUtils.GetOptionalAttributeValue(prev, "ref", null) == null ||
+				DataTree.SpecialPartRefs.Contains(XmlUtils.GetOptionalAttributeValue(prev, "ref"))))
 			{
-				var node = obj as XmlNode;
-				if (node == null || node.Name != "part" || XmlUtils.GetOptionalAttributeValue(node, "ref", null) == null)
-					continue;
-				lastPartRef = node;
+				prev = prev.PreviousSibling;
 			}
-			return lastPartRef != null && XmlUtils.GetOptionalAttributeValue(lastPartRef, "visibility", "always") == visibility;
+			return prev;
+		}
+
+		private XmlNode NextPartSibling(XmlNode partRef)
+		{
+			XmlNode next = partRef.NextSibling;
+			while (next != null && (next.NodeType != XmlNodeType.Element || next.Name != "part" ||
+				XmlUtils.GetOptionalAttributeValue(next, "ref", null) == null))
+			{
+				next = next.NextSibling;
+			}
+			return next;
+		}
+
+		private XmlNode PartParent(XmlNode partRef)
+		{
+			XmlNode parent = partRef.ParentNode;
+			while (parent != null && (parent.NodeType != XmlNodeType.Element || (parent.Name != "part" && parent.Name != "layout")))
+			{
+				parent = parent.ParentNode;
+			}
+			if(parent == null)
+				throw new ConfigurationException("Could not find parent part node", m_configurationNode);
+			return parent;
+		}
+
+		/// <summary></summary>
+		public bool OnDisplayMoveFieldUp(object args, ref UIItemDisplayProperties display)
+		{
+			CheckDisposed();
+			display.Enabled = ContainingDataTree.ShowingAllFields && CheckValidMove(display, Direction.Up);
+
+			return true;
+		}
+
+		/// <summary></summary>
+		public bool OnDisplayMoveFieldDown(object args, ref UIItemDisplayProperties display)
+		{
+			CheckDisposed();
+			display.Enabled = ContainingDataTree.ShowingAllFields && CheckValidMove(display, Direction.Down);
+			return true;
+		}
+
+		/// <summary></summary>
+		public bool OnMoveFieldUp(object args)
+		{
+			CheckDisposed();
+			MoveField(Direction.Up);
+			return true;
+		}
+
+		/// <summary></summary>
+		public bool OnMoveFieldDown(object args)
+		{
+			CheckDisposed();
+			MoveField(Direction.Down);
+			return true;
 		}
 
 		/// <summary></summary>

Src/Common/Controls/DetailControls/SliceTreeNode.cs

@@ -42,9 +42,7 @@ public class SliceTreeNode : UserControl
 		internal const int kdxpLeftMargin = 2; // Gap at the far left of everything.
 		#endregion
 
-		protected bool m_inMenuButton = false;
-
-		private bool m_fShowPlusMinus = false;
+		private bool m_fShowPlusMinus;
 		/// <summary>
 		/// Required designer variable.
 		/// </summary>
@@ -57,13 +55,8 @@ public Slice Slice
 			{
 				CheckDisposed();
 
-				// Depending on compile switch for SLICE_IS_SPLITCONTAINER,
-				// grandParent will be both a Slice and a SplitContainer
-				// (Slice is a subclass of SplitContainer),
-				// or just a SplitContainer (SplitContainer is the only child Control of a Slice).
-				// If grandParent is not a Slice, then we have to move up to the great-grandparent
-				// to find the Slice.
-				Control parent = Parent;
+				// Return the Slice parent of this button, even if the button buried in other controls
+				var parent = Parent;
 				while (!(parent is Slice))
 					parent = parent.Parent;