Skip to content

Commit 85bede2

Browse files
lijy91lijy91
committed
Refactor menu item accelerator and submenu removal
Removed explicit RemoveAccelerator and RemoveSubmenu methods from MenuItem and C API, replacing them with SetAccelerator(nullptr) and SetSubmenu(nullptr) to remove accelerators and submenus. Updated documentation and platform-specific implementations to support this unified approach.
1 parent c510a21 commit 85bede2

File tree

6 files changed

+82
-139
lines changed

6 files changed

+82
-139
lines changed

‎src/capi/menu_c.cpp‎

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -335,13 +335,17 @@ char* native_menu_item_get_tooltip(native_menu_item_t item) {
335335

336336
void native_menu_item_set_accelerator(native_menu_item_t item,
337337
const native_keyboard_accelerator_t* accelerator) {
338-
if (!item || !accelerator)
338+
if (!item)
339339
return;
340340

341341
try {
342342
auto menu_item = static_cast<MenuItem*>(item);
343-
KeyboardAccelerator cpp_accelerator = convert_keyboard_accelerator(accelerator);
344-
menu_item->SetAccelerator(cpp_accelerator);
343+
if (accelerator) {
344+
KeyboardAccelerator cpp_accelerator = convert_keyboard_accelerator(accelerator);
345+
menu_item->SetAccelerator(cpp_accelerator);
346+
} else {
347+
menu_item->SetAccelerator(std::nullopt);
348+
}
345349
} catch (...) {
346350
// Ignore exceptions
347351
}
@@ -367,17 +371,7 @@ bool native_menu_item_get_accelerator(native_menu_item_t item,
367371
}
368372
}
369373

370-
void native_menu_item_remove_accelerator(native_menu_item_t item) {
371-
if (!item)
372-
return;
373374

374-
try {
375-
auto menu_item = static_cast<MenuItem*>(item);
376-
menu_item->RemoveAccelerator();
377-
} catch (...) {
378-
// Ignore exceptions
379-
}
380-
}
381375

382376
void native_menu_item_set_enabled(native_menu_item_t item, bool enabled) {
383377
if (!item)
@@ -452,7 +446,7 @@ int native_menu_item_get_radio_group(native_menu_item_t item) {
452446
}
453447

454448
void native_menu_item_set_submenu(native_menu_item_t item, native_menu_t submenu) {
455-
if (!item || !submenu)
449+
if (!item)
456450
return;
457451

458452
try {
@@ -461,10 +455,14 @@ void native_menu_item_set_submenu(native_menu_item_t item, native_menu_t submenu
461455
return;
462456

463457
auto menu_item = static_cast<MenuItem*>(item);
464-
// Get the shared_ptr from global storage instead of creating a new one
465-
auto submenu_ptr = GlobalRegistry<Menu>().Get(submenu);
466-
if (submenu_ptr) {
467-
menu_item->SetSubmenu(submenu_ptr);
458+
if (submenu) {
459+
// Get the shared_ptr from global storage instead of creating a new one
460+
auto submenu_ptr = GlobalRegistry<Menu>().Get(submenu);
461+
if (submenu_ptr) {
462+
menu_item->SetSubmenu(submenu_ptr);
463+
}
464+
} else {
465+
menu_item->SetSubmenu(nullptr);
468466
}
469467
} catch (...) {
470468
// Ignore exceptions
@@ -484,17 +482,7 @@ native_menu_t native_menu_item_get_submenu(native_menu_item_t item) {
484482
}
485483
}
486484

487-
void native_menu_item_remove_submenu(native_menu_item_t item) {
488-
if (!item)
489-
return;
490485

491-
try {
492-
auto menu_item = static_cast<MenuItem*>(item);
493-
menu_item->RemoveSubmenu();
494-
} catch (...) {
495-
// Ignore exceptions
496-
}
497-
}
498486

499487
// New event listener API implementation
500488
int native_menu_item_add_listener(native_menu_item_t item,

‎src/capi/menu_c.h‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ char* native_menu_item_get_tooltip(native_menu_item_t item);
231231
/**
232232
* Set the keyboard accelerator for a menu item
233233
* @param item The menu item
234-
* @param accelerator The keyboard accelerator to set
234+
* @param accelerator The keyboard accelerator to set, or NULL to remove the accelerator
235235
*/
236236
FFI_PLUGIN_EXPORT
237237
void native_menu_item_set_accelerator(native_menu_item_t item,
@@ -247,12 +247,7 @@ FFI_PLUGIN_EXPORT
247247
bool native_menu_item_get_accelerator(native_menu_item_t item,
248248
native_keyboard_accelerator_t* accelerator);
249249

250-
/**
251-
* Remove the keyboard accelerator from a menu item
252-
* @param item The menu item
253-
*/
254-
FFI_PLUGIN_EXPORT
255-
void native_menu_item_remove_accelerator(native_menu_item_t item);
250+
256251

257252
/**
258253
* Set the enabled state of a menu item
@@ -305,7 +300,7 @@ int native_menu_item_get_radio_group(native_menu_item_t item);
305300
/**
306301
* Set the submenu for a menu item
307302
* @param item The menu item
308-
* @param submenu The submenu to attach
303+
* @param submenu The submenu to attach, or NULL to remove the submenu
309304
*/
310305
FFI_PLUGIN_EXPORT
311306
void native_menu_item_set_submenu(native_menu_item_t item, native_menu_t submenu);
@@ -318,12 +313,7 @@ void native_menu_item_set_submenu(native_menu_item_t item, native_menu_t submenu
318313
FFI_PLUGIN_EXPORT
319314
native_menu_t native_menu_item_get_submenu(native_menu_item_t item);
320315

321-
/**
322-
* Remove the submenu from a menu item
323-
* @param item The menu item
324-
*/
325-
FFI_PLUGIN_EXPORT
326-
void native_menu_item_remove_submenu(native_menu_item_t item);
316+
327317

328318
/**
329319
* Add event listener for a menu item

‎src/menu.h‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ class Menu;
170170
*
171171
* // Create a checkbox item
172172
* auto checkbox = std::make_shared<MenuItem>("Show Toolbar", MenuItemType::Checkbox);
173-
* checkbox->SetChecked(true);
173+
* checkbox->SetState(MenuItemState::Checked);
174174
* checkbox->AddListener<MenuItemClickedEvent>([](const MenuItemClickedEvent&
175175
* event) { std::cout << "Toolbar clicked, handle state change manually" <<
176176
* std::endl;
@@ -315,7 +315,7 @@ class MenuItem : public EventEmitter<MenuEvent>, public NativeObjectProvider {
315315
* keyboard shortcuts. The accelerator is typically displayed
316316
* next to the menu item label.
317317
*
318-
* @param accelerator The keyboard accelerator to set
318+
* @param accelerator The keyboard accelerator to set, or std::nullopt to remove
319319
*
320320
* @example
321321
* ```cpp
@@ -327,9 +327,12 @@ class MenuItem : public EventEmitter<MenuEvent>, public NativeObjectProvider {
327327
*
328328
* // Set Alt+F4 as accelerator
329329
* item->SetAccelerator(KeyboardAccelerator("F4", KeyboardAccelerator::Alt));
330+
*
331+
* // Remove accelerator
332+
* item->SetAccelerator(std::nullopt);
330333
* ```
331334
*/
332-
void SetAccelerator(const KeyboardAccelerator& accelerator);
335+
void SetAccelerator(const std::optional<KeyboardAccelerator>& accelerator);
333336

334337
/**
335338
* @brief Get the current keyboard accelerator of the menu item.
@@ -339,11 +342,6 @@ class MenuItem : public EventEmitter<MenuEvent>, public NativeObjectProvider {
339342
*/
340343
KeyboardAccelerator GetAccelerator() const;
341344

342-
/**
343-
* @brief Remove the keyboard accelerator from the menu item.
344-
*/
345-
void RemoveAccelerator();
346-
347345
/**
348346
* @brief Enable or disable the menu item.
349347
*
@@ -363,7 +361,7 @@ class MenuItem : public EventEmitter<MenuEvent>, public NativeObjectProvider {
363361
/**
364362
* @brief Set the state of a checkbox or radio menu item.
365363
*
366-
* This method provides more control than SetChecked, allowing
364+
* This method allows you to set the checked state, including
367365
* you to set mixed/indeterminate state for checkboxes.
368366
* For radio items, only Unchecked and Checked states are valid.
369367
*
@@ -412,11 +410,6 @@ class MenuItem : public EventEmitter<MenuEvent>, public NativeObjectProvider {
412410
*/
413411
std::shared_ptr<Menu> GetSubmenu() const;
414412

415-
/**
416-
* @brief Remove the submenu from this menu item.
417-
*/
418-
void RemoveSubmenu();
419-
420413
protected:
421414
/**
422415
* @brief Internal method to get the platform-specific native menu item object.

‎src/platform/linux/menu_linux.cpp‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -180,20 +180,19 @@ std::optional<std::string> MenuItem::GetTooltip() const {
180180
return pimpl_->tooltip_;
181181
}
182182

183-
void MenuItem::SetAccelerator(const KeyboardAccelerator& accelerator) {
184-
pimpl_->accelerator_ = accelerator;
183+
void MenuItem::SetAccelerator(const std::optional<KeyboardAccelerator>& accelerator) {
184+
if (accelerator.has_value()) {
185+
pimpl_->accelerator_ = *accelerator;
186+
} else {
187+
pimpl_->accelerator_ = KeyboardAccelerator("", KeyboardAccelerator::None);
188+
}
185189
// TODO: Implement GTK accelerator setting
186190
}
187191

188192
KeyboardAccelerator MenuItem::GetAccelerator() const {
189193
return pimpl_->accelerator_;
190194
}
191195

192-
void MenuItem::RemoveAccelerator() {
193-
pimpl_->accelerator_ = KeyboardAccelerator("", KeyboardAccelerator::None);
194-
// TODO: Implement GTK accelerator removal
195-
}
196-
197196
void MenuItem::SetEnabled(bool enabled) {
198197
if (pimpl_->gtk_menu_item_) {
199198
gtk_widget_set_sensitive(pimpl_->gtk_menu_item_, enabled ? TRUE : FALSE);
@@ -212,15 +211,31 @@ void MenuItem::SetState(MenuItemState state) {
212211
if (pimpl_->gtk_menu_item_) {
213212
if (pimpl_->type_ == MenuItemType::Checkbox) {
214213
gboolean active = (state == MenuItemState::Checked) ? TRUE : FALSE;
214+
// Block the "activate" signal to prevent recursive triggering
215+
g_signal_handlers_block_by_func(G_OBJECT(pimpl_->gtk_menu_item_),
216+
(gpointer)OnGtkMenuItemActivate, this);
215217
gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(pimpl_->gtk_menu_item_), active);
218+
g_signal_handlers_unblock_by_func(G_OBJECT(pimpl_->gtk_menu_item_),
219+
(gpointer)OnGtkMenuItemActivate, this);
216220
} else if (pimpl_->type_ == MenuItemType::Radio) {
217221
gboolean active = (state == MenuItemState::Checked) ? TRUE : FALSE;
222+
// Block the "activate" signal to prevent recursive triggering
223+
g_signal_handlers_block_by_func(G_OBJECT(pimpl_->gtk_menu_item_),
224+
(gpointer)OnGtkMenuItemActivate, this);
218225
gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(pimpl_->gtk_menu_item_), active);
226+
g_signal_handlers_unblock_by_func(G_OBJECT(pimpl_->gtk_menu_item_),
227+
(gpointer)OnGtkMenuItemActivate, this);
219228
}
220229
}
221230
}
222231

223232
MenuItemState MenuItem::GetState() const {
233+
// For checkbox and radio items, get the actual state from GTK widget
234+
if (pimpl_->gtk_menu_item_ &&
235+
(pimpl_->type_ == MenuItemType::Checkbox || pimpl_->type_ == MenuItemType::Radio)) {
236+
gboolean active = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(pimpl_->gtk_menu_item_));
237+
return active ? MenuItemState::Checked : MenuItemState::Unchecked;
238+
}
224239
return pimpl_->state_;
225240
}
226241

@@ -253,13 +268,6 @@ std::shared_ptr<Menu> MenuItem::GetSubmenu() const {
253268
return pimpl_->submenu_;
254269
}
255270

256-
void MenuItem::RemoveSubmenu() {
257-
pimpl_->submenu_.reset();
258-
if (pimpl_->gtk_menu_item_) {
259-
gtk_menu_item_set_submenu(GTK_MENU_ITEM(pimpl_->gtk_menu_item_), nullptr);
260-
}
261-
}
262-
263271
void* MenuItem::GetNativeObjectInternal() const {
264272
return (void*)pimpl_->gtk_menu_item_;
265273
}

0 commit comments

Comments
 (0)