Use MenuManager for processing and handling menu state changes

v2/internal/platform/menu/manager.go

@@ -0,0 +1,145 @@
+package menu
+
+import (
+	"github.com/wailsapp/wails/v2/pkg/menu"
+)
+
+// MenuManager manages the menus for the application
+var MenuManager = NewManager()
+
+type radioGroup []*menu.MenuItem
+
+// Click updates the radio group state based on the item clicked
+func (g *radioGroup) Click(item *menu.MenuItem) {
+	for _, radioGroupItem := range *g {
+		if radioGroupItem != item {
+			radioGroupItem.Checked = false
+		}
+	}
+}
+
+type processedMenu struct {
+
+	// the menu we processed
+	menu *menu.Menu
+
+	// updateMenuItemCallback is called when the menu item needs to be updated in the UI
+	updateMenuItemCallback func(*menu.MenuItem)
+
+	// items is a map of all menu items in this menu
+	items map[*menu.MenuItem]struct{}
+
+	// radioGroups tracks which radiogroup a menu item belongs to
+	radioGroups map[*menu.MenuItem][]*radioGroup
+}
+
+func newProcessedMenu(topLevelMenu *menu.Menu, updateMenuItemCallback func(*menu.MenuItem)) *processedMenu {
+	result := &processedMenu{
+		updateMenuItemCallback: updateMenuItemCallback,
+		menu:                   topLevelMenu,
+		items:                  make(map[*menu.MenuItem]struct{}),
+		radioGroups:            make(map[*menu.MenuItem][]*radioGroup),
+	}
+	result.process(topLevelMenu.Items)
+	return result
+}
+
+func (p *processedMenu) process(items []*menu.MenuItem) {
+	var currentRadioGroup radioGroup
+	for index, item := range items {
+		// Save the reference to the top level menu for this item
+		p.items[item] = struct{}{}
+
+		// If this is a radio item, add it to the radio group
+		if item.Type == menu.RadioType {
+			currentRadioGroup = append(currentRadioGroup, item)
+		}
+
+		// If this is not a radio item, or we are processing the last item in the menu,
+		// then we need to add the current radio group to the map if it has items
+		if item.Type != menu.RadioType || index == len(items)-1 {
+			if len(currentRadioGroup) > 0 {
+				p.addRadioGroup(currentRadioGroup)
+				currentRadioGroup = nil
+			}
+		}
+
+		// Process the submenu
+		if item.SubMenu != nil {
+			p.process(item.SubMenu.Items)
+		}
+	}
+}
+
+func (p *processedMenu) processClick(item *menu.MenuItem) {
+	// If this item is not in our menu, then we can't process it
+	if _, ok := p.items[item]; !ok {
+		return
+	}
+
+	// If this is a radio item, then we need to update the radio group
+	if item.Type == menu.RadioType {
+		// Get the radio groups for this item
+		radioGroups := p.radioGroups[item]
+		// Iterate each radio group this item belongs to and set the checked state
+		// of all items apart from the one that was clicked to false
+		for _, thisRadioGroup := range radioGroups {
+			thisRadioGroup.Click(item)
+			for _, thisRadioGroupItem := range *thisRadioGroup {
+				p.updateMenuItemCallback(thisRadioGroupItem)
+			}
+		}
+	}
+
+	if item.Type == menu.CheckboxType {
+		p.updateMenuItemCallback(item)
+	}
+
+}
+
+func (p *processedMenu) addRadioGroup(r radioGroup) {
+	for _, item := range r {
+		p.radioGroups[item] = append(p.radioGroups[item], &r)
+	}
+}
+
+type Manager struct {
+	menus map[*menu.Menu]*processedMenu
+}
+
+func NewManager() *Manager {
+	return &Manager{
+		menus: make(map[*menu.Menu]*processedMenu),
+	}
+}
+
+func (m *Manager) AddMenu(menu *menu.Menu, updateMenuItemCallback func(*menu.MenuItem)) {
+	m.menus[menu] = newProcessedMenu(menu, updateMenuItemCallback)
+}
+
+func (m *Manager) ProcessClick(item *menu.MenuItem) {
+
+	// if menuitem is a checkbox, then we need to toggle the state
+	if item.Type == menu.CheckboxType {
+		item.Checked = !item.Checked
+	}
+
+	// Set the radio item to checked
+	if item.Type == menu.RadioType {
+		item.Checked = true
+	}
+
+	for _, thisMenu := range m.menus {
+		thisMenu.processClick(item)
+	}
+
+	if item.Click != nil {
+		item.Click(&menu.CallbackData{
+			MenuItem: item,
+		})
+	}
+}
+
+func (m *Manager) RemoveMenu(data *menu.Menu) {
+	delete(m.menus, data)
+}

