Skip to content

Commit 0cad78b

Browse files
committed
Improve menu item ID handling for Windows popup menus
Refactored WM_COMMAND handling to correctly extract the full 32-bit menu item ID for popup menus, ensuring reliable event triggering. Also updated parameter naming for clarity and improved code formatting in constructors and InsertItem.
1 parent 2512364 commit 0cad78b

File tree

1 file changed

+45
-20
lines changed

1 file changed

+45
-20
lines changed

src/platform/windows/menu_windows.cpp

Lines changed: 45 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,15 @@ class MenuItem::Impl {
122122
};
123123

124124
MenuItem::MenuItem(void* native_item)
125-
: pimpl_(std::make_unique<Impl>(IdAllocator::Allocate<MenuItem>(), nullptr, MenuItemType::Normal)) {}
126-
127-
MenuItem::MenuItem(const std::string& text, MenuItemType type)
128-
: pimpl_(std::make_unique<Impl>(IdAllocator::Allocate<MenuItem>(), nullptr, type)) {
129-
pimpl_->label_ = text;
125+
: pimpl_(std::make_unique<Impl>(IdAllocator::Allocate<MenuItem>(),
126+
nullptr,
127+
MenuItemType::Normal)) {}
128+
129+
MenuItem::MenuItem(const std::string& label, MenuItemType type)
130+
: pimpl_(std::make_unique<Impl>(IdAllocator::Allocate<MenuItem>(),
131+
nullptr,
132+
type)) {
133+
pimpl_->label_ = label;
130134
}
131135

132136
MenuItem::~MenuItem() {}
@@ -329,28 +333,48 @@ class Menu::Impl {
329333
UINT message,
330334
WPARAM wparam,
331335
LPARAM lparam) {
332-
std::cout << "Menu: HandleWindowProc, message = " << message << std::endl;
333336
if (message == WM_COMMAND) {
334-
UINT menu_item_id = LOWORD(wparam);
335-
336-
std::cout << "Menu: HandleWindowProc, menu_item_id = " << menu_item_id << std::endl;
337-
338-
// Find the menu item by Windows menu item ID
339-
for (const auto& item : items_) {
340-
if (item->pimpl_->id_ == menu_item_id) {
341-
std::cout << "Menu: Item clicked, menu_item_id = " << menu_item_id << std::endl;
342-
// Call Trigger() to emit the event
343-
item->Trigger();
344-
return 0;
337+
// For WM_COMMAND from menus, wparam contains the menu item ID
338+
// When using popup menus (TrackPopupMenu), the full 32-bit ID is
339+
// preserved in wparam, unlike menu bars which only use 16-bit IDs
340+
WORD hiword = HIWORD(wparam);
341+
WORD loword = LOWORD(wparam);
342+
343+
std::cout << "Menu: WM_COMMAND - HIWORD=" << hiword
344+
<< ", LOWORD=" << loword << std::endl;
345+
346+
// For popup menus, lparam is NULL and wparam contains menu item ID
347+
if (lparam == 0) {
348+
// Reconstruct the full 32-bit menu item ID from wparam
349+
// wparam for menus contains the full ID we passed to AppendMenuW
350+
MenuItemId menu_item_id = static_cast<MenuItemId>(wparam);
351+
352+
std::cout << "Menu: Looking for menu item with ID = " << menu_item_id
353+
<< " (0x" << std::hex << menu_item_id << std::dec << ")"
354+
<< std::endl;
355+
356+
// Find the menu item by IdAllocator-generated ID
357+
for (const auto& item : items_) {
358+
if (item->pimpl_->id_ == menu_item_id) {
359+
std::cout << "Menu: Item clicked, ID = " << menu_item_id
360+
<< std::endl;
361+
// Call Trigger() to emit the event
362+
item->Trigger();
363+
return 0;
364+
}
345365
}
366+
367+
std::cout << "Menu: No matching menu item found for ID " << menu_item_id
368+
<< std::endl;
346369
}
347370
}
348371
return std::nullopt; // Let default window procedure handle it
349372
}
350373
};
351374

352375
Menu::Menu(void* native_menu)
353-
: pimpl_(std::make_unique<Impl>(IdAllocator::Allocate<Menu>(), static_cast<HMENU>(native_menu))) {
376+
: pimpl_(std::make_unique<Impl>(IdAllocator::Allocate<Menu>(),
377+
static_cast<HMENU>(native_menu))) {
354378
// Register window procedure handler for menu commands
355379
HWND host_window = WindowMessageDispatcher::GetInstance().GetHostWindow();
356380
if (host_window) {
@@ -364,7 +388,8 @@ Menu::Menu(void* native_menu)
364388
}
365389

366390
Menu::Menu()
367-
: pimpl_(std::make_unique<Impl>(IdAllocator::Allocate<Menu>(), CreatePopupMenu())) {
391+
: pimpl_(std::make_unique<Impl>(IdAllocator::Allocate<Menu>(),
392+
CreatePopupMenu())) {
368393
// Register window procedure handler for menu commands
369394
HWND host_window = WindowMessageDispatcher::GetInstance().GetHostWindow();
370395
if (host_window) {
@@ -435,7 +460,7 @@ void Menu::InsertItem(size_t index, std::shared_ptr<MenuItem> item) {
435460
std::string label_str = label_opt.has_value() ? *label_opt : "";
436461
std::wstring w_label_str = StringToWString(label_str);
437462
InsertMenuW(pimpl_->hmenu_, static_cast<UINT>(index), flags, item->GetId(),
438-
w_label_str.c_str());
463+
w_label_str.c_str());
439464

440465
item->pimpl_->parent_menu_ = pimpl_->hmenu_;
441466
}

0 commit comments

Comments
 (0)