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.

Sign up for GitHub

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

Use GLib.MenuModel for context menu #769

Merged
merged 7 commits into from
Aug 9, 2024
Merged

Use GLib.MenuModel for context menu #769

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4aeae4a
Use GLib.Menu
jeremypw Aug 5, 2024
bc4d1e4
Merge branch 'master' into jeremypw/glib-context-menu
jeremypw Aug 5, 2024
2a93fae
Remove temporary label and add comment
jeremypw Aug 7, 2024
6eac613
Lose unnecessary properties
jeremypw Aug 7, 2024
52935ab
Merge branch 'master' into jeremypw/glib-context-menu
jeremypw Aug 7, 2024
6cff018
Merge master
danirabbit Aug 8, 2024
2c40905
Use Gtk.Menu (#770)
danirabbit Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 61 additions & 45 deletions src/MainWindow.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace Terminal {
private Gtk.Stack title_stack;
private Gtk.ToggleButton search_button;
private Dialogs.ColorPreferences? color_preferences_dialog;
private Granite.AccelLabel open_in_browser_menuitem_label;
private MenuItem open_in_browser_menuitem;

private bool is_fullscreen {
get {
Expand Down Expand Up @@ -57,7 +57,7 @@ namespace Terminal {
public bool unsafe_ignored;
public bool focus_restored_tabs { get; construct; default = true; }
public bool recreate_tabs { get; construct; default = true; }
public Gtk.Menu menu { get; private set; }
public Menu context_menu_model { get; private set; }
public Terminal.Application app { get; construct; }
public SimpleActionGroup actions { get; construct; }

Expand All @@ -80,9 +80,11 @@ namespace Terminal {
public const string ACTION_MOVE_TAB_LEFT = "action-move-tab-left";
public const string ACTION_MOVE_TAB_TO_NEW_WINDOW = "action-move-tab-to-new-window";
public const string ACTION_SEARCH = "action-search";
public const string ACTION_SEARCH_ACCEL = "<Control><Shift>f";
public const string ACTION_SEARCH_NEXT = "action-search-next";
public const string ACTION_SEARCH_PREVIOUS = "action-search-previous";
public const string ACTION_OPEN_IN_BROWSER = "action-open-in-browser";
public const string ACTION_OPEN_IN_BROWSER_ACCEL = "<Control><Shift>e";


private static Gee.MultiMap<string, string> action_accelerators = new Gee.HashMultiMap<string, string> ();
Expand Down Expand Up @@ -128,8 +130,8 @@ namespace Terminal {
action_accelerators[ACTION_PREVIOUS_TAB] = "<Control>Page_Up";
action_accelerators[ACTION_MOVE_TAB_RIGHT] = "<Control><Alt>Right";
action_accelerators[ACTION_MOVE_TAB_LEFT] = "<Control><Alt>Left";
action_accelerators[ACTION_SEARCH] = "<Control><Shift>f";
action_accelerators[ACTION_OPEN_IN_BROWSER] = "<Control><Shift>e";
action_accelerators[ACTION_SEARCH] = ACTION_SEARCH_ACCEL;
action_accelerators[ACTION_OPEN_IN_BROWSER] = ACTION_OPEN_IN_BROWSER_ACCEL;
}

construct {
Expand All @@ -154,53 +156,57 @@ namespace Terminal {
clipboard = Gtk.Clipboard.get (Gdk.Atom.intern ("CLIPBOARD", false));
primary_selection = Gtk.Clipboard.get (Gdk.Atom.intern ("PRIMARY", false));

var open_in_browser_menuitem = new Gtk.MenuItem () {
action_name = ACTION_PREFIX + ACTION_OPEN_IN_BROWSER
};
open_in_browser_menuitem_label = new Granite.AccelLabel.from_action_name (
"", open_in_browser_menuitem.action_name
//Window actions
open_in_browser_menuitem = new MenuItem (
"DUMMY open in name",
danirabbit marked this conversation as resolved.
Show resolved Hide resolved
ACTION_PREFIX + ACTION_OPEN_IN_BROWSER
);
open_in_browser_menuitem.add (open_in_browser_menuitem_label);
open_in_browser_menuitem.set_attribute_value ("accel", ACTION_OPEN_IN_BROWSER_ACCEL);

var copy_menuitem = new Gtk.MenuItem () {
action_name = TerminalWidget.ACTION_COPY
};
copy_menuitem.add (new Granite.AccelLabel.from_action_name (_("Copy"), TerminalWidget.ACTION_COPY));
var search_menuitem = new MenuItem (
_("Show Search Bar"),
ACTION_PREFIX + ACTION_SEARCH
);
search_menuitem.set_attribute_value ("accel", ACTION_SEARCH_ACCEL);

var copy_last_output_menuitem = new Gtk.MenuItem () {
action_name = TerminalWidget.ACTION_COPY_OUTPUT
};
copy_last_output_menuitem.add (
new Granite.AccelLabel.from_action_name (_("Copy Last Output"), TerminalWidget.ACTION_COPY_OUTPUT)
//TerminalWidget actions
var copy_menuitem = new MenuItem (
_("Copy"),
TerminalWidget.ACTION_COPY
);
copy_menuitem.set_attribute_value ("accel", new Variant ("s", TerminalWidget.ACCELS_COPY[0]));

var paste_menuitem = new Gtk.MenuItem () {
action_name = TerminalWidget.ACTION_PASTE
};
paste_menuitem.add (new Granite.AccelLabel.from_action_name (_("Paste"), TerminalWidget.ACTION_PASTE));
var copy_last_output_menuitem = new MenuItem (
_("Copy Last Output"),
TerminalWidget.ACTION_COPY_OUTPUT
);
copy_last_output_menuitem.set_attribute_value ("accel", new Variant ("s", TerminalWidget.ACCELS_COPY_OUTPUT[0]));

var select_all_menuitem = new Gtk.MenuItem () {
action_name = TerminalWidget.ACTION_SELECT_ALL
};
select_all_menuitem.add (
new Granite.AccelLabel.from_action_name (_("Select All"), TerminalWidget.ACTION_SELECT_ALL)
var paste_menuitem = new MenuItem (
_("Paste"),
TerminalWidget.ACTION_PASTE
);
paste_menuitem.set_attribute_value ("accel", new Variant ("s", TerminalWidget.ACCELS_PASTE[0]));

var search_menuitem = new Gtk.MenuItem () {
action_name = ACTION_PREFIX + ACTION_SEARCH
};
search_menuitem.add (new Granite.AccelLabel.from_action_name (_("Find…"), search_menuitem.action_name));

menu = new Gtk.Menu ();
menu.append (open_in_browser_menuitem);
menu.append (new Gtk.SeparatorMenuItem ());
menu.append (copy_menuitem);
menu.append (copy_last_output_menuitem);
menu.append (paste_menuitem);
menu.append (select_all_menuitem);
menu.append (new Gtk.SeparatorMenuItem ());
menu.append (search_menuitem);
menu.insert_action_group ("win", actions);
var select_all_menuitem = new MenuItem (
_("Select All"),
TerminalWidget.ACTION_SELECT_ALL
);
select_all_menuitem.set_attribute_value ("accel", new Variant ("s", TerminalWidget.ACCELS_SELECT_ALL[0]));

context_menu_model = new Menu ();
// "Open in" item must be in position 0 (see update_menu_label ())
context_menu_model.append_item (open_in_browser_menuitem);

var terminal_action_section = new Menu ();
terminal_action_section.append_item (copy_menuitem);
terminal_action_section.append_item (copy_last_output_menuitem);
terminal_action_section.append_item (paste_menuitem);
terminal_action_section.append_item (select_all_menuitem);
context_menu_model.append_section ("", terminal_action_section);
var search_section = new Menu ();
search_section.append_item (search_menuitem);
context_menu_model.append_section ("", search_section);

setup_ui ();

Expand Down Expand Up @@ -533,10 +539,20 @@ namespace Terminal {
private void update_menu_label (string? uri) {
AppInfo? appinfo = get_default_app_for_uri (uri);

//Changing atributes has no effect after adding item to menu so remove and re-add.
context_menu_model.remove (0); // This item was added first
get_simple_action (ACTION_OPEN_IN_BROWSER).set_enabled (appinfo != null);
open_in_browser_menuitem_label.label = _("Show in %s").printf (
appinfo != null ? appinfo.get_display_name () : _("Default application")
var new_name = _("Show in %s").printf (
appinfo != null ?
appinfo.get_display_name () : _("Default application")
);

open_in_browser_menuitem.set_attribute_value (
"label",
new Variant ("s", new_name)
);

context_menu_model.prepend_item (open_in_browser_menuitem);
}

private AppInfo? get_default_app_for_uri (string? uri) {
Expand Down
36 changes: 21 additions & 15 deletions src/Widgets/TerminalWidget.vala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,13 @@ namespace Terminal {
}
}

private Gtk.Menu menu {
private Menu context_menu_model {
danirabbit marked this conversation as resolved.
Show resolved Hide resolved
get {
return main_window.menu;
return main_window.context_menu_model;
}
}

private Gtk.PopoverMenu context_popover;
danirabbit marked this conversation as resolved.
Show resolved Hide resolved
// There may be no associated tab while made restorable
public unowned Hdy.TabPage tab;
public string? link_uri;
Expand Down Expand Up @@ -314,12 +315,7 @@ namespace Terminal {
copy_action.set_enabled (true);
}

Gdk.Rectangle rect = { (int) x, (int) y };
main_window.update_context_menu ();
setup_menu ();

menu.popup_at_rect (get_window (), rect, SOUTH_WEST, NORTH_WEST);
menu.select_first (false);
popup_context_menu ({ (int) x, (int) y });

gesture.set_state (CLAIMED);
}
Expand Down Expand Up @@ -387,12 +383,7 @@ namespace Terminal {
(int) cell_height
};

main_window.update_context_menu ();
setup_menu ();

// Popup context menu below cursor position
menu.popup_at_rect (get_window (), rect, SOUTH_WEST, NORTH_WEST);
menu.select_first (false);
popup_context_menu (rect);
break;

case Gdk.Key.Alt_L:
Expand Down Expand Up @@ -460,11 +451,26 @@ namespace Terminal {
paste_action.set_enabled (can_paste);
});

// Update the "Copy Last Outptut" menu option
// Update the "Copy Last Output" menu option
var has_output = !resized && get_last_output ().length > 0;
copy_output_action.set_enabled (has_output);
}

private void popup_context_menu (Gdk.Rectangle rect) {
main_window.update_context_menu ();
setup_menu ();

// Popup context menu below cursor position
context_popover = new Gtk.PopoverMenu () {
relative_to = this,
pointing_to = rect
};

context_popover.bind_model (context_menu_model, null);
context_popover.show_all ();
context_popover.popup ();
Copy link
Member

Choose a reason for hiding this comment

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

I think show_all and popup are redundant here. I'm not sure show_all is necessary when using a menu from a model

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It does not work in Gtk3 without these. Revisit in Gtk4

}

protected override void copy_clipboard () {
if (link_uri != null && !get_has_selection ()) {
clipboard.set_text (link_uri, link_uri.length);
Expand Down