Creating a MenuItem with a Shortcut of Control+Plus – is using reflection to modify MenuItem's private fields the best method?

Question

I'm using the legacy MainMenu control (with MenuItems) control in an application, and would like to implement zoom in and zoom out menu items (with Control++ and Control+- keyboard shortcuts). (Note that I'm using MainMenu and not MenuStrip). MenuItem does have a Shortcut property, of type Shortcut, but that doesn't have a CtrlPlus option.

I decided to look at how Shortcut was implemented in the referencesource, and it looks like the way that each of those enum values is just a combination of several Keys enum values (such as CtrlA being just Keys.Control + Keys.A). So I tried creating a custom Shortcut value that should be equal to Control+Plus:

const Shortcut CONTROL_PLUS = (Shortcut)(Keys.Control | Keys.Oemplus);

zoomInMenuItem.Shortcut = CONTROL_PLUS;

However, this throws an InvalidEnumArgumentException when I try to assign the Shortcut property.

So I've decided to use reflection, and modify the (not public) MenuItemData's shortcut property and then call the (non-public) UpdateMenuItem method. This actually works (with a side effect of displaying as Control+Oemplus in the menu item):

const Shortcut CONTROL_PLUS = (Shortcut)(Keys.Control | Keys.Oemplus);

var dataField = typeof(MenuItem).GetField("data", BindingFlags.NonPublic | BindingFlags.Instance);
var updateMenuItemMethod = typeof(MenuItem).GetMethod("UpdateMenuItem", BindingFlags.NonPublic | BindingFlags.Instance);
var menuItemDataShortcutField = typeof(MenuItem).GetNestedType("MenuItemData", BindingFlags.NonPublic)
    .GetField("shortcut", BindingFlags.NonPublic | BindingFlags.Instance);

var zoomInData = dataField.GetValue(zoomInMenuItem);
menuItemDataShortcutField.SetValue(zoomInData, CONTROL_PLUS);
updateMenuItemMethod.Invoke(zoomInMenuItem, new object[] { true });

While that method works, it uses reflection, and I'm not sure if it's future-proof.

I'm using MenuItem and not the newer ToolStripMenuItem because I need to have the RadioCheck property (among other reasons); switching away from that is not an option.

Here's some full code that creates the above dialog, which shows what I'm trying to accomplish (the most relevant code is in the OnLoad method):

ZoomForm.cs

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.Reflection;

namespace ZoomMenuItemMCVE
{
    public partial class ZoomForm : Form
    {
        private double zoom = 1.0;
        public double Zoom {
            get { return zoom; }
            set {
                zoom = value;
                zoomTextBox.Text = "Zoom: " + zoom;
            }
        }

        public ZoomForm() {
            InitializeComponent();
        }

        protected override void OnLoad(EventArgs e) {
            const Shortcut CONTROL_PLUS = (Shortcut)((int)Keys.Control + (int)Keys.Oemplus);
            const Shortcut CONTROL_MINUS = (Shortcut)((int)Keys.Control + (int)Keys.OemMinus);

            base.OnLoad(e);

            //We set menu later as otherwise the designer goes insane (http://stackoverflow.com/q/28461091/3991344)
            this.Menu = mainMenu;

            var dataField = typeof(MenuItem).GetField("data", BindingFlags.NonPublic | BindingFlags.Instance);
            var updateMenuItemMethod = typeof(MenuItem).GetMethod("UpdateMenuItem", BindingFlags.NonPublic | BindingFlags.Instance);
            var menuItemDataShortcutField = typeof(MenuItem).GetNestedType("MenuItemData", BindingFlags.NonPublic)
                .GetField("shortcut", BindingFlags.NonPublic | BindingFlags.Instance);

            var zoomInData = dataField.GetValue(zoomInMenuItem);
            menuItemDataShortcutField.SetValue(zoomInData, CONTROL_PLUS);
            updateMenuItemMethod.Invoke(zoomInMenuItem, new object[] { true });

            var zoomOutData = dataField.GetValue(zoomOutMenuItem);
            menuItemDataShortcutField.SetValue(zoomOutData, CONTROL_MINUS);
            updateMenuItemMethod.Invoke(zoomOutMenuItem, new object[] { true });
        }

        private void zoomInMenuItem_Click(object sender, EventArgs e) {
            Zoom *= 2;
        }

