From 39c1cfba04f0682ae79e6629188eb291bc0d203e Mon Sep 17 00:00:00 2001 From: Jeremy Kuhne Date: Wed, 3 Nov 2021 12:48:52 -0700 Subject: [PATCH] Combine PropertyGrid tab information. (#6108) Combine tab related data into a record for clarity and safety. Use a List<> instead of an array to avoid complicated insert logic. Maintain tab selection as an instance instead of an index. Move error handling to CreateTab so thrown exceptions with messages (missing a bitmap / name for the tab) are actually thrown instead of a silent failure. --- .../PropertyGrid.PropertyTabCollection.cs | 15 +- .../Windows/Forms/PropertyGrid.TabInfo.cs | 17 + .../src/System/Windows/Forms/PropertyGrid.cs | 452 +++++++----------- .../src/System/Windows/Forms/ToolStripItem.cs | 10 +- 4 files changed, 219 insertions(+), 275 deletions(-) create mode 100644 src/System.Windows.Forms/src/System/Windows/Forms/PropertyGrid.TabInfo.cs diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGrid.PropertyTabCollection.cs b/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGrid.PropertyTabCollection.cs index 5c7cbcd7f36..f01319e701f 100644 --- a/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGrid.PropertyTabCollection.cs +++ b/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGrid.PropertyTabCollection.cs @@ -35,7 +35,7 @@ public int Count return 0; } - return _ownerPropertyGrid._viewTabs.Length; + return _ownerPropertyGrid._tabs.Count; } } @@ -55,7 +55,7 @@ public PropertyTab this[int index] throw new InvalidOperationException(SR.PropertyGridPropertyTabCollectionReadOnly); } - return _ownerPropertyGrid._viewTabs[index]; + return _ownerPropertyGrid._tabs[index].Tab; } } @@ -100,9 +100,14 @@ void ICollection.CopyTo(Array dest, int index) return; } - if (_ownerPropertyGrid._viewTabs.Length > 0) + if (_ownerPropertyGrid._tabs.Count > 0) { - Array.Copy(_ownerPropertyGrid._viewTabs, 0, dest, index, _ownerPropertyGrid._viewTabs.Length); + Array.Copy( + _ownerPropertyGrid._tabs.Select(i => i.Tab).ToArray(), + 0, + dest, + index, + _ownerPropertyGrid._tabs.Count); } } @@ -116,7 +121,7 @@ public IEnumerator GetEnumerator() return Array.Empty().GetEnumerator(); } - return _ownerPropertyGrid._viewTabs.GetEnumerator(); + return _ownerPropertyGrid._tabs.Select(i => i.Tab).GetEnumerator(); } public void RemoveTabType(Type propertyTabType) diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGrid.TabInfo.cs b/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGrid.TabInfo.cs new file mode 100644 index 00000000000..34a5081e852 --- /dev/null +++ b/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGrid.TabInfo.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.ComponentModel; +using System.Windows.Forms.Design; + +namespace System.Windows.Forms +{ + public partial class PropertyGrid + { + private record TabInfo(PropertyTab Tab, PropertyTabScope Scope, ToolStripButton Button) + { + public Type TabType => Tab.GetType(); + } + } +} diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGrid.cs b/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGrid.cs index a716331e0a8..c5265843914 100644 --- a/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGrid.cs +++ b/src/System.Windows.Forms/src/System/Windows/Forms/PropertyGrid.cs @@ -43,15 +43,13 @@ public partial class PropertyGrid : ContainerControl, IComPropertyBrowser, Ole32 private Bitmap _categoryBitmap; private Bitmap _propertyPageBitmap; - // Our array of view tabs - private bool _viewTabsDirty = true; + private readonly List _tabs = new(); + private TabInfo _selectedTab; + private bool _tabsDirty = true; + private bool _drawFlatToolBar; - private PropertyTab[] _viewTabs = Array.Empty(); - private PropertyTabScope[] _viewTabScopes = Array.Empty(); - private Dictionary _viewTabProperties; - private ToolStripButton[] _viewTabButtons; - private int _selectedViewTab; + private Dictionary _viewTabProperties; // Our view type buttons (Alpha vs. categorized) private ToolStripButton[] _viewSortButtons; @@ -1023,22 +1021,22 @@ public object[] SelectedObjects _rootEntry?.Dispose(); _rootEntry = null; - if (!classesSame && !GetFlag(Flags.TabsChanging) && _selectedViewTab < _viewTabButtons.Length) + if (!classesSame && !GetFlag(Flags.TabsChanging)) { // Throw away any extra component only tabs. - Type tabType = _selectedViewTab == -1 ? null : _viewTabs[_selectedViewTab].GetType(); + Type tabType = _selectedTab?.TabType; ToolStripButton viewTabButton = null; RefreshTabs(PropertyTabScope.Component); EnableTabs(); if (tabType is not null) { - for (int i = 0; i < _viewTabs.Length; i++) + foreach (var tab in _tabs) { - if (_viewTabs[i].GetType() == tabType && _viewTabButtons[i].Visible) + if (tab.TabType == tabType && tab.Button.Visible) { - viewTabButton = _viewTabButtons[i]; + viewTabButton = tab.Button; break; } } @@ -1048,10 +1046,10 @@ public object[] SelectedObjects } // Make sure we've also got events on all the objects. - if (showEvents && _viewTabs is not null && _viewTabs.Length > EventsTabIndex - && (_viewTabs[EventsTabIndex] is EventsTab eventsTab)) + if (showEvents && _tabs.Count > EventsTabIndex + && _tabs[EventsTabIndex] is { } eventTab && eventTab.Tab is EventsTab) { - showEvents = _viewTabButtons[EventsTabIndex].Visible; + showEvents = eventTab.Button.Visible; var attributes = new Attribute[BrowsableAttributes.Count]; BrowsableAttributes.CopyTo(attributes, 0); @@ -1080,7 +1078,7 @@ public object[] SelectedObjects && component.Site is not null // Use the original (possibly ICustomTypeDescriptor) to check whether there are // properties as this is what what we'll actually use to fill the events tab. - && (eventsTab.GetProperties(_selectedObjects[i], attributes)?.Count ?? 0) > 0; + && (eventTab.Tab.GetProperties(_selectedObjects[i], attributes)?.Count ?? 0) > 0; if (showEvents) { @@ -1110,9 +1108,9 @@ public object[] SelectedObjects // Get the active designer and see if we've stashed away state for it. if (TryGetSavedTabSelection(out int selectedTab)) { - if (selectedTab < _viewTabs.Length && (selectedTab == PropertiesTabIndex || _viewTabButtons[selectedTab].Visible)) + if (selectedTab < _tabs.Count && (selectedTab == PropertiesTabIndex || _tabs[selectedTab].Button.Visible)) { - SelectViewTabButton(_viewTabButtons[selectedTab], updateSelection: true); + SelectViewTabButton(_tabs[selectedTab].Button, updateSelection: true); } } else @@ -1141,8 +1139,8 @@ public PropertyTab SelectedTab { get { - Debug.Assert(_selectedViewTab < _viewTabs.Length && _selectedViewTab >= 0, "Invalid tab selection!"); - return _viewTabs[_selectedViewTab]; + Debug.Assert(_selectedTab is not null, "Invalid tab selection!"); + return _selectedTab?.Tab; } } @@ -1256,7 +1254,7 @@ public virtual bool ToolbarVisible OnLayoutInternal(dividerOnly: false); if (value) { - SetupToolbar(_viewTabsDirty); + SetupToolbar(fullRebuild: _tabsDirty); } Invalidate(); @@ -1329,7 +1327,8 @@ private int AddImage(Bitmap image) } // Resize bitmap only if resizing is needed in order to avoid image distortion. - if (DpiHelper.IsScalingRequired && (image.Size.Width != s_normalButtonSize.Width || image.Size.Height != s_normalButtonSize.Height)) + if (DpiHelper.IsScalingRequired + && (image.Size.Width != s_normalButtonSize.Width || image.Size.Height != s_normalButtonSize.Height)) { image = DpiHelper.CreateResizedBitmap(image, s_normalButtonSize); } @@ -1469,29 +1468,22 @@ public event EventHandler SelectedObjectsChanged remove => Events.RemoveHandler(s_selectedObjectsChangedEvent, value); } - internal void AddTab(Type tabType, PropertyTabScope type, object @object = null, bool setupToolbar = true) + internal void AddTab(Type tabType, PropertyTabScope scope, object @object = null, bool setupToolbar = true) { PropertyTab tab = null; int tabIndex = -1; - if (_viewTabs is not null) + // Check to see if we've already got a tab of this type. + for (int i = 0; i < _tabs.Count; i++) { - // Check to see if we've already got a tab of this type. - for (int i = 0; i < _viewTabs.Length; i++) + Debug.Assert(_tabs[i] is not null, "Null item in tab array!"); + if (tabType == _tabs[i].Tab.GetType()) { - Debug.Assert(_viewTabs[i] is not null, "Null item in tab array!"); - if (tabType == _viewTabs[i].GetType()) - { - tab = _viewTabs[i]; - tabIndex = i; - break; - } + tab = _tabs[i].Tab; + tabIndex = i; + break; } } - else - { - tabIndex = 0; - } if (tab is null) { @@ -1503,65 +1495,54 @@ internal void AddTab(Type tabType, PropertyTabScope type, object @object = null, host = site.GetService(); } - try - { - tab = CreateTab(tabType, host); - } - catch (Exception) + tab = CreateTab(tabType, host); + + if (tab is null) { return; } - // Add it at the end of the array. - if (_viewTabs is not null) - { - tabIndex = _viewTabs.Length; + // Default to inserting at the end. + tabIndex = _tabs.Count; - // Find the insertion position. Special case for event's and properties. - if (tabType == DefaultTabType) - { - tabIndex = PropertiesTabIndex; - } - else if (typeof(EventsTab).IsAssignableFrom(tabType)) - { - tabIndex = EventsTabIndex; - } - else + // Find the insertion position. Special case for event's and properties. + if (tabType == DefaultTabType) + { + tabIndex = PropertiesTabIndex; + } + else if (typeof(EventsTab).IsAssignableFrom(tabType)) + { + tabIndex = EventsTabIndex; + } + else + { + // Order tabs alphabetically, we've always got a property tab, so start after that. + for (int i = 1; i < _tabs.Count; i++) { - // Order tabs alphabetically, we've always got a property tab, so start after that. - for (int i = 1; i < _viewTabs.Length; i++) + var current = _tabs[i].Tab; + + // Skip the event tab. + if (current is EventsTab) { - // Skip the event tab. - if (_viewTabs[i] is EventsTab) - { - continue; - } + continue; + } - if (string.Compare(tab.TabName, _viewTabs[i].TabName, ignoreCase: false, CultureInfo.InvariantCulture) < 0) - { - tabIndex = i; - break; - } + if (string.Compare(tab.TabName, current.TabName, ignoreCase: false, CultureInfo.InvariantCulture) < 0) + { + tabIndex = i; + break; } } } - // Now add the tab to the tabs array. - var newTabs = new PropertyTab[_viewTabs.Length + 1]; - Array.Copy(_viewTabs, 0, newTabs, 0, tabIndex); - Array.Copy(_viewTabs, tabIndex, newTabs, tabIndex + 1, _viewTabs.Length - tabIndex); - newTabs[tabIndex] = tab; - _viewTabs = newTabs; - - _viewTabsDirty = true; - - var newTabScopes = new PropertyTabScope[_viewTabScopes.Length + 1]; - Array.Copy(_viewTabScopes, 0, newTabScopes, 0, tabIndex); - Array.Copy(_viewTabScopes, tabIndex, newTabScopes, tabIndex + 1, _viewTabScopes.Length - tabIndex); - newTabScopes[tabIndex] = type; - _viewTabScopes = newTabScopes; + ToolStripButton button = CreatePushButton( + tab.TabName, + imageIndex: -1, + OnViewTabButtonClick, + useRadioButtonRole: true); - Debug.Assert(_viewTabs is not null, "Tab array destroyed!"); + _tabs.Insert(tabIndex, new(tab, scope, button)); + _tabsDirty = true; } if (tab is not null && @object is not null) @@ -1666,7 +1647,17 @@ private ToolStripSeparator CreateSeparatorButton() private PropertyTab CreateTab(Type tabType, IDesignerHost host) { - PropertyTab tab = CreatePropertyTab(tabType); + PropertyTab tab; + + try + { + tab = CreatePropertyTab(tabType); + } + catch (Exception exception) + { + Debug.Fail($"{nameof(CreatePropertyTab)} failed. {exception.Message}"); + return null; + } if (tab is null) { @@ -1687,19 +1678,25 @@ private PropertyTab CreateTab(Type tabType, IDesignerHost host) parameter = Site; } - if (parameter is not null && constructor is not null) + try { - tab = (PropertyTab)constructor.Invoke(new object[] { parameter }); + if (parameter is not null && constructor is not null) + { + tab = (PropertyTab)constructor.Invoke(new object[] { parameter }); + } + else + { + // Just call the default constructor. + tab = (PropertyTab)Activator.CreateInstance(tabType); + } } - else + catch (Exception exception) { - // Just call the default constructor. - tab = (PropertyTab)Activator.CreateInstance(tabType); + Debug.Fail($"Failed to create {nameof(PropertyTab)}. {exception.Message}"); + tab = null; } } - Debug.Assert(tab is not null, "Failed to create tab!"); - if (tab is not null) { if (tab.Bitmap is null) @@ -1716,7 +1713,11 @@ private PropertyTab CreateTab(Type tabType, IDesignerHost host) return tab; } - private ToolStripButton CreatePushButton(string toolTipText, int imageIndex, EventHandler eventHandler, bool useRadioButtonRole = false) + private ToolStripButton CreatePushButton( + string toolTipText, + int imageIndex, + EventHandler eventHandler, + bool useRadioButtonRole = false) { PropertyGridToolStripButton button = new(this, useRadioButtonRole) { @@ -1825,16 +1826,13 @@ protected override void Dispose(bool disposing) ActiveDesigner = null; - if (_viewTabs is not null) + foreach (var tabInfo in _tabs) { - for (int i = 0; i < _viewTabs.Length; i++) - { - _viewTabs[i].Dispose(); - } - - _viewTabs = null; + tabInfo.Tab.Dispose(); } + _tabs.Clear(); + _normalButtonImages?.Dispose(); _normalButtonImages = null; @@ -2007,17 +2005,10 @@ private void EnableTabs() // Make sure our toolbars are okay. SetupToolbar(); - Debug.Assert(_viewTabs is not null, "Invalid tab array"); - Debug.Assert( - _viewTabs.Length == _viewTabScopes.Length && _viewTabScopes.Length == _viewTabButtons.Length, - $"Uh oh, tab arrays aren't all the same length! tabs={_viewTabs.Length}, scopes={_viewTabScopes.Length}, buttons={_viewTabButtons.Length}"); - // Walk through the current tabs to validate that they apply to all currently selected objects. // Skip the property tab since it's always valid. - for (int i = 1; i < _viewTabs.Length; i++) + foreach (var tab in _tabs) { - Debug.Assert(_viewTabs[i] is not null, "Invalid tab array entry"); - bool canExtend = true; // Make sure the tab is valid for all objects. @@ -2025,7 +2016,7 @@ private void EnableTabs() { try { - if (!_viewTabs[i].CanExtend(GetUnwrappedObject(j))) + if (!tab.Tab.CanExtend(GetUnwrappedObject(j))) { canExtend = false; break; @@ -2039,12 +2030,12 @@ private void EnableTabs() } } - if (canExtend != _viewTabButtons[i].Visible) + if (canExtend != tab.Button.Visible) { - _viewTabButtons[i].Visible = canExtend; - if (!canExtend && i == _selectedViewTab) + tab.Button.Visible = canExtend; + if (!canExtend && tab == _selectedTab) { - SelectViewTabButton(_viewTabButtons[PropertiesTabIndex], updateSelection: true); + SelectViewTabButton(_tabs[PropertiesTabIndex].Button, updateSelection: true); } } } @@ -2067,9 +2058,9 @@ private void EnsureLargeButtons() AddLargeImage(_alphaBitmap); AddLargeImage(_categoryBitmap); - foreach (PropertyTab tab in _viewTabs) + foreach (var tab in _tabs) { - AddLargeImage(tab.Bitmap); + AddLargeImage(tab.Tab.Bitmap); } AddLargeImage(_propertyPageBitmap); @@ -2571,7 +2562,7 @@ private void OnLayoutInternal(bool dividerOnly) if (_oldDeviceDpi != _deviceDpi) { RescaleConstants(); - SetupToolbar(true); + SetupToolbar(fullRebuild: true); } // No toolbar or help or commands visible, just fill the whole thing with the grid. @@ -3362,9 +3353,9 @@ private void Refresh(bool clearCached) internal void RefreshProperties(bool clearCached) { // Clear our current cache so we can do a full refresh. - if (clearCached && _selectedViewTab != -1 && _viewTabs is not null) + if (clearCached && _selectedTab is not null) { - PropertyTab selectedTab = _viewTabs[_selectedViewTab]; + PropertyTab selectedTab = _selectedTab.Tab; if (selectedTab is not null && _viewTabProperties is not null) { _viewTabProperties.Remove($"{selectedTab.TabName}{_propertySortValue}"); @@ -3435,11 +3426,11 @@ internal void ReleaseTab(Type tabType, object component) { PropertyTab tab = null; int tabIndex = -1; - for (int i = 0; i < _viewTabs.Length; i++) + for (int i = 0; i < _tabs.Count; i++) { - if (tabType == _viewTabs[i].GetType()) + if (tabType == _tabs[i].Tab.GetType()) { - tab = _viewTabs[i]; + tab = _tabs[i].Tab; tabIndex = i; break; } @@ -3479,7 +3470,7 @@ internal void ReleaseTab(Type tabType, object component) } // We don't remove PropertyTabScope.Global tabs here. Our owner has to do that. - if (killTab && _viewTabScopes[tabIndex] > PropertyTabScope.Global) + if (killTab && _tabs[tabIndex].Scope > PropertyTabScope.Global) { RemoveTab(tabIndex, false); } @@ -3498,77 +3489,53 @@ internal void RemoveTabs(PropertyTabScope classification, bool setupToolbar) } // In case we've been disposed. - if (_viewTabButtons is null || _viewTabs is null || _viewTabScopes is null) + if (_tabs.Count == 0) { return; } - ToolStripButton selectedButton = _selectedViewTab >= 0 && _selectedViewTab < _viewTabButtons.Length - ? _viewTabButtons[_selectedViewTab] - : null; + ToolStripButton selectedButton = _selectedTab?.Button; - for (int i = _viewTabs.Length - 1; i >= 0; i--) + if (_tabs.RemoveAll(i => i.Scope >= classification) > 0) { - if (_viewTabScopes[i] >= classification) - { - // Adjust the selected view tab because we're deleting. - if (_selectedViewTab == i) - { - _selectedViewTab = -1; - } - else if (_selectedViewTab > i) - { - _selectedViewTab--; - } - - var newTabs = new PropertyTab[_viewTabs.Length - 1]; - Array.Copy(_viewTabs, 0, newTabs, 0, i); - Array.Copy(_viewTabs, i + 1, newTabs, i, _viewTabs.Length - i - 1); - _viewTabs = newTabs; - - var newTabScopes = new PropertyTabScope[_viewTabScopes.Length - 1]; - Array.Copy(_viewTabScopes, 0, newTabScopes, 0, i); - Array.Copy(_viewTabScopes, i + 1, newTabScopes, i, _viewTabScopes.Length - i - 1); - _viewTabScopes = newTabScopes; - - _viewTabsDirty = true; - } + _tabsDirty = true; + _selectedTab = _tabs.FirstOrDefault(); } - if (setupToolbar && _viewTabsDirty) + if (setupToolbar && _tabsDirty) { SetupToolbar(); - Debug.Assert(_viewTabs is not null && _viewTabs.Length > 0, "We don't have any tabs left!"); + Debug.Assert(_tabs.Count > 0, "We don't have any tabs left!"); - _selectedViewTab = -1; + _selectedTab = null; SelectViewTabButtonDefault(selectedButton); // Clear the component refs of the tabs. - for (int i = 0; i < _viewTabs.Length; i++) + foreach (TabInfo info in _tabs) { - _viewTabs[i].Components = Array.Empty(); + info.Tab.Components = Array.Empty(); } } } internal void RemoveTab(int tabIndex, bool setupToolbar) { - Debug.Assert(_viewTabs is not null, "Tab array destroyed!"); + Debug.Assert(_tabs.Count > 0, "Tab array destroyed!"); - if (tabIndex >= _viewTabs.Length || tabIndex < 0) + if (tabIndex >= _tabs.Count || tabIndex < 0) { throw new ArgumentOutOfRangeException(nameof(tabIndex), SR.PropertyGridBadTabIndex); } - if (_viewTabScopes[tabIndex] == PropertyTabScope.Static) + if (_tabs[tabIndex].Scope == PropertyTabScope.Static) { throw new ArgumentException(SR.PropertyGridRemoveStaticTabs); } - if (_selectedViewTab == tabIndex) + if (_selectedTab == _tabs[tabIndex]) { - _selectedViewTab = PropertiesTabIndex; + _selectedTab = _tabs[PropertiesTabIndex]; } // Remove this tab from our "last selected" group @@ -3577,24 +3544,15 @@ internal void RemoveTab(int tabIndex, bool setupToolbar) _designerSelections.Remove(ActiveDesigner.GetHashCode()); } - ToolStripButton selectedButton = _viewTabButtons[_selectedViewTab]; - - var newTabs = new PropertyTab[_viewTabs.Length - 1]; - Array.Copy(_viewTabs, 0, newTabs, 0, tabIndex); - Array.Copy(_viewTabs, tabIndex + 1, newTabs, tabIndex, _viewTabs.Length - tabIndex - 1); - _viewTabs = newTabs; - - var newTabScopes = new PropertyTabScope[_viewTabScopes.Length - 1]; - Array.Copy(_viewTabScopes, 0, newTabScopes, 0, tabIndex); - Array.Copy(_viewTabScopes, tabIndex + 1, newTabScopes, tabIndex, _viewTabScopes.Length - tabIndex - 1); - _viewTabScopes = newTabScopes; + ToolStripButton selectedButton = _selectedTab.Button; - _viewTabsDirty = true; + _tabs.RemoveAt(tabIndex); + _tabsDirty = true; if (setupToolbar) { SetupToolbar(); - _selectedViewTab = -1; + _selectedTab = null; SelectViewTabButtonDefault(selectedButton); } } @@ -3602,9 +3560,9 @@ internal void RemoveTab(int tabIndex, bool setupToolbar) internal void RemoveTab(Type tabType) { int tabIndex = -1; - for (int i = 0; i < _viewTabs.Length; i++) + for (int i = 0; i < _tabs.Count; i++) { - if (tabType == _viewTabs[i].GetType()) + if (tabType == _tabs[i].GetType()) { tabIndex = i; break; @@ -3617,17 +3575,8 @@ internal void RemoveTab(Type tabType) return; } - var newTabs = new PropertyTab[_viewTabs.Length - 1]; - Array.Copy(_viewTabs, 0, newTabs, 0, tabIndex); - Array.Copy(_viewTabs, tabIndex + 1, newTabs, tabIndex, _viewTabs.Length - tabIndex - 1); - _viewTabs = newTabs; - - var newTabScopes = new PropertyTabScope[_viewTabScopes.Length - 1]; - Array.Copy(_viewTabScopes, 0, newTabScopes, 0, tabIndex); - Array.Copy(_viewTabScopes, tabIndex + 1, newTabScopes, tabIndex, _viewTabScopes.Length - tabIndex - 1); - _viewTabScopes = newTabScopes; - - _viewTabsDirty = true; + _tabs.RemoveAt(tabIndex); + _tabsDirty = true; SetupToolbar(); } @@ -3674,7 +3623,7 @@ private void SaveTabSelection() if (_designerHost is not null) { _designerSelections ??= new(); - _designerSelections[_designerHost.GetHashCode()] = _selectedViewTab; + _designerSelections[_designerHost.GetHashCode()] = _selectedTab; } } @@ -3703,7 +3652,7 @@ private void SetHotCommandColors() private void SelectViewTabButton(ToolStripButton button, bool updateSelection) { - Debug.Assert(_viewTabButtons is not null, "No view tab buttons to select!"); + Debug.Assert(_tabs.Count > 0, "No view tab buttons to select!"); if (!SelectViewTabButtonDefault(button)) { @@ -3718,40 +3667,34 @@ private void SelectViewTabButton(ToolStripButton button, bool updateSelection) private bool SelectViewTabButtonDefault(ToolStripButton button) { - // Make sure our selection number is valid. - if (_selectedViewTab >= 0 && _selectedViewTab >= _viewTabButtons.Length) - { - _selectedViewTab = -1; - } - // Is this tab button checked? If so, do nothing. - if (_selectedViewTab >= 0 && _selectedViewTab < _viewTabButtons.Length && - button == _viewTabButtons[_selectedViewTab]) + if (button == _selectedTab?.Button) { - _viewTabButtons[_selectedViewTab].Checked = true; + button.Checked = true; return true; } PropertyTab oldTab = null; // Unselect what's selected. - if (_selectedViewTab != -1) + if (_selectedTab is not null) { - _viewTabButtons[_selectedViewTab].Checked = false; - oldTab = _viewTabs[_selectedViewTab]; + _selectedTab.Button.Checked = false; + oldTab = _selectedTab.Tab; } // Get the new index of the button. - for (int i = 0; i < _viewTabButtons.Length; i++) + foreach (TabInfo info in _tabs) { - if (_viewTabButtons[i] == button) + if (info.Button == button) { - _selectedViewTab = i; - _viewTabButtons[i].Checked = true; + _selectedTab = info; + info.Button.Checked = true; + try { SetFlag(Flags.TabsChanging, true); - OnPropertyTabChanged(new PropertyTabChangedEventArgs(oldTab, _viewTabs[i])); + OnPropertyTabChanged(new(oldTab, info.Tab)); } finally { @@ -3763,21 +3706,21 @@ private bool SelectViewTabButtonDefault(ToolStripButton button) } // Select the first tab if we didn't find that one. - _selectedViewTab = PropertiesTabIndex; - Debug.Assert(_viewTabs[PropertiesTabIndex].GetType() == DefaultTabType, "First item is not property tab!"); - SelectViewTabButton(_viewTabButtons[PropertiesTabIndex], false); + _selectedTab = _tabs[PropertiesTabIndex]; + Debug.Assert(_tabs[PropertiesTabIndex].Tab.GetType() == DefaultTabType, "First item is not property tab!"); + SelectViewTabButton(_tabs[PropertiesTabIndex].Button, updateSelection: false); return false; } private void SetSelectState(int state) { - if (state >= (_viewTabs.Length * _viewSortButtons.Length)) + if (state >= (_tabs.Count * _viewSortButtons.Length)) { state = 0; } else if (state < 0) { - state = (_viewTabs.Length * _viewSortButtons.Length) - 1; + state = (_tabs.Count * _viewSortButtons.Length) - 1; } // NOTE: See GetSelectState for the full description of the state transitions @@ -3797,7 +3740,7 @@ private void SetSelectState(int state) Debug.Assert(view < _viewSortButtons.Length, "Can't select view type > 1"); - OnViewTabButtonClick(_viewTabButtons[tab], EventArgs.Empty); + OnViewTabButtonClick(_tabs[tab].Button, EventArgs.Empty); OnViewSortButtonClick(_viewSortButtons[view], EventArgs.Empty); } } @@ -3825,7 +3768,7 @@ private void SetToolStripRenderer() private void SetupToolbar(bool fullRebuild) { // If the tab array hasn't changed, don't bother to do all this work. - if (!_viewTabsDirty && !fullRebuild) + if (!_tabsDirty && !fullRebuild) { return; } @@ -3847,7 +3790,6 @@ private void SetupToolbar(bool fullRebuild) EventHandler sortButtonHandler = OnViewSortButtonClick; EventHandler propertyPagesButtonHandler = OnViewPropertyPagesButtonClick; - Bitmap b; int i; // We manage the buttons as a separate list so the toolbar doesn't flash. @@ -3923,38 +3865,23 @@ private void SetupToolbar(bool fullRebuild) buttonList.Add(_separator1); - // Here's our buttons array. - _viewTabButtons = new ToolStripButton[_viewTabs.Length]; - // If we've only got the properties tab, don't add the button // (or we'll just have a properties button that you can't do anything with). - bool addViewTabButtons = _viewTabs.Length > 1; - - // Setup the view tab buttons. - for (i = 0; i < _viewTabs.Length; i++) + if (_tabs.Count > 1) { - try + foreach (TabInfo info in _tabs) { - b = _viewTabs[i].Bitmap; - _viewTabButtons[i] = CreatePushButton( - _viewTabs[i].TabName, - AddImage(b), - tabButtonHandler, - useRadioButtonRole: true); - if (addViewTabButtons) + try { - buttonList.Add(_viewTabButtons[i]); + info.Button.ImageIndex = AddImage(info.Tab.Bitmap); + buttonList.Add(info.Button); + } + catch (Exception ex) + { + Debug.Fail(ex.ToString()); } } - catch (Exception ex) - { - Debug.Fail(ex.ToString()); - } - } - // If we didn't add anything, we don't need another separator either. - if (addViewTabButtons) - { buttonList.Add(_separator2); } @@ -3999,27 +3926,23 @@ private void SetupToolbar(bool fullRebuild) _toolStrip.ResumeLayout(); - if (_viewTabsDirty) + if (_tabsDirty) { // If we're redoing our tabs make sure we setup the toolbar area correctly. OnLayoutInternal(dividerOnly: false); } - _viewTabsDirty = false; + _tabsDirty = false; } protected void ShowEventsButton(bool value) { - if (_viewTabs is not null && _viewTabs.Length > EventsTabIndex && (_viewTabs[EventsTabIndex] is EventsTab)) + if (_tabs.Count > EventsTabIndex && _tabs[EventsTabIndex] is { } info && info.Tab is EventsTab) { - Debug.Assert( - _viewTabButtons is not null && _viewTabButtons.Length > EventsTabIndex && _viewTabButtons[EventsTabIndex] is not null, - "Events button is not at EVENTS position"); - - _viewTabButtons[EventsTabIndex].Visible = value; - if (!value && _selectedViewTab == EventsTabIndex) + info.Button.Visible = value; + if (!value && _selectedTab == info) { - SelectViewTabButton(_viewTabButtons[PropertiesTabIndex], true); + SelectViewTabButton(_tabs[PropertiesTabIndex].Button, updateSelection: true); } } @@ -4110,7 +4033,7 @@ private void SinkPropertyNotifyEvents() } } - private bool ShouldForwardChildMouseMessage(Control child, MouseEventArgs e, ref Point pt) + private bool ShouldForwardChildMouseMessage(Control child, MouseEventArgs e, ref Point point) { Size size = child.Size; @@ -4122,7 +4045,7 @@ private bool ShouldForwardChildMouseMessage(Control child, MouseEventArgs e, ref temp = WindowsFormsUtils.TranslatePoint(temp, child, this); // Forward the message. - pt = temp; + point = temp; return true; } @@ -4133,7 +4056,7 @@ private void UpdatePropertiesViewTabVisibility() { // If the only view available is properties-view, there's no need to show the button. - if (_viewTabButtons is null) + if (_tabs.Count <= 1) { return; } @@ -4141,35 +4064,27 @@ private void UpdatePropertiesViewTabVisibility() bool shouldBeVisible = false; // Starts at index 1, since index 0 is properties-view - for (int i = 1; i < _viewTabButtons.Length; i++) + for (int i = 1; i < _tabs.Count; i++) { - if (_viewTabButtons[i].Visible) + if (_tabs[i].Button.Visible) { shouldBeVisible = true; break; } } - if (shouldBeVisible) - { - _viewTabButtons[PropertiesTabIndex].Visible = true; - _separator2.Visible = true; - } - else - { - _viewTabButtons[PropertiesTabIndex].Visible = false; - _separator2.Visible = false; - } + _tabs[PropertiesTabIndex].Button.Visible = shouldBeVisible; + _separator2.Visible = shouldBeVisible; } internal void UpdateSelection() { - if (!GetFlag(Flags.PropertiesChanged) || _viewTabs is null) + if (!GetFlag(Flags.PropertiesChanged) || _tabs.Count == 0) { return; } - string tabName = $"{_viewTabs[_selectedViewTab].TabName}{_propertySortValue}"; + string tabName = $"{_selectedTab.Tab.TabName}{_propertySortValue}"; if (_viewTabProperties is not null && _viewTabProperties.ContainsKey(tabName)) { @@ -4455,11 +4370,14 @@ protected unsafe override void WndProc(ref Message m) { string tabTypeName = AutomationMessages.ReadAutomationText(m.LParamInternal); - for (int i = 0; i < _viewTabs.Length; i++) + foreach (TabInfo info in _tabs) { - if (_viewTabs[i].GetType().FullName == tabTypeName && _viewTabButtons[i].Visible) + if (info.Tab.GetType().FullName == tabTypeName && info.Button.Visible) { - SelectViewTabButtonDefault(_viewTabButtons[i]); + SelectViewTabButtonDefault(info.Button); + + // This gets set again to 0 below. This seems wrong, but has always been this way. + // Leaving this should we find we need to return instead of break. m.ResultInternal = 1; break; } diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/ToolStripItem.cs b/src/System.Windows.Forms/src/System/Windows/Forms/ToolStripItem.cs index e8ea6ab6110..74d9e5b106e 100644 --- a/src/System.Windows.Forms/src/System/Windows/Forms/ToolStripItem.cs +++ b/src/System.Windows.Forms/src/System/Windows/Forms/ToolStripItem.cs @@ -1083,14 +1083,15 @@ public Color ImageTransparentColor [SRCategory(nameof(SR.CatBehavior))] [RefreshProperties(RefreshProperties.Repaint)] [TypeConverter(typeof(NoneExcludedImageIndexConverter))] - [Editor("System.Windows.Forms.Design.ImageIndexEditor, " + AssemblyRef.SystemDesign, typeof(UITypeEditor))] + [Editor($"System.Windows.Forms.Design.ImageIndexEditor, {AssemblyRef.SystemDesign}", typeof(UITypeEditor))] [Browsable(false)] [RelatedImageList("Owner.ImageList")] public int ImageIndex { get { - if ((Owner is not null) && ImageIndexer.Index != ImageList.Indexer.DefaultIndex && Owner.ImageList is not null && ImageIndexer.Index >= Owner.ImageList.Images.Count) + if ((Owner is not null) && ImageIndexer.Index != ImageList.Indexer.DefaultIndex + && Owner.ImageList is not null && ImageIndexer.Index >= Owner.ImageList.Images.Count) { return Owner.ImageList.Images.Count - 1; } @@ -1101,11 +1102,14 @@ public int ImageIndex { if (value < ImageList.Indexer.DefaultIndex) { - throw new ArgumentOutOfRangeException(nameof(value), string.Format(SR.InvalidLowBoundArgumentEx, nameof(ImageIndex), value, ImageList.Indexer.DefaultIndex)); + throw new ArgumentOutOfRangeException( + nameof(value), + string.Format(SR.InvalidLowBoundArgumentEx, nameof(ImageIndex), value, ImageList.Indexer.DefaultIndex)); } ImageIndexer.Index = value; _state[s_stateInvalidMirroredImage] = true; + // Set the Image Property to null Properties.SetObject(s_imageProperty, null);