diff --git a/Src/XCore/FlexUIAdapter/NavBarAdapter.cs b/Src/XCore/FlexUIAdapter/NavBarAdapter.cs index 17e24df45c..8fab2c4bc0 100644 --- a/Src/XCore/FlexUIAdapter/NavBarAdapter.cs +++ b/Src/XCore/FlexUIAdapter/NavBarAdapter.cs @@ -26,6 +26,7 @@ public class SidebarAdapter : AdapterBase, IxCoreColleague, IDisposable private ChoiceGroupCollection m_choiceGroupCollectionCache; // cache of what was received by CreateUIForChoiceGroupCollection() private List m_choiceGroupCache = new List(); // cache of what was received by CreateUIForChoiceGroup() private string areasLabel; // label of the special case ChoiceGroup for Areas + private bool m_selectingInSidePane; // true while the adapter's own SelectTab/SelectItem runs #region Disposable stuff #if DEBUG @@ -141,10 +142,13 @@ public void Init(Mediator mediator, PropertyTable propertyTable, XmlNode configu } /// - /// Handle when an item on the sidepane is clicked. + /// Handle when an item on the sidepane is clicked. A selection the adapter itself makes + /// while reflecting a property change raises the same event and is ignored here. /// private void SidePaneItemClickedHandler(Item itemClicked) { + if (m_selectingInSidePane) + return; var choice = itemClicked.Tag as ChoiceBase; Debug.Assert(choice != null, "SidePaneItemClickedHandler received an Item with an invalid Tag."); if (choice != null) @@ -152,16 +156,36 @@ private void SidePaneItemClickedHandler(Item itemClicked) } /// - /// Handle when a tab on the sidepane is clicked. + /// Handle when a tab on the sidepane is clicked. A selection the adapter itself makes + /// while reflecting a property change raises the same event and is ignored here. /// private void SidePaneTabClickedHandler(Tab tabClicked) { + if (m_selectingInSidePane) + return; var choice = tabClicked.Tag as ChoiceBase; Debug.Assert(choice != null, "SidePaneTabClickedHandler received a Tab with an invalid Tag."); if (choice != null) choice.OnClick(this, null); } + /// + /// Runs one of the side pane's selection calls with the click handlers silenced, so + /// that showing a property's value does not set that property again. + /// + private bool SelectInSidePane(Func select) + { + m_selectingInSidePane = true; + try + { + return select(); + } + finally + { + m_selectingInSidePane = false; + } + } + public override void PersistLayout() { // use MyPanel @@ -555,12 +579,12 @@ public void OnPropertyChanged(string propertyName) // overwriting the property table value. See FWR-2004. string propToolForArea = "ToolForAreaNamed_" + areaName; string toolForArea = m_propertyTable.GetStringProperty(propToolForArea, null); - m_sidepane.SelectTab(tab); + SelectInSidePane(() => m_sidepane.SelectTab(tab)); // FWR-2895 Deleting a Custom list could result in needing to // update the PropertyTable. if (!String.IsNullOrEmpty(toolForArea) && !toolForArea.StartsWith("?")) { - var fsuccess = m_sidepane.SelectItem(tab, toolForArea); + var fsuccess = SelectInSidePane(() => m_sidepane.SelectItem(tab, toolForArea)); if (!fsuccess) { m_propertyTable.SetProperty(propToolForArea, null, true); @@ -605,7 +629,7 @@ private void SetToolForCurrentArea(string tool) // calling the SelectItem if it is already the CurrentItem. if(m_sidepane.CurrentItem != item) { - m_sidepane.SelectItem(tab, item.Name); + SelectInSidePane(() => m_sidepane.SelectItem(tab, item.Name)); } break; } diff --git a/Src/XCore/xCoreTests/SidebarAdapterSelectionTests.cs b/Src/XCore/xCoreTests/SidebarAdapterSelectionTests.cs new file mode 100644 index 0000000000..67f0a76075 --- /dev/null +++ b/Src/XCore/xCoreTests/SidebarAdapterSelectionTests.cs @@ -0,0 +1,189 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Windows.Forms; +using System.Xml; +using NUnit.Framework; +using SIL.SilSidePane; + +namespace XCore +{ + /// + /// How reflects areaChoice and ToolForAreaNamed_ changes in + /// the side pane without setting those properties again. + /// + [TestFixture] + [Apartment(ApartmentState.STA)] + public class SidebarAdapterSelectionTests + { + /// + /// Two areas with two tools each, in the shape of the sidebar section of Main.xml. + /// + private const string SidebarXml = + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + "" + + ""; + + private Form m_window; + private Mediator m_mediator; + private PropertyTable m_propertyTable; + private ImageCollection m_smallImages; + private ImageCollection m_largeImages; + private SidebarAdapter m_adapter; + private Control m_panel; + private ChoiceGroupCollection m_groups; + + [SetUp] + public void SetUp() + { + m_window = new Form(); + m_mediator = new Mediator(); + m_propertyTable = new PropertyTable(m_mediator); + m_smallImages = new ImageCollection(false); + m_largeImages = new ImageCollection(true); + m_adapter = new SidebarAdapter(); + m_panel = m_adapter.Init(m_window, m_smallImages, m_largeImages, m_mediator, m_propertyTable); + m_window.Controls.Add(m_panel); + + var doc = new XmlDocument(); + doc.LoadXml(SidebarXml); + m_groups = new ChoiceGroupCollection(m_mediator, m_propertyTable, m_adapter, + doc.DocumentElement.SelectSingleNode("sidebar")); + m_groups.Init(); + m_adapter.FinishInit(); + CreateHandles(m_window); + } + + [TearDown] + public void TearDown() + { + m_adapter.Dispose(); + m_window.Dispose(); + m_smallImages.Dispose(); + m_largeImages.Dispose(); + m_propertyTable.Dispose(); + m_mediator.Dispose(); + } + + /// + /// The side pane's list views raise their selection events only once they have a + /// window handle, which a hidden child of an unshown form never gets on its own. + /// + private static void CreateHandles(Control control) + { + var unused = control.Handle; + foreach (Control child in control.Controls) + CreateHandles(child); + } + + private static IEnumerable AllControls(Control root) + { + yield return root; + foreach (Control child in root.Controls) + foreach (var descendant in AllControls(child)) + yield return descendant; + } + + /// + /// Every side-pane item widget, across all the tabs' item lists. The form is never + /// shown, so a list's Visible flag cannot tell which tab is current. + /// + private IEnumerable AllItemWidgets() + { + return AllControls(m_panel).OfType().SelectMany(list => list.Items.Cast()); + } + + /// + /// The tool selected in the item list that holds the named tool, or null when none is. + /// Each tab's list keeps its own selection, so the list is named by one of its tools. + /// + private string SelectedToolInTheListHolding(string tool) + { + var list = AllItemWidgets().First(widget => ((Item)widget.Tag).Name == tool).ListView; + if (list.SelectedItems.Count == 0) + return null; + return ((ListPropertyChoice)((Item)list.SelectedItems[0].Tag).Tag).Value; + } + + private void ShowArea(string area, string rememberedTool) + { + m_propertyTable.SetProperty("areaChoice", area, false); + m_propertyTable.SetProperty("ToolForAreaNamed_" + area, rememberedTool, false); + m_adapter.OnPropertyChanged("areaChoice"); + } + + [Test] + public void AreaChoiceChange_SelectsTheRememberedToolWithoutSettingAnyProperty() + { + ShowArea("grammar", "posBrowse"); + + Assert.That(SelectedToolInTheListHolding("posBrowse"), Is.EqualTo("posBrowse")); + Assert.That(m_mediator.JobItems, Is.EqualTo(0), "the selection must not set a property"); + Assert.That(m_propertyTable.PropertyExists("currentContentControl"), Is.False); + Assert.That(m_propertyTable.PropertyExists("areaChoiceParameters"), Is.False); + } + + [Test] + public void ReturnToAnArea_SelectsTheToolRememberedSinceTheLastVisit() + { + ShowArea("lexicon", "lexiconEdit"); + ShowArea("grammar", "posEdit"); + + ShowArea("lexicon", "lexiconBrowse"); + + Assert.That(SelectedToolInTheListHolding("lexiconBrowse"), Is.EqualTo("lexiconBrowse")); + Assert.That(m_mediator.JobItems, Is.EqualTo(0), "the selection must not set a property"); + Assert.That(m_propertyTable.PropertyExists("currentContentControl"), Is.False); + } + + [Test] + public void ToolForAreaChange_SelectsThatToolWithoutSettingAnyProperty() + { + ShowArea("lexicon", "lexiconEdit"); + m_propertyTable.SetProperty("ToolForAreaNamed_lexicon", "lexiconBrowse", false); + + m_adapter.OnPropertyChanged("ToolForAreaNamed_lexicon"); + + Assert.That(SelectedToolInTheListHolding("lexiconBrowse"), Is.EqualTo("lexiconBrowse")); + Assert.That(m_mediator.JobItems, Is.EqualTo(0), "the selection must not set a property"); + Assert.That(m_propertyTable.PropertyExists("currentContentControl"), Is.False); + } + + [Test] + public void ItemSelectedOutsideTheAdapter_StillSetsTheProperty() + { + ShowArea("lexicon", "lexiconEdit"); + + AllItemWidgets().First(widget => ((Item)widget.Tag).Name == "lexiconBrowse").Selected = true; + + Assert.That(m_propertyTable.GetStringProperty("currentContentControl", null), Is.EqualTo("lexiconBrowse")); + Assert.That(m_mediator.JobItems, Is.GreaterThan(0), "a selection the adapter did not make is a click"); + } + } +}