        private void zoomOutMenuItem_Click(object sender, EventArgs e) {
            Zoom /= 2;
        }
    }
}

ZoomForm.Designer.cs

namespace ZoomMenuItemMCVE
{
    partial class ZoomForm
    {
        /// <summary>
        /// Required designer variable.
        /// </summary>
        private System.ComponentModel.IContainer components = null;

        /// <summary>
        /// Clean up any resources being used.
        /// </summary>
        /// <param name="disposing">true if managed resources should be disposed; otherwise, false.</param>
        protected override void Dispose(bool disposing) {
            if (disposing && (components != null)) {
                components.Dispose();
            }
            base.Dispose(disposing);
        }

        #region Windows Form Designer generated code

        /// <summary>
        /// Required method for Designer support - do not modify
        /// the contents of this method with the code editor.
        /// </summary>
        private void InitializeComponent() {
            this.components = new System.ComponentModel.Container();
            System.Windows.Forms.MenuItem viewMenuItem;
            this.zoomTextBox = new System.Windows.Forms.TextBox();
            this.mainMenu = new System.Windows.Forms.MainMenu(this.components);
            this.zoomInMenuItem = new System.Windows.Forms.MenuItem();
            this.zoomOutMenuItem = new System.Windows.Forms.MenuItem();
            viewMenuItem = new System.Windows.Forms.MenuItem();
            this.SuspendLayout();
            // 
            // zoomTextBox
            // 
            this.zoomTextBox.Dock = System.Windows.Forms.DockStyle.Bottom;
            this.zoomTextBox.Location = new System.Drawing.Point(0, 81);
            this.zoomTextBox.Name = "zoomTextBox";
            this.zoomTextBox.ReadOnly = true;
            this.zoomTextBox.Size = new System.Drawing.Size(292, 20);
            this.zoomTextBox.TabIndex = 0;
            this.zoomTextBox.Text = "Zoom: 1.0";
            // 
            // mainMenu
            // 
            this.mainMenu.MenuItems.AddRange(new System.Windows.Forms.MenuItem[] {
            viewMenuItem});
            // 
            // viewMenuItem
            // 
            viewMenuItem.Index = 0;
            viewMenuItem.MenuItems.AddRange(new System.Windows.Forms.MenuItem[] {
            this.zoomInMenuItem,
            this.zoomOutMenuItem});
            viewMenuItem.Text = "View";
            // 
            // zoomInMenuItem
            // 
            this.zoomInMenuItem.Index = 0;
            this.zoomInMenuItem.Text = "Zoom in";
            this.zoomInMenuItem.Click += new System.EventHandler(this.zoomInMenuItem_Click);
            // 
            // zoomOutMenuItem
            // 
            this.zoomOutMenuItem.Index = 1;
            this.zoomOutMenuItem.Text = "Zoom out";
            this.zoomOutMenuItem.Click += new System.EventHandler(this.zoomOutMenuItem_Click);
            // 
            // ZoomForm
            // 
            this.AutoScaleDimensions = new System.Drawing.SizeF(6F, 13F);
            this.AutoScaleMode = System.Windows.Forms.AutoScaleMode.Font;
            this.ClientSize = new System.Drawing.Size(292, 101);
            this.Controls.Add(this.zoomTextBox);
            this.Name = "ZoomForm";
            this.Text = "ZoomForm";
            this.ResumeLayout(false);
            this.PerformLayout();

        }

        #endregion

        private System.Windows.Forms.MainMenu mainMenu;
        private System.Windows.Forms.TextBox zoomTextBox;
        private System.Windows.Forms.MenuItem zoomInMenuItem;
        private System.Windows.Forms.MenuItem zoomOutMenuItem;
    }
}

The above code works and does what I want, but I'm not sure if it is the right way to do it (using reflection to modify a private variable generally seems like the incorrect method). My questions are:

• Is there a better way to set a MenuItem's shortcut to Control++?
• Is this reflection-based method going to cause issues?

Answer 1

it uses reflection, and I'm not sure if it's future-proof

You'll get away with it, nothing particularly dangerous happens under the hood. The MainMenu/MenuItem classes are cast in stone and are never going to change anymore. You are future-proof for Windows versions, this short-cut behavior isn't actually implemented by Windows but was added to MenuItem. It is actually the Form.ProcessCmdKey() method that makes it work. Which is your cue to doing without hacking the menu item. Paste this code into your form class:

protected override bool ProcessCmdKey(ref Message msg, Keys keyData) {
    if (keyData == (Keys.Control | Keys.Oemplus)) {
        zoomInMenuItem.PerformClick();
        return true;
    }
    if (keyData == (Keys.Control | Keys.OemMinus)) {
        zoomOutMenuItem.PerformClick();
        return true;
    }
    return base.ProcessCmdKey(ref msg, keyData);
}

The shortcut key description you get is gawdawful, no normal human knows what "Oemplus" could possibly mean. Don't use the auto-generated one, write your own. You can't do this with the designer, it won't let you enter the tab character between the item text and the shortcut key description. But no problem in code:

public ZoomForm() {
    InitializeComponent();
    zoomInMenuItem.Text = "Zoom in\tCtrl +";
    zoomOutMenuItem.Text = "Zoom out\tCtrl -";
}

I'm using MenuItem and not the newer ToolStripMenuItem because...

That's not a very good reason. ToolStripMenuItem indeed doesn't give an out-of-the-box implementation for radiobutton behavior but it is very easy to add yourself. Winforms makes it simple to create your own ToolStrip item classes that are usable at design-time and can behave however you choose at runtime. Add a new class to your project and paste the code shown below. Compile. Use the Insert > RadioItem context menu item at design time to insert one, the Edit DropdownItems... context menu item make it easy to add several. You can set the Group property to indicate which items belong together and should behave as a radio group.

using System;
using System.Windows.Forms;
using System.Windows.Forms.Design;

[ToolStripItemDesignerAvailability(ToolStripItemDesignerAvailability.MenuStrip | ToolStripItemDesignerAvailability.ContextMenuStrip)]
public class ToolStripRadioItem : ToolStripMenuItem {
    public int Group { get; set; }

    protected override void OnClick(EventArgs e) {
        if (!this.DesignMode) {
            this.Checked = true;
            var parent = this.Owner as ToolStripDropDownMenu;
            if (parent != null) {
                foreach (var item in parent.Items) {
                    var sibling = item as ToolStripRadioItem;
                    if (sibling != null && sibling != this and sibling.Group == this.Group) sibling.Checked = false;
                }
            }
        }
        base.OnClick(e);
    }
}

Answer 2

Add the following class (e.g. as "MenuItemEx.cs"):

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace System.Windows.Forms
{
    public class MenuItemEx : MenuItem
    {
        public MenuItemEx()
        {

        }

        private Keys myShortcut = Keys.None;
        public new Keys Shortcut
        {
            get { return myShortcut; }
            set { myShortcut = value; UpdateShortcutText(); }
        }

        private string myText = string.Empty;

        public new string Text
        {
            get { return myText; }
            set
            {
                myText = value;
                UpdateShortcutText();
            }
        }

        private void UpdateShortcutText()
        {
            base.Text = myText;

            if (myShortcut != Keys.None)
                base.Text += "\t" + myShortcut.ToString(); // you can adjust that
        }
    }
}

Replace every MenuItem with MenuItemEx inside "ZoomForm.Designer.cs".

Add the following code to OnLoad of your form:

zoomInMenuItem.Shortcut = Keys.Control | Keys.Oemplus;
zoomOutMenuItem.Shortcut = Keys.Control | Keys.OemMinus;

Then add the following code to your form class:

protected override bool ProcessCmdKey(ref Message msg, Keys keyData)
{
    MenuItem item = FindMenuItem(mainMenu.MenuItems, keyData);

    if (item != null)
    {
        item.PerformClick();
        return true;
    }

    return base.ProcessCmdKey(ref msg, keyData);
}

private MenuItem FindMenuItem(Menu.MenuItemCollection collection, Keys shortcut)
{
    foreach (MenuItem item in collection)
    {
        if (item is MenuItemEx && (item as MenuItemEx).Shortcut == shortcut)
            return item;

        MenuItem sub = FindMenuItem(item.MenuItems, shortcut);

        if (sub != null)
            return sub;
    }

    return null;
}

If you want you can also add your own property for the display string of the shortcut to the MenuItemEx class. You can also see and set this properties in the designer. You even get the new shortcut input dialog for the above property Shortcut if you use MenuItemEx.