[Gtk4] Support MenuItem images on GTK4 - #3441
Conversation
924149f to
9f101f4
Compare
8b1bbe3 to
fead2fd
Compare
fead2fd to
19c9796
Compare
There was a problem hiding this comment.
🟡 Changes recommended
HiDPI sizing, accessibility semantics, theme overriding, and integration coverage need correction.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds GTK4 menu-item images using custom popover rows while preserving alignment, selection, actions, and submenu behavior.
Changes:
- Adds custom image-capable PUSH and CHECK menu rows.
- Adds GTK4/JNI APIs for custom popover children and sizing.
- Re-enables the GTK4 image test.
File summaries
| File | Description |
|---|---|
Test_org_eclipse_swt_widgets_MenuItem.java |
Re-enables image API testing. |
MenuItem.java |
Implements custom image menu rows. |
Menu.java |
Injects, aligns, and synchronizes custom rows. |
Display.java |
Adds custom-row highlight styling. |
GTK4.java |
Declares required GTK4 APIs. |
OS.java |
Declares supporting GLib APIs. |
GTK.java |
Declares widget construction and CSS APIs. |
os.c |
Adds generated JNI implementations. |
os_stats.h |
Updates generated OS JNI statistics. |
gtk4.c |
Adds generated GTK4 JNI implementations. |
gtk4_stats.h |
Updates generated GTK4 JNI statistics. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
19c9796 to
5af0212
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Custom-row highlighting, CHECK accessibility state, and teardown state handling remain incomplete.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 3
- Review effort level: Balanced
5af0212 to
7174d06
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Image-bearing CHECK items regress native Space-key behavior by closing the menu.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 2
- Review effort level: Balanced
GtkModelButton (GTK4's native menu row) never shows both an icon and text, so menu items with images rendered without their icon. Work around this by embedding a custom icon+label(+accelerator) GtkButton into the popover via gtk_popover_menu_add_child() for PUSH and CHECK items. The custom row uses the "modelbutton" node name so the menu row rules apply to it, and announces itself as a menu item like GtkModelButton does. Its leading box and the native rows' indicator boxes share one GtkSizeGroup, so icons and the check marks of native CHECK rows sit in one column and all labels line up. CHECK rows carry a GtkCheckButton bound to the item's action in front of the icon, and while a menu has such a row the other custom rows keep that check column in front of their icons, so the icons line up too; CASCADE and RADIO items keep the native GtkModelButton. The custom rows take part in the popover's row selection sync through the "selected" state the theme paints, and hovering one closes a sibling's open submenu after a short delay, as GtkModelButton does. Injection is self-healing: GTK's "custom" ids are single-use, so a detached or rebuilt row is re-injected under a fresh id. Contributes to eclipse-platform#2511 Assisted-by: Anthropic Claude Code (claude-sonnet-5) Assisted-by: Anthropic Claude Code (claude-fable-5-1)
7174d06 to
befbf90
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The complex native row lifecycle lacks regression coverage beyond basic image property storage.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 1
- Review effort level: Balanced
|
Good enough state for initial support. Whatever problems are found from now on should be handled in separate issues/PRs |
GtkModelButton (GTK4's native menu row) never shows both an icon and
text, so menu items with images rendered without their icon. Work
around this by embedding a custom icon+label(+accelerator) GtkButton
into the popover via gtk_popover_menu_add_child() for PUSH and CHECK
items. The custom row uses the "modelbutton" node name so the menu row
rules apply to it, and announces itself as a menu item like
GtkModelButton does. Its leading box and the native rows'
indicator boxes share one GtkSizeGroup, so icons and the check marks
of native CHECK rows sit in one column and all labels line up. CHECK
rows carry a GtkCheckButton bound to the item's action in front of the
icon, and while a menu has such a row the other custom rows keep that
check column in front of their icons, so the icons line up too;
CASCADE and RADIO items keep the native GtkModelButton.
The custom rows take part in the popover's row selection sync through
the "selected" state the theme paints, and hovering one closes a
sibling's open submenu after a short delay, as GtkModelButton does.
Injection is self-healing: GTK's "custom" ids are single-use, so a
detached or rebuilt row is re-injected under a fresh id.
Contributes to #2511