From 9850096c3bed05f7f076a512bd1b64639e3a2e9c Mon Sep 17 00:00:00 2001 From: RKrom Date: Wed, 20 Feb 2013 09:45:34 +0000 Subject: [PATCH] Fixed an issue where the destination picker is displayed while we are already in the destination. This is done by storing the value of the clicked item in the ContextMenuStrip.Tag property, if this is null the user didn't click on the menu but somewhere else and we cancel the close... if the value is set, the user clicked on the menu and the close is allowed. git-svn-id: http://svn.code.sf.net/p/greenshot/code/trunk@2504 7dccd23d-a4a3-4e1f-8c07-b4c1b4018ab4 --- Greenshot/Forms/ImageEditorForm.cs | 2 +- GreenshotPlugin/Core/AbstractDestination.cs | 170 ++++++++++++-------- GreenshotPlugin/Interfaces/IDestination.cs | 3 +- 3 files changed, 107 insertions(+), 68 deletions(-) diff --git a/Greenshot/Forms/ImageEditorForm.cs b/Greenshot/Forms/ImageEditorForm.cs index 5cd7e9afc..c07a4f26c 100644 --- a/Greenshot/Forms/ImageEditorForm.cs +++ b/Greenshot/Forms/ImageEditorForm.cs @@ -299,7 +299,7 @@ namespace Greenshot { continue; } - ToolStripMenuItem item = destination.GetMenuItem(true, new EventHandler(DestinationToolStripMenuItemClick)); + ToolStripMenuItem item = destination.GetMenuItem(true, null, new EventHandler(DestinationToolStripMenuItemClick)); if (item != null) { item.ShortcutKeys = destination.EditorShortcutKeys; fileStripMenuItem.DropDownItems.Add(item); diff --git a/GreenshotPlugin/Core/AbstractDestination.cs b/GreenshotPlugin/Core/AbstractDestination.cs index ed53c7c08..dbaf5134d 100644 --- a/GreenshotPlugin/Core/AbstractDestination.cs +++ b/GreenshotPlugin/Core/AbstractDestination.cs @@ -71,68 +71,6 @@ namespace GreenshotPlugin.Core { } } - /// - /// Return a menu item - /// - /// - /// ToolStripMenuItem - public virtual ToolStripMenuItem GetMenuItem(bool addDynamics, EventHandler destinationClickHandler) { - ToolStripMenuItem basisMenuItem; - basisMenuItem = new ToolStripMenuItem(Description); - basisMenuItem.Image = DisplayIcon; - basisMenuItem.Tag = this; - basisMenuItem.Text = Description; - if (isDynamic && addDynamics) { - basisMenuItem.DropDownOpening += delegate (object source, EventArgs eventArgs) { - if (basisMenuItem.DropDownItems.Count == 0) { - List subDestinations = new List(); - // Fixing Bug #3536968 by catching the COMException (every exception) and not displaying the "subDestinations" - try { - subDestinations.AddRange(DynamicDestinations()); - } catch(Exception ex) { - LOG.ErrorFormat("Skipping {0}, due to the following error: {1}", Description, ex.Message); - } - if (subDestinations.Count > 0) { - subDestinations.Sort(); - - ToolStripMenuItem destinationMenuItem = null; - if (!useDynamicsOnly) { - destinationMenuItem = new ToolStripMenuItem(Description); - destinationMenuItem.Tag = this; - destinationMenuItem.Image = DisplayIcon; - destinationMenuItem.Click += destinationClickHandler; - basisMenuItem.DropDownItems.Add(destinationMenuItem); - } - if (useDynamicsOnly && subDestinations.Count == 1) { - basisMenuItem.Tag = subDestinations[0]; - basisMenuItem.Text = subDestinations[0].Description; - basisMenuItem.Click -= destinationClickHandler; - basisMenuItem.Click += destinationClickHandler; - } else { - foreach (IDestination subDestination in subDestinations) { - destinationMenuItem = new ToolStripMenuItem(subDestination.Description); - destinationMenuItem.Tag = subDestination; - destinationMenuItem.Image = subDestination.DisplayIcon; - destinationMenuItem.Click += destinationClickHandler; - basisMenuItem.DropDownItems.Add(destinationMenuItem); - } - } - } else { - // Setting base "click" only if there are no sub-destinations - - // Make sure any previous handler is removed, otherwise it would be added multiple times! - basisMenuItem.Click -= destinationClickHandler; - basisMenuItem.Click += destinationClickHandler; - } - } - }; - } else { - basisMenuItem.Click += destinationClickHandler; - } - - return basisMenuItem; - } - public virtual IEnumerable DynamicDestinations() { yield break; } @@ -201,6 +139,25 @@ namespace GreenshotPlugin.Core { return Description; } + /// + /// Helper method to add events which set the tag, this way we can see why there might be a close. + /// + /// Item to add the events to + /// Menu to set the tag + /// Value for the tag + private void AddTagEvents(ToolStripMenuItem menuItem, ContextMenuStrip menu, string tagValue) { + if (menuItem != null && menu != null) { + menuItem.MouseDown += delegate(object source, MouseEventArgs eventArgs) { + LOG.DebugFormat("Setting tag to '{0}'", tagValue); + menu.Tag = tagValue; + }; + menuItem.MouseUp += delegate(object source, MouseEventArgs eventArgs) { + LOG.Debug("Deleting tag"); + menu.Tag = null; + }; + } + } + /// /// This method will create and show the destination picker menu /// @@ -213,11 +170,22 @@ namespace GreenshotPlugin.Core { // Generate an empty ExportInformation object, for when nothing was selected. ExportInformation exportInformation = new ExportInformation(Designation, Language.GetString("settings_destination_picker")); ContextMenuStrip menu = new ContextMenuStrip(); + menu.Tag = null; + menu.Closing += delegate(object source, ToolStripDropDownClosingEventArgs eventArgs) { LOG.DebugFormat("Close reason: {0}", eventArgs.CloseReason); switch (eventArgs.CloseReason) { + case ToolStripDropDownCloseReason.AppFocusChange: + if (menu.Tag == null) { + // Do not allow the close if the tag is not set, this means the user clicked somewhere else. + eventArgs.Cancel = true; + } else { + LOG.DebugFormat("Letting the menu 'close' as the tag is set to '{0}'", menu.Tag); + } + break; case ToolStripDropDownCloseReason.ItemClicked: case ToolStripDropDownCloseReason.CloseCalled: + // The ContextMenuStrip can be "closed" for these reasons. break; case ToolStripDropDownCloseReason.Keyboard: // Dispose as the close is clicked @@ -231,7 +199,7 @@ namespace GreenshotPlugin.Core { }; foreach (IDestination destination in destinations) { // Fix foreach loop variable for the delegate - ToolStripMenuItem item = destination.GetMenuItem(addDynamics, + ToolStripMenuItem item = destination.GetMenuItem(addDynamics, menu, delegate(object sender, EventArgs e) { ToolStripMenuItem toolStripMenuItem = sender as ToolStripMenuItem; if (toolStripMenuItem == null) { @@ -242,9 +210,7 @@ namespace GreenshotPlugin.Core { return; } bool isEditor = "Editor".Equals(clickedDestination.Designation); - // Make sure the menu is invisible, don't close it - menu.Hide(); - + menu.Tag = clickedDestination.Designation; // Export exportInformation = clickedDestination.ExportCapture(true, surface, captureDetails); if (exportInformation != null && exportInformation.ExportMade) { @@ -259,6 +225,10 @@ namespace GreenshotPlugin.Core { } } else { LOG.Info("Export cancelled or failed, showing menu again"); + + // Make sure a click besides the menu don't close it. + menu.Tag = null; + // This prevents the problem that the context menu shows in the task-bar ShowMenuAtCursor(menu); } @@ -316,5 +286,73 @@ namespace GreenshotPlugin.Core { } } } + + /// + /// Return a menu item + /// + /// + /// ToolStripMenuItem + public virtual ToolStripMenuItem GetMenuItem(bool addDynamics, ContextMenuStrip menu, EventHandler destinationClickHandler) { + ToolStripMenuItem basisMenuItem; + basisMenuItem = new ToolStripMenuItem(Description); + basisMenuItem.Image = DisplayIcon; + basisMenuItem.Tag = this; + basisMenuItem.Text = Description; + AddTagEvents(basisMenuItem, menu, Description); + if (isDynamic && addDynamics) { + basisMenuItem.DropDownOpening += delegate(object source, EventArgs eventArgs) { + if (basisMenuItem.DropDownItems.Count == 0) { + List subDestinations = new List(); + // Fixing Bug #3536968 by catching the COMException (every exception) and not displaying the "subDestinations" + try { + subDestinations.AddRange(DynamicDestinations()); + } catch (Exception ex) { + LOG.ErrorFormat("Skipping {0}, due to the following error: {1}", Description, ex.Message); + } + if (subDestinations.Count > 0) { + subDestinations.Sort(); + + ToolStripMenuItem destinationMenuItem = null; + + if (!useDynamicsOnly) { + destinationMenuItem = new ToolStripMenuItem(Description); + destinationMenuItem.Tag = this; + destinationMenuItem.Image = DisplayIcon; + destinationMenuItem.Click += destinationClickHandler; + AddTagEvents(destinationMenuItem, menu, Description); + + basisMenuItem.DropDownItems.Add(destinationMenuItem); + } + if (useDynamicsOnly && subDestinations.Count == 1) { + basisMenuItem.Tag = subDestinations[0]; + basisMenuItem.Text = subDestinations[0].Description; + basisMenuItem.Click -= destinationClickHandler; + basisMenuItem.Click += destinationClickHandler; + } else { + foreach (IDestination subDestination in subDestinations) { + destinationMenuItem = new ToolStripMenuItem(subDestination.Description); + destinationMenuItem.Tag = subDestination; + destinationMenuItem.Image = subDestination.DisplayIcon; + destinationMenuItem.Click += destinationClickHandler; + AddTagEvents(destinationMenuItem, menu, subDestination.Description); + basisMenuItem.DropDownItems.Add(destinationMenuItem); + } + } + } else { + // Setting base "click" only if there are no sub-destinations + + // Make sure any previous handler is removed, otherwise it would be added multiple times! + basisMenuItem.Click -= destinationClickHandler; + basisMenuItem.Click += destinationClickHandler; + } + } + }; + } else { + basisMenuItem.Click += destinationClickHandler; + } + + return basisMenuItem; + } + } } diff --git a/GreenshotPlugin/Interfaces/IDestination.cs b/GreenshotPlugin/Interfaces/IDestination.cs index 27f149ab2..c9701e9c6 100644 --- a/GreenshotPlugin/Interfaces/IDestination.cs +++ b/GreenshotPlugin/Interfaces/IDestination.cs @@ -136,9 +136,10 @@ namespace Greenshot.Plugin { /// Return a menu item /// /// Resolve the dynamic destinations too? + /// The menu for which the item is created /// Handler which is called when clicked /// ToolStripMenuItem - ToolStripMenuItem GetMenuItem(bool addDynamics, EventHandler destinationClickHandler); + ToolStripMenuItem GetMenuItem(bool addDynamics, ContextMenuStrip menu, EventHandler destinationClickHandler); /// /// Gets the ShortcutKeys for the Editor