Skip to content

Commit c9d5159

Browse files
committed
Implement proper radio group handling for GTK menus
Added thread-safe static mapping of radio group IDs to GTK radio menu item groups. The new ApplyRadioGroup method ensures radio menu items are correctly grouped in GTK when SetRadioGroup is called.
1 parent b21d5f0 commit c9d5159

File tree

1 file changed

+72
-11
lines changed

1 file changed

+72
-11
lines changed

src/platform/linux/menu_linux.cpp

Lines changed: 72 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
#include <optional>
77
#include <string>
88
#include <vector>
9+
#include <unordered_map>
10+
#include <mutex>
911
#include "../../foundation/id_allocator.h"
1012
#include "../../image.h"
1113
#include "../../menu.h"
@@ -22,6 +24,25 @@ static void OnGtkMenuItemActivate(GtkMenuItem* /*item*/, gpointer user_data) {
2224
menu_item->Emit(MenuItemClickedEvent(menu_item->GetId()));
2325
}
2426

27+
// For checkbox and radio items we listen to "toggled" to avoid recursive
28+
// activate emissions when GTK internally updates group state. For radio items,
29+
// we only emit when the item becomes active.
30+
static void OnGtkCheckMenuItemToggled(GtkCheckMenuItem* item, gpointer user_data) {
31+
MenuItem* menu_item = static_cast<MenuItem*>(user_data);
32+
if (!menu_item) {
33+
return;
34+
}
35+
gboolean active = gtk_check_menu_item_get_active(item);
36+
if (menu_item->GetType() == MenuItemType::Radio) {
37+
if (active) {
38+
menu_item->Emit(MenuItemClickedEvent(menu_item->GetId()));
39+
}
40+
} else {
41+
// Checkbox: emit on any toggle
42+
menu_item->Emit(MenuItemClickedEvent(menu_item->GetId()));
43+
}
44+
}
45+
2546
static void OnGtkMenuShow(GtkWidget* /*menu*/, gpointer user_data) {
2647
Menu* menu_obj = static_cast<Menu*>(user_data);
2748
if (!menu_obj) {
@@ -69,6 +90,29 @@ class MenuItem::Impl {
6990
radio_group_(-1),
7091
accelerator_("", KeyboardAccelerator::None) {}
7192

93+
void ApplyRadioGroup() {
94+
if (!gtk_menu_item_ || type_ != MenuItemType::Radio || radio_group_ < 0) {
95+
return;
96+
}
97+
98+
std::lock_guard<std::mutex> lock(s_group_map_mutex_);
99+
100+
GSList* target_group = nullptr;
101+
auto it = s_group_map_.find(radio_group_);
102+
if (it != s_group_map_.end()) {
103+
target_group = it->second;
104+
} else {
105+
target_group = gtk_radio_menu_item_get_group(GTK_RADIO_MENU_ITEM(gtk_menu_item_));
106+
s_group_map_[radio_group_] = target_group;
107+
}
108+
109+
gtk_radio_menu_item_set_group(GTK_RADIO_MENU_ITEM(gtk_menu_item_), target_group);
110+
111+
// Update stored head pointer after potential re-linking
112+
s_group_map_[radio_group_] =
113+
gtk_radio_menu_item_get_group(GTK_RADIO_MENU_ITEM(gtk_menu_item_));
114+
}
115+
72116
MenuItemId id_;
73117
GtkWidget* gtk_menu_item_;
74118
std::optional<std::string> title_;
@@ -79,8 +123,16 @@ class MenuItem::Impl {
79123
int radio_group_;
80124
KeyboardAccelerator accelerator_;
81125
std::shared_ptr<Menu> submenu_;
126+
127+
// Shared map from logical group id to GTK group list
128+
static std::unordered_map<int, GSList*> s_group_map_;
129+
static std::mutex s_group_map_mutex_;
82130
};
83131

132+
// Static member definitions
133+
std::unordered_map<int, GSList*> MenuItem::Impl::s_group_map_;
134+
std::mutex MenuItem::Impl::s_group_map_mutex_;
135+
84136
MenuItem::MenuItem(const std::string& label, MenuItemType type) {
85137
MenuItemId id = IdAllocator::Allocate<MenuItem>();
86138
GtkWidget* gtk_item = nullptr;
@@ -110,9 +162,13 @@ MenuItem::MenuItem(const std::string& label, MenuItemType type) {
110162
pimpl_->title_.reset();
111163
}
112164

113-
// Connect activation signal for click events (except separators)
165+
// Connect signals for click/toggle events (except separators)
114166
if (gtk_item && type != MenuItemType::Separator) {
115-
g_signal_connect(G_OBJECT(gtk_item), "activate", G_CALLBACK(OnGtkMenuItemActivate), this);
167+
if (type == MenuItemType::Checkbox || type == MenuItemType::Radio) {
168+
g_signal_connect(G_OBJECT(gtk_item), "toggled", G_CALLBACK(OnGtkCheckMenuItemToggled), this);
169+
} else {
170+
g_signal_connect(G_OBJECT(gtk_item), "activate", G_CALLBACK(OnGtkMenuItemActivate), this);
171+
}
116172
}
117173
}
118174

@@ -129,8 +185,13 @@ MenuItem::MenuItem(void* menu_item) {
129185
}
130186

131187
if (pimpl_->gtk_menu_item_) {
132-
g_signal_connect(G_OBJECT(pimpl_->gtk_menu_item_), "activate",
133-
G_CALLBACK(OnGtkMenuItemActivate), this);
188+
if (GTK_IS_CHECK_MENU_ITEM(pimpl_->gtk_menu_item_)) {
189+
g_signal_connect(G_OBJECT(pimpl_->gtk_menu_item_), "toggled",
190+
G_CALLBACK(OnGtkCheckMenuItemToggled), this);
191+
} else {
192+
g_signal_connect(G_OBJECT(pimpl_->gtk_menu_item_), "activate",
193+
G_CALLBACK(OnGtkMenuItemActivate), this);
194+
}
134195
}
135196
}
136197

@@ -284,20 +345,20 @@ void MenuItem::SetState(MenuItemState state) {
284345
if (pimpl_->gtk_menu_item_) {
285346
if (pimpl_->type_ == MenuItemType::Checkbox) {
286347
gboolean active = (state == MenuItemState::Checked) ? TRUE : FALSE;
287-
// Block the "activate" signal to prevent recursive triggering
348+
// Block the "toggled" signal to prevent recursive triggering
288349
g_signal_handlers_block_by_func(G_OBJECT(pimpl_->gtk_menu_item_),
289-
(gpointer)OnGtkMenuItemActivate, this);
350+
(gpointer)OnGtkCheckMenuItemToggled, this);
290351
gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(pimpl_->gtk_menu_item_), active);
291352
g_signal_handlers_unblock_by_func(G_OBJECT(pimpl_->gtk_menu_item_),
292-
(gpointer)OnGtkMenuItemActivate, this);
353+
(gpointer)OnGtkCheckMenuItemToggled, this);
293354
} else if (pimpl_->type_ == MenuItemType::Radio) {
294355
gboolean active = (state == MenuItemState::Checked) ? TRUE : FALSE;
295-
// Block the "activate" signal to prevent recursive triggering
356+
// Block the "toggled" signal to prevent recursive triggering
296357
g_signal_handlers_block_by_func(G_OBJECT(pimpl_->gtk_menu_item_),
297-
(gpointer)OnGtkMenuItemActivate, this);
358+
(gpointer)OnGtkCheckMenuItemToggled, this);
298359
gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(pimpl_->gtk_menu_item_), active);
299360
g_signal_handlers_unblock_by_func(G_OBJECT(pimpl_->gtk_menu_item_),
300-
(gpointer)OnGtkMenuItemActivate, this);
361+
(gpointer)OnGtkCheckMenuItemToggled, this);
301362
}
302363
}
303364
}
@@ -314,7 +375,7 @@ MenuItemState MenuItem::GetState() const {
314375

315376
void MenuItem::SetRadioGroup(int group_id) {
316377
pimpl_->radio_group_ = group_id;
317-
// TODO: Implement proper radio grouping for GTK
378+
pimpl_->ApplyRadioGroup();
318379
}
319380

320381
int MenuItem::GetRadioGroup() const {

0 commit comments

Comments
 (0)