v2/internal/platform/menu/manager_test.go

@@ -0,0 +1,295 @@
+package menu_test
+
+import (
+	"github.com/stretchr/testify/require"
+	platformMenu "github.com/wailsapp/wails/v2/internal/platform/menu"
+	"github.com/wailsapp/wails/v2/pkg/menu"
+	"testing"
+)
+
+func TestManager_ProcessClick_Checkbox(t *testing.T) {
+
+	checkbox := menu.Label("Checkbox").SetChecked(false)
+	menu1 := &menu.Menu{
+		Items: []*menu.MenuItem{
+			checkbox,
+		},
+	}
+	menu2 := &menu.Menu{
+		Items: []*menu.MenuItem{
+			checkbox,
+		},
+	}
+	menuWithNoCheckbox := &menu.Menu{
+		Items: []*menu.MenuItem{
+			menu.Label("No Checkbox"),
+		},
+	}
+	clicked := false
+
+	tests := []struct {
+		name                string
+		inputs              []*menu.Menu
+		startState          bool
+		expectedState       bool
+		expectedMenuUpdates map[*menu.Menu][]*menu.MenuItem
+		click               func(*menu.CallbackData)
+	}{
+		{
+			name:   "should callback menu checkbox state when clicked (false -> true)",
+			inputs: []*menu.Menu{menu1},
+			expectedMenuUpdates: map[*menu.Menu][]*menu.MenuItem{
+				menu1: {checkbox},
+			},
+			startState:    false,
+			expectedState: true,
+		},
+		{
+			name:          "should callback multiple menus when checkbox state when clicked (false -> true)",
+			inputs:        []*menu.Menu{menu1, menu2},
+			startState:    false,
+			expectedState: true,
+			expectedMenuUpdates: map[*menu.Menu][]*menu.MenuItem{
+				menu1: {checkbox},
+				menu2: {checkbox},
+			},
+		},
+		{
+			name:          "should callback only for the menus that the checkbox is in (false -> true)",
+			inputs:        []*menu.Menu{menu1, menuWithNoCheckbox},
+			startState:    false,
+			expectedState: true,
+			expectedMenuUpdates: map[*menu.Menu][]*menu.MenuItem{
+				menu1: {checkbox},
+			},
+		},
+		{
+			name:   "should callback menu checkbox state when clicked (true->false)",
+			inputs: []*menu.Menu{menu1},
+			expectedMenuUpdates: map[*menu.Menu][]*menu.MenuItem{
+				menu1: {checkbox},
+			},
+			startState:    true,
+			expectedState: false,
+		},
+		{
+			name:          "should callback multiple menus when checkbox state when clicked (true->false)",
+			inputs:        []*menu.Menu{menu1, menu2},
+			startState:    true,
+			expectedState: false,
+			expectedMenuUpdates: map[*menu.Menu][]*menu.MenuItem{
+				menu1: {checkbox},
+				menu2: {checkbox},
+			},
+		},
+		{
+			name:          "should callback only for the menus that the checkbox is in (true->false)",
+			inputs:        []*menu.Menu{menu1, menuWithNoCheckbox},
+			startState:    true,
+			expectedState: false,
+			expectedMenuUpdates: map[*menu.Menu][]*menu.MenuItem{
+				menu1: {checkbox},
+			},
+		},
+		{
+			name:                "should callback no menus if checkbox not in them",
+			inputs:              []*menu.Menu{menuWithNoCheckbox},
+			startState:          false,
+			expectedState:       false,
+			expectedMenuUpdates: nil,
+		},
+		{
+			name:          "should call Click on the checkbox",
+			inputs:        []*menu.Menu{menu1, menu2},
+			startState:    false,
+			expectedState: true,
+			expectedMenuUpdates: map[*menu.Menu][]*menu.MenuItem{
+				menu1: {checkbox},
+				menu2: {checkbox},
+			},
+			click: func(data *menu.CallbackData) {
+				clicked = true
+			},
+		},
+	}
+	for _, tt := range tests {
+
+		menusUpdated := map[*menu.Menu][]*menu.MenuItem{}
+		clicked = false
+
+		var checkMenuItemStateInMenu func(menu *menu.Menu)
+
+		checkMenuItemStateInMenu = func(menu *menu.Menu) {
+			for _, item := range menusUpdated[menu] {
+				if item == checkbox {
+					require.Equal(t, tt.expectedState, item.Checked)
+				}
+				if item.SubMenu != nil {
+					checkMenuItemStateInMenu(item.SubMenu)
+				}
+			}
+		}
+
+		t.Run(tt.name, func(t *testing.T) {
+			m := platformMenu.NewManager()
+			checkbox.SetChecked(tt.startState)
+			checkbox.Click = tt.click
+			for _, thisMenu := range tt.inputs {
+				thisMenu := thisMenu
+				m.AddMenu(thisMenu, func(menuItem *menu.MenuItem) {
+					menusUpdated[thisMenu] = append(menusUpdated[thisMenu], menuItem)
+				})
+			}
+			m.ProcessClick(checkbox)
+
+			// Check the item has the correct state in all the menus
+			for thisMenu := range menusUpdated {
+				require.EqualValues(t, tt.expectedMenuUpdates[thisMenu], menusUpdated[thisMenu])
+			}
+
+			if tt.click != nil {
+				require.Equal(t, true, clicked)
+			}
+		})
+	}
+}
+
+func TestManager_ProcessClick_RadioGroups(t *testing.T) {
+
+	radio1 := menu.Radio("Radio1", false, nil, nil)
+	radio2 := menu.Radio("Radio2", false, nil, nil)
+	radio3 := menu.Radio("Radio3", false, nil, nil)
+	radio4 := menu.Radio("Radio4", false, nil, nil)
+	radio5 := menu.Radio("Radio5", false, nil, nil)
+	radio6 := menu.Radio("Radio6", false, nil, nil)
+
+	radioGroupOne := &menu.Menu{
+		Items: []*menu.MenuItem{
+			radio1,
+			radio2,
+			radio3,
+		},
+	}
+
+	radioGroupTwo := &menu.Menu{
+		Items: []*menu.MenuItem{
+			radio4,
+			radio5,
+			radio6,
+		},
+	}
+
+	radioGroupThree := &menu.Menu{
+		Items: []*menu.MenuItem{
+			radio1,
+			radio2,
+			radio3,
+		},
+	}
+
+	clicked := false
+
+	tests := []struct {
+		name                string
+		inputs              []*menu.Menu
+		startState          map[*menu.MenuItem]bool
+		selected            *menu.MenuItem
+		expectedMenuUpdates map[*menu.Menu][]*menu.MenuItem
+		click               func(*menu.CallbackData)
+		expectedState       map[*menu.MenuItem]bool
+	}{
+		{
+			name:   "should only set the clicked radio item",
+			inputs: []*menu.Menu{radioGroupOne},
+			expectedMenuUpdates: map[*menu.Menu][]*menu.MenuItem{
+				radioGroupOne: {radio1, radio2, radio3},
+			},
+			startState: map[*menu.MenuItem]bool{
+				radio1: true,
+				radio2: false,
+				radio3: false,
+			},
+			selected: radio2,
+			expectedState: map[*menu.MenuItem]bool{
+				radio1: false,
+				radio2: true,
+				radio3: false,
+			},
+		},
+		{
+			name:   "should not affect other radio groups or menus",
+			inputs: []*menu.Menu{radioGroupOne, radioGroupTwo},
+			expectedMenuUpdates: map[*menu.Menu][]*menu.MenuItem{
+				radioGroupOne: {radio1, radio2, radio3},
+			},
+			startState: map[*menu.MenuItem]bool{
+				radio1: true,
+				radio2: false,
+				radio3: false,
+				radio4: true,
+				radio5: false,
+				radio6: false,
+			},
+			selected: radio2,
+			expectedState: map[*menu.MenuItem]bool{
+				radio1: false,
+				radio2: true,
+				radio3: false,
+				radio4: true,
+				radio5: false,
+				radio6: false,
+			},
+		},
+		{
+			name:   "menus with the same radio group should be updated",
+			inputs: []*menu.Menu{radioGroupOne, radioGroupThree},
+			expectedMenuUpdates: map[*menu.Menu][]*menu.MenuItem{
+				radioGroupOne:   {radio1, radio2, radio3},
+				radioGroupThree: {radio1, radio2, radio3},
+			},
+			startState: map[*menu.MenuItem]bool{
+				radio1: true,
+				radio2: false,
+				radio3: false,
+			},
+			selected: radio2,
+			expectedState: map[*menu.MenuItem]bool{
+				radio1: false,
+				radio2: true,
+				radio3: false,
+			},
+		},
+	}
+	for _, tt := range tests {
+
+		menusUpdated := map[*menu.Menu][]*menu.MenuItem{}
+		clicked = false
+
+		t.Run(tt.name, func(t *testing.T) {
+			m := platformMenu.NewManager()
+
+			for item, value := range tt.startState {
+				item.SetChecked(value)
+			}
+
+			tt.selected.Click = tt.click
+			for _, thisMenu := range tt.inputs {
+				thisMenu := thisMenu
+				m.AddMenu(thisMenu, func(menuItem *menu.MenuItem) {
+					menusUpdated[thisMenu] = append(menusUpdated[thisMenu], menuItem)
+				})
+			}
+			m.ProcessClick(tt.selected)
+			require.Equal(t, tt.expectedMenuUpdates, menusUpdated)
+
+			// Check the items have the correct state in all the menus
+			for item, expectedValue := range tt.expectedState {
+				require.Equal(t, expectedValue, item.Checked)
+			}
+
+			if tt.click != nil {
+				require.Equal(t, true, clicked)
+			}
+		})
+	}
+}

