Skip to content

Commit 297beee

Browse files
committed
Fix Windows menu item state and enabled properties ignored before addItem
On Windows, MenuItem.state and MenuItem.enabled properties set before the item is added to a Menu via addItem() or insertItem() were silently ignored. The root cause had two parts: 1. MenuItem::Impl had no enabled_ field - SetEnabled() only called EnableMenuItem() if parent_menu_ was set (null before addItem), and IsEnabled() fell back to returning true when parent_menu_ was null. 2. Menu::AddItem() and Menu::InsertItem() hardcoded MF_UNCHECKED for checkbox/radio types instead of reading the item's current state_, and never applied MF_GRAYED for disabled items. Fixes applied: - Added bool enabled_ field to MenuItem::Impl (default true) - SetEnabled() always stores to enabled_ regardless of parent_menu_ - IsEnabled() reads from enabled_ field directly - AddItem() reads item->GetState() for MF_CHECKED/MF_UNCHECKED - AddItem() checks item->IsEnabled() for MF_GRAYED - InsertItem() same fixes plus submenu support (was missing entirely) Resolves libnativeapi/nativeapi-flutter#4 Resolves libnativeapi/nativeapi-flutter#5
1 parent 00b6fff commit 297beee

2 files changed

Lines changed: 33 additions & 10 deletions

File tree

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ add_subdirectory(examples/display_c_example)
1717
add_subdirectory(examples/id_allocator_example)
1818
add_subdirectory(examples/keyboard_example)
1919
add_subdirectory(examples/menu_example)
20+
add_subdirectory(examples/menu_state_example)
2021
add_subdirectory(examples/menu_c_example)
2122
add_subdirectory(examples/message_dialog_example)
2223
add_subdirectory(examples/message_dialog_c_example)

src/platform/windows/menu_windows.cpp

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ class MenuItem::Impl {
107107
KeyboardAccelerator accelerator_;
108108
bool has_accelerator_;
109109
MenuItemState state_;
110+
bool enabled_;
110111
int radio_group_;
111112
std::shared_ptr<Menu> submenu_;
112113
int window_proc_handle_id_;
@@ -123,6 +124,7 @@ class MenuItem::Impl {
123124
accelerator_("", ModifierKey::None),
124125
has_accelerator_(false),
125126
state_(MenuItemState::Unchecked),
127+
enabled_(true),
126128
radio_group_(-1),
127129
window_proc_handle_id_(-1),
128130
submenu_opened_listener_id_(0),
@@ -355,19 +357,14 @@ KeyboardAccelerator MenuItem::GetAccelerator() const {
355357
}
356358

357359
void MenuItem::SetEnabled(bool enabled) {
360+
pimpl_->enabled_ = enabled;
358361
if (pimpl_->parent_menu_) {
359362
EnableMenuItem(pimpl_->parent_menu_, pimpl_->id_, enabled ? MF_ENABLED : MF_GRAYED);
360363
}
361364
}
362365

363366
bool MenuItem::IsEnabled() const {
364-
if (pimpl_->parent_menu_) {
365-
UINT state = GetMenuState(pimpl_->parent_menu_, pimpl_->id_, MF_BYCOMMAND);
366-
if (state != (UINT)-1) {
367-
return !(state & MF_GRAYED);
368-
}
369-
}
370-
return true;
367+
return pimpl_->enabled_;
371368
}
372369

373370
void MenuItem::SetState(MenuItemState state) {
@@ -575,9 +572,15 @@ void Menu::AddItem(std::shared_ptr<MenuItem> item) {
575572
if (item->GetType() == MenuItemType::Separator) {
576573
flags = MF_SEPARATOR;
577574
} else if (item->GetType() == MenuItemType::Checkbox) {
578-
flags |= MF_UNCHECKED;
575+
flags |= (item->GetState() == MenuItemState::Checked) ? MF_CHECKED
576+
: MF_UNCHECKED;
579577
} else if (item->GetType() == MenuItemType::Radio) {
580-
flags |= MF_UNCHECKED;
578+
flags |= (item->GetState() == MenuItemState::Checked) ? MF_CHECKED
579+
: MF_UNCHECKED;
580+
}
581+
582+
if (!item->IsEnabled()) {
583+
flags |= MF_GRAYED;
581584
}
582585

583586
UINT_PTR menu_id = item->GetId();
@@ -611,12 +614,31 @@ void Menu::InsertItem(size_t index, std::shared_ptr<MenuItem> item) {
611614
UINT flags = MF_STRING | MF_BYPOSITION;
612615
if (item->GetType() == MenuItemType::Separator) {
613616
flags = MF_SEPARATOR | MF_BYPOSITION;
617+
} else if (item->GetType() == MenuItemType::Checkbox) {
618+
flags |= (item->GetState() == MenuItemState::Checked) ? MF_CHECKED
619+
: MF_UNCHECKED;
620+
} else if (item->GetType() == MenuItemType::Radio) {
621+
flags |= (item->GetState() == MenuItemState::Checked) ? MF_CHECKED
622+
: MF_UNCHECKED;
623+
}
624+
625+
if (!item->IsEnabled()) {
626+
flags |= MF_GRAYED;
627+
}
628+
629+
UINT_PTR menu_id = item->GetId();
630+
if (item->GetSubmenu()) {
631+
auto sub_menu =
632+
static_cast<HMENU>(item->GetSubmenu()->GetNativeObject());
633+
flags |= MF_POPUP;
634+
menu_id = reinterpret_cast<UINT_PTR>(sub_menu);
614635
}
615636

616637
auto label_opt = item->GetLabel();
617638
std::string label_str = label_opt.has_value() ? *label_opt : "";
618639
std::wstring w_label_str = StringToWString(label_str);
619-
InsertMenuW(pimpl_->hmenu_, static_cast<UINT>(index), flags, item->GetId(), w_label_str.c_str());
640+
InsertMenuW(pimpl_->hmenu_, static_cast<UINT>(index), flags, menu_id,
641+
w_label_str.c_str());
620642

621643
item->pimpl_->parent_menu_ = pimpl_->hmenu_;
622644
}

0 commit comments

Comments
 (0)