diff mbox

Behavior of Menu key, next round

Message ID 43CBFE88.4060305@gmx.de
State New
Headers

Commit Message

Udo Richter Jan. 16, 2006, 8:14 p.m. UTC 
  Hi list,

After the release of 1.3.39, the menu key debate has cooled down again, 
and now I'll do another attempt on intuitive behavior. Comments are of 
course welcome, as this is just one possible solution of many.

I really like to close things like OSDTeletext using the menu key, using 
the new behavior. However, for example if I switch to some channel and 
directly want to start OSDTT again, I'm annoyed because I have to press 
menu twice, one time to dismiss the channel status, one time to open the 
menu.

My suggestion is to let the OSD decide about how the menu key behaves, 
so that some more informational OSDs can directly open the menu.

The attached patch does that by adding cOsdObject::menuKeyAction that 
can be set to one of the following values:

menuKeyAlwaysCloses: Close this OSD if the menu key is pressed, and 
don't open the main menu. This is set for VDR native menus.

menuKeyAlwaysOpens: Close this OSD and open the main menu always. This 
is set for status displays like channel display, volume control and 
replay 'cutmarks' control.

menuKeyConfigurable: For compatibility sake, this decides based on the 
"Menu button closes" setup option. This is the default for plugins, and 
is used for the audio track menu.

To summarize, with "Menu button closes" disabled, the patch should 
behave as 1.3.37. With "Menu button closes" enabled, behavior is as 
1.3.39, except for channel display, volume control and replay control. 
Plugins and patches that re-open the OSD quickly (like subtitles), or 
only show additional information, may set menuKeyAction to 
menuKeyAlwaysOpens.


Don't forget to rebuild your plugins after applying the attached patch!

Cheers,

Udo

Comments

Luca Olivetti Jan. 16, 2006, 10:48 p.m. UTC | #1 
En/na Udo Richter ha escrit:
> Hi list,
> 
> After the release of 1.3.39, the menu key debate has cooled down again,
> and now I'll do another attempt on intuitive behavior. Comments are of
> course welcome, as this is just one possible solution of many.

I still think is that the cause of the original problem is that cOsdMenu
  (and the plugins that use it) works one way (back=go to previous menu,
menu=closes the menu) while cOsdObject works the other way (back=closes
the menu, menu=opens the main menu).
The coherent thing is that inside a menu tree (be it vdr native or
coming from a plugin) back goes to the previous level of the tree and
menu closes the menu, while everywhere else menu opens the main menu.

Bye
Udo Richter Jan. 17, 2006, 1:52 a.m. UTC | #2 
Luca Olivetti wrote:
> The coherent thing is that inside a menu tree (be it vdr native or
> coming from a plugin) back goes to the previous level of the tree and
> menu closes the menu, while everywhere else menu opens the main menu.

But what if it looks like a menu, but is not? Like the audio track 
dialog: This is a generic cOsdObject, not a cMenu, though it looks the 
same. Consequently, you cannot close this 'menu' with the menu key.

Cheers,

Udo
Luca Olivetti Jan. 17, 2006, 8:51 a.m. UTC | #3 
En/na Udo Richter ha escrit:
> Luca Olivetti wrote:
>> The coherent thing is that inside a menu tree (be it vdr native or
                              ^^^^^^^^^^^^^^^^^^
>> coming from a plugin) back goes to the previous level of the tree and
>> menu closes the menu, while everywhere else menu opens the main menu.
> 
> But what if it looks like a menu, but is not? Like the audio track 
> dialog: This is a generic cOsdObject, not a cMenu, though it looks the 
> same. Consequently, you cannot close this 'menu' with the menu key.

Correct, it's a generic cOsdObject but it isn't "inside a menu tree".
Maybe I didn't explain myself: for "inside a menu tree" I mean it is a
submenu under the main menu (or under a submenu of the main menu, and so
on).

Bye
Udo Richter Jan. 17, 2006, 5:47 p.m. UTC | #4 
Luca Olivetti wrote:
> Correct, it's a generic cOsdObject but it isn't "inside a menu tree".
> Maybe I didn't explain myself: for "inside a menu tree" I mean it is a
> submenu under the main menu (or under a submenu of the main menu, and so
> on).

well... By calling the main menu action of a plugin - via
cRemote::CallPlugin or SVDRP PLUG command or key macro - a plugin can
open a menu that is not a sub-menu of the main menu, and the back key
will not lead to the main menu again... ;)

Cheers,

Udo
Luca Olivetti Jan. 17, 2006, 8:32 p.m. UTC | #5 
En/na Udo Richter ha escrit:
> Luca Olivetti wrote:
>> Correct, it's a generic cOsdObject but it isn't "inside a menu tree".
>> Maybe I didn't explain myself: for "inside a menu tree" I mean it is a
>> submenu under the main menu (or under a submenu of the main menu, and so
>> on).
> 
> well... By calling the main menu action of a plugin - via
> cRemote::CallPlugin or SVDRP PLUG command or key macro - a plugin can
> open a menu that is not a sub-menu of the main menu, and the back key
> will not lead to the main menu again... ;)

That should be no different that, e.g., pressing the Red key that in the
default configuration opens the recordings menu. The back key then leads
to the main menu. The fact that you jumped directly to a submenu doesn't
 change the fact that it *is* a submenu.

Bye
Udo Richter Jan. 17, 2006, 8:54 p.m. UTC | #6 
Luca Olivetti wrote:
> That should be no different that, e.g., pressing the Red key that in the
> default configuration opens the recordings menu. The back key then leads
> to the main menu. The fact that you jumped directly to a submenu doesn't
>  change the fact that it *is* a submenu.

I've double-checked: If you directly jump to a plugin main menu, the
back key doesn't lead to the main menu... ;)

And:
The red key opens the recording menu, and it behaves like a menu.
The green key from main menu opens the track list, and it doesn't behave
like a menu - though it seems to be a sub-menu of the main menu.

Anyway, my point is: From an user's point of view, the difference
between menu and not-menu is difficult, and from an usage habit point of
view, even more.

Cheers,

Udo
diff mbox

Patch

diff -au vdr-1.3.39-old/menu.c vdr-1.3.39/menu.c
--- vdr-1.3.39-old/menu.c	2006-01-15 19:35:23.000000000 +0100
+++ vdr-1.3.39/menu.c	2006-01-16 19:07:06.000000000 +0100
@@ -3021,6 +3021,7 @@ 
   timeout = Switched || Setup.TimeoutRequChInfo;
   channel = Channels.GetByNumber(Number);
   lastPresent = lastFollowing = NULL;
+  menuKeyAction = menuKeyAlwaysOpens;
   if (channel) {
      DisplayChannel();
      DisplayInfo();
@@ -3035,6 +3036,7 @@ 
   group = -1;
   number = 0;
   lastPresent = lastFollowing = NULL;
+  menuKeyAction = menuKeyAlwaysOpens;
   lastTime.Set();
   withInfo = Setup.ShowInfoOnChSwitch;
   displayChannel = Skins.Current()->DisplayChannel(withInfo);
@@ -3223,6 +3225,7 @@ 
 :cOsdObject(true)
 {
   currentDisplayVolume = this;
+  menuKeyAction = menuKeyAlwaysOpens;
   timeout.Set(cDevice::PrimaryDevice()->IsMute() ? MUTETIMEOUT : VOLUMETIMEOUT);
   displayVolume = Skins.Current()->DisplayVolume();
   Show();
@@ -3712,6 +3715,7 @@ 
   lastSpeed = -1;
   timeoutShow = 0;
   timeSearchActive = false;
+  menuKeyAction = menuKeyAlwaysOpens;
   marks.Load(fileName);
   cRecording Recording(fileName);
   cStatus::MsgReplaying(this, Recording.Name(), Recording.FileName(), true);
diff -au vdr-1.3.39-old/osdbase.c vdr-1.3.39/osdbase.c
--- vdr-1.3.39-old/osdbase.c	2006-01-08 12:40:02.000000000 +0100
+++ vdr-1.3.39/osdbase.c	2006-01-16 19:04:56.000000000 +0100
@@ -76,6 +76,7 @@ 
 cOsdMenu::cOsdMenu(const char *Title, int c0, int c1, int c2, int c3, int c4)
 {
   isMenu = true;
+  menuKeyAction = menuKeyAlwaysCloses;
   digit = 0;
   hasHotkeys = false;
   title = NULL;
diff -au vdr-1.3.39-old/osdbase.h vdr-1.3.39/osdbase.h
--- vdr-1.3.39-old/osdbase.h	2006-01-06 12:55:30.000000000 +0100
+++ vdr-1.3.39/osdbase.h	2006-01-16 19:14:25.000000000 +0100
@@ -66,16 +66,20 @@ 
   virtual eOSState ProcessKey(eKeys Key);
   };
 
+enum eMenuKeyBehavior { menuKeyAlwaysCloses, menuKeyConfigurable, menuKeyAlwaysOpens };
+
 class cOsdObject {
   friend class cOsdMenu;
 private:
   bool isMenu;
 protected:
   bool needsFastResponse;
+  eMenuKeyBehavior menuKeyAction;
 public:
-  cOsdObject(bool FastResponse = false) { isMenu = false; needsFastResponse = FastResponse; }
+  cOsdObject(bool FastResponse = false) { isMenu = false; needsFastResponse = FastResponse; menuKeyAction = menuKeyConfigurable; }
   virtual ~cOsdObject() {}
   bool NeedsFastResponse(void) { return needsFastResponse; }
+  eMenuKeyBehavior MenuKeyAction(void) { return menuKeyAction; }
   bool IsMenu(void) { return isMenu; }
   virtual void Show(void);
   virtual eOSState ProcessKey(eKeys Key) { return osUnknown; }
diff -au vdr-1.3.39-old/vdr.c vdr-1.3.39/vdr.c
--- vdr-1.3.39-old/vdr.c	2006-01-15 14:31:57.000000000 +0100
+++ vdr-1.3.39/vdr.c	2006-01-16 19:41:44.000000000 +0100
@@ -791,13 +791,12 @@ 
           // Menu control:
           case kMenu: {
                key = kNone; // nobody else needs to see this key
-               bool WasOpen = Interact != NULL;
-               bool WasMenu = Interact && Interact->IsMenu();
+               eMenuKeyBehavior MenuAction = (Interact) ? (Interact->MenuKeyAction()) : (menuKeyAlwaysOpens);
                if (Menu)
                   DELETE_MENU;
                else if (cControl::Control() && cOsd::IsOpen())
                   cControl::Control()->Hide();
-               if (!WasOpen || !WasMenu && !Setup.MenuButtonCloses)
+               if (MenuAction == menuKeyAlwaysOpens || MenuAction == menuKeyConfigurable && !Setup.MenuButtonCloses)
                   Menu = new cMenuMain;
                }
                break;