v2/internal/platform/systray/menu.go

@@ -2,6 +2,7 @@ package systray
 
 import (
 	"errors"
+	platformMenu "github.com/wailsapp/wails/v2/internal/platform/menu"
 	"github.com/wailsapp/wails/v2/internal/platform/win32"
 	"github.com/wailsapp/wails/v2/pkg/menu"
 )
@@ -62,6 +63,7 @@ func (p *PopupMenu) buildMenu(parentMenu win32.PopupMenu, inputMenu *menu.Menu,
 
 func (p *PopupMenu) Update() error {
 	p.menu = win32.CreatePopupMenu()
+	p.menuMapping = make(map[int]*menu.MenuItem)
 	return p.buildMenu(p.menu, p.menuData, win32.MenuItemMsgID)
 }
 
@@ -69,10 +71,10 @@ func NewPopupMenu(parent win32.HWND, inputMenu *menu.Menu) (*PopupMenu, error) {
 	result := &PopupMenu{
 		parent:        parent,
 		menuData:      inputMenu,
-		menuMapping:   make(map[int]*menu.MenuItem),
 		checkboxItems: make(map[*menu.MenuItem][]int),
 	}
 	err := result.Update()
+	platformMenu.MenuManager.AddMenu(inputMenu, result.UpdateMenuItem)
 	return result, err
 }
 
@@ -99,20 +101,17 @@ func (p *PopupMenu) ShowAtCursor() error {
 
 func (p *PopupMenu) ProcessCommand(cmdMsgID int) {
 	item := p.menuMapping[cmdMsgID]
-	if item != nil {
-		if item.Type == menu.CheckboxType {
-			item.Checked = !item.Checked
-			for _, menuID := range p.checkboxItems[item] {
-				p.menu.Check(uintptr(menuID), item.Checked)
-			}
-			// TODO: Check duplicate menu items
-		}
-		if item.Click != nil {
-			item.Click(&menu.CallbackData{MenuItem: item})
-		}
-	}
+	platformMenu.MenuManager.ProcessClick(item)
 }
 
 func (p *PopupMenu) Destroy() {
 	p.menu.Destroy()
 }
+
+func (p *PopupMenu) UpdateMenuItem(item *menu.MenuItem) {
+	if item.IsCheckbox() {
+		for _, itemID := range p.checkboxItems[item] {
+			p.menu.Check(uintptr(itemID), item.Checked)
+		}
+	}
+}

v2/pkg/menu/menuitem.go

@@ -253,7 +253,9 @@ func (m *MenuItem) SetAccelerator(acc *keys.Accelerator) *MenuItem {
 
 func (m *MenuItem) SetChecked(value bool) *MenuItem {
 	m.Checked = value
-	m.Type = CheckboxType
+	if m.Type != RadioType {
+		m.Type = CheckboxType
+	}
 	return m
 }