I am unable to review the patch right now, but it no longer opens with the menu key. I also don't understand the Gtk push to remove icons in menus which aid in quickly finding important items.

The menu key is now working.

Icons can be specified in the menu model, I put one in for a test, but the icon is not shown, at least in my system.

Do the accelerators work for you? I don't think control keys can be shifted so ^S either does nothing or does the same as ^s which syncs. Same with most of the other accelerators.

I like the popover because it doesn't grab the keyboard, allowing focus to change to another window without closing the popover. It also doesn't appear to display a blank menu and require scrolling to see any items like the normal context menus do near the edge of screen. The lack of icons is the biggest issue, and while the popover looks nice, and might fit in with CSD, it is a different menu from those those in the menu bar for non-CSD. However, that might be acceptable due to the other issues they solve.

As for the icons, when using GTK_DEBUG=interactive I can set the visible and icon-name properties of the GtkImage before each GtkLabel and it does display an icon. It looks odd and is hard to read because items without icons don't shift over. https://github.com/GNOME/gtk/blob/main/gtk/gtkpopovermenu.c mentions 'icon' and 'verb-icon' attributes, and a 'display-hint' attribute at top of section will use 'verb-icon' attributes to convert a section into an icon button bar (only the 'horizontal-buttons` value works for me). There might be another section attribute that displays the 'icon' attribute and aligns each label for items without icons.

Do the accelerators work for you?

Accelerators set in menus.ui do not work, but they can be attached to actions with gtk_applicaton_set_accels_for_action.

The accelerator keys are only shown in GTK4, and there seems to be little hope of having them in GTK3. But, there are no accelerators shown in current context menus either, so this is not a regression.

I'm not personally missing the icons in context menus, but I suppose the visible property of the item icons could be set in code when creating the menu. The alignment could be fixed by setting the width-request property to the menu icon width for all images in ModelButtons. This would be a brittle hack though.

there are no accelerators shown in current context menus either, so this is not a regression.

I know the current context menus lack accelerators, I just meant the patch includes accelerators that don't work, or might collide with existing accelerators if they were made to work (unless control-shift is possible).

This would be a brittle hack though.

We don't want hacks. Sadly I learned the left margin in the menubar menus is only for radio/check buttons, and adding icons does exactly what the popover does, except it actually works without enabled the visible property. I was really hoping to get a couple icons in the Episodes menu after dynamically generating it. :(

What do you think about adding a horizontal strip of icon buttons for the most common actions at the top of context menus? The named actions could still exist in the menu, they would just have a secondary way to access them via icon at top. This might also be possible in the Episodes menu.

Added a horizontal button row with common actions and removed the accelerators from the menu definition.

I'm not sure if repeating actions in a menu is good UI design, but at least the common actions are now easy to see.

Ok, I think this is ready for review now. Rebased to master, now converts the channel, episode and download list context menus to Actions and Popovers.

Just tested it: popovers on list right-click on desktop feel strange to me. I understand they are better suited for touch.
There also seems to be a weird delay while the popover opens.

The latest commit removes the transitions which cause at least some of the delay. The looks are a matter of taste of course.

Just by using GMenu models and Actions would get me 90% there in terms of having the menu definitions to be the same in master and adaptive. This is prototyped in #1305.

Thanks @tpikonen: now the transition delay is gone. Indeed I'd be happier with #1305. Sorry I missed it 2 months ago.

@elelay Other than looking different, the popovers seem superior. When the window is placed at bottom of screen, and a context menu is opened using menu key for a channel/episode at bottom of window, the menu is completely blank with scroll buttons. Popovers don't have that issue, and they are also part of the window meaning they can be captured in screenshots and don't grab the keyboard when cursor is outside of the window.

The commit stack needs serious refactoring, so setting to draft again.

Fixes from review are now mostly included in the patches. Ready for another round.

Is it not possible to create the verb-icon menu items from code?

Is it not possible to create the verb-icon menu items from code?

If you're after icons in front of the menu labels, I think the only way is to set the icon visibility of the ModelButtons to true in code. The section display-hint property values all produce a horizontal row of icon buttons.

Of course the content of a Popover can also be constructed by hand just as any other widget tree.

The bug in extension callbacks was a pretty subtle one, but it should be fixed now. I also integrated your patches and rebased to master, so it should be ready for testing (which I did not do much, except for the extension callbacks).

The patch stack is a mess and needs to be squashed before merging.

This patch disables the toolbar download button when the progress list is empty.

diff --git a/src/gpodder/gtkui/main.py b/src/gpodder/gtkui/main.py
index 8aa13f4e..76582aef 100644
--- a/src/gpodder/gtkui/main.py
+++ b/src/gpodder/gtkui/main.py
@@ -2406,6 +2406,8 @@ class gPodder(BuilderWidget, dbus.service.Object):
                     can_pause = can_pause or task.can_pause()
                     can_cancel = can_cancel or task.can_cancel()
                     can_remove = can_remove or task.can_remove()
+            else:
+                can_force = False
 
             self.set_episode_actions(False, False, can_queue or can_force, can_pause, can_cancel, can_remove, False, False)
 

The toolbar download button is not disabled for queued downloads, but does nothing when clicked. This patch adds a toolbar force download button.

diff --git a/share/gpodder/ui/gtk/gpodder.ui b/share/gpodder/ui/gtk/gpodder.ui
index 9977f655..50f65217 100644
--- a/share/gpodder/ui/gtk/gpodder.ui
+++ b/share/gpodder/ui/gtk/gpodder.ui
@@ -44,6 +44,21 @@
                 <property name="homogeneous">True</property>
               </packing>
             </child>
+            <child>
+              <object class="GtkToolButton" id="toolForceDownload">
+                <property name="visible">False</property>
+                <property name="sensitive">False</property>
+                <property name="can-focus">False</property>
+                <property name="is-important">True</property>
+                <property name="label" translatable="yes">Start download now</property>
+                <property name="icon-name">document-save-symbolic</property>
+                <signal name="clicked" handler="on_force_download_selected_episodes" swapped="no"/>
+              </object>
+              <packing>
+                <property name="expand">False</property>
+                <property name="homogeneous">True</property>
+              </packing>
+            </child>
             <child>
               <object class="GtkToolButton" id="toolDownload">
                 <property name="visible">True</property>
diff --git a/src/gpodder/gtkui/main.py b/src/gpodder/gtkui/main.py
index 76582aef..4fdcd57e 100644
--- a/src/gpodder/gtkui/main.py
+++ b/src/gpodder/gtkui/main.py
@@ -2161,8 +2161,8 @@ class gPodder(BuilderWidget, dbus.service.Object):
             self.episodes_popover.show()
             return True
 
-    def set_episode_actions(self, open_instead_of_play=False, can_play=False, can_download=False, can_pause=False, can_cancel=False,
-                            can_delete=False, can_lock=False, is_episode_selected=False):
+    def set_episode_actions(self, open_instead_of_play=False, can_play=False, can_force=False, can_download=False,
+                            can_pause=False, can_cancel=False, can_delete=False, can_lock=False, is_episode_selected=False):
         episodes = self.get_selected_episodes() if is_episode_selected else []
 
         # play icon and label
@@ -2184,6 +2184,9 @@ class gPodder(BuilderWidget, dbus.service.Object):
 
         # toolbar
         self.toolPlay.set_sensitive(can_play)
+        self.toolForceDownload.set_visible(can_force)
+        self.toolForceDownload.set_sensitive(can_force)
+        self.toolDownload.set_visible(not can_force)
         self.toolDownload.set_sensitive(can_download)
         self.toolPause.set_sensitive(can_pause)
         self.toolCancel.set_sensitive(can_cancel)
@@ -2191,7 +2194,7 @@ class gPodder(BuilderWidget, dbus.service.Object):
         # Episodes menu
         self.play_action.set_enabled(can_play and not open_instead_of_play)
         self.open_action.set_enabled(can_play and open_instead_of_play)
-        self.download_action.set_enabled(can_download)
+        self.download_action.set_enabled(can_force or can_download)
         self.pause_action.set_enabled(can_pause)
         self.cancel_action.set_enabled(can_cancel)
         self.delete_action.set_enabled(can_delete)
@@ -2374,7 +2377,7 @@ class gPodder(BuilderWidget, dbus.service.Object):
                     can_delete = can_delete or episode.can_delete()
                     can_lock = can_lock or episode.can_lock()
 
-            self.set_episode_actions(open_instead_of_play, can_play, can_download, can_pause, can_cancel, can_delete, can_lock,
+            self.set_episode_actions(open_instead_of_play, can_play, False, can_download, can_pause, can_cancel, can_delete, can_lock,
                                     selection.count_selected_rows() > 0)
 
             return (open_instead_of_play, can_play, can_preview, can_download,
@@ -2409,7 +2412,7 @@ class gPodder(BuilderWidget, dbus.service.Object):
             else:
                 can_force = False
 
-            self.set_episode_actions(False, False, can_queue or can_force, can_pause, can_cancel, can_remove, False, False)
+            self.set_episode_actions(False, False, can_force, can_queue, can_pause, can_cancel, can_remove, False, False)
 
             return (False, False, False, can_queue, can_pause, can_cancel,
                     can_remove, False)

Other than that, the PR is ready to merge. I would like to fix #1538 and get out another release first, and then this could be merged.

I added the patches by @auouymous. Will refactor the patch stack soonish.

The commit stack is now cleaned up, so as far as I'm concerned, this is ready.

Thanks for all the work you did on this.

From me a big thank you, too

In the new context menu the buttons and the menu entries are redundant (see screenshot 1). Here are my ideas:

• Alternative 1: Remove the redundant text entries below the buttons and make those text appear only as a tooltip that appears when the mouse cursor hovers above the icon for a couple of seconds.
• Alternative 2: Merge text and buttons like in the new Windows 11's system's context menu (see screenshot 2) resulting in buttons with text below them.

Screenshot 1:

Screenshot 2:

Gtk removed icons for menu items, the buttons provide that icon and the textual menu items describe what each icon does. We couldn't find a better way to have both.

I see... still, the redundancy is, I think, confusing.

Maybe decide on one of the both things: buttons or text. But not two identical menu items at the same time. I'd vote for text only to keep the context menu consistent. Too bad that Gtk removed icons for menu items. It's a regression in my opinion.