Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GTK4 Prep] Replace menu items with menu model #1428

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

colinkiama
Copy link
Sponsor Member

@colinkiama colinkiama commented Apr 14, 2024

I replaced as much as I possibly could.

The SourceView requires GTK4. We'd have access to Gtk.TextView.extra_menu.

Also, the PopoverMenus used for the replaced Gtk.MenuItems currently don't show keyboard shortcuts. As far as I know, this should be fixed after moving over to GTK4.

Part of #1157.

@colinkiama colinkiama requested a review from a team April 14, 2024 09:02
Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tested this yet or looked into it too much, but just made a note about code style and detailed action names.

Since there's issues in Gtk3 with PopoverMenu, you can use Gtk.Menu with a model https://valadoc.org/gtk+-3.0/Gtk.Menu.Menu.from_model.html and then it's a pretty easy drop-in replacement during the GTK 4 port to change that

Since this is quite a huge branch I wonder if it might be better to do this in smaller steps, like just a branch to switch to using GLib.Actions I imagine is already a huge review in itself. It might help to get your work merged in faster if branches are more incremental

Comment on lines +31 to +36
var open_in_terminal_pane_item = new GLib.MenuItem (_("Open in Terminal Pane"),
MainWindow.ACTION_PREFIX
+ MainWindow.ACTION_OPEN_IN_TERMINAL
+ "::"
+ file.path);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style for wrapping should be like:

Suggested change
var open_in_terminal_pane_item = new GLib.MenuItem (_("Open in Terminal Pane"),
MainWindow.ACTION_PREFIX
+ MainWindow.ACTION_OPEN_IN_TERMINAL
+ "::"
+ file.path);
var open_in_terminal_pane_item = new GLib.MenuItem (
_("Open in Terminal Pane"),
MainWindow.ACTION_PREFIX
+ MainWindow.ACTION_OPEN_IN_TERMINAL
+ "::"
+ file.path
);

There's actually a function for creating detailed action names as well: https://valadoc.org/gio-2.0/GLib.Action.print_detailed_name.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants