Author: stippi Date: 2011-06-07 14:46:00 +0200 (Tue, 07 Jun 2011) New Revision: 42004 Changeset: https://dev.haiku-os.org/changeset/42004 Ticket: https://dev.haiku-os.org/ticket/7182 Modified: haiku/trunk/headers/private/interface/MenuPrivate.h haiku/trunk/src/kits/interface/Menu.cpp haiku/trunk/src/kits/interface/MenuBar.cpp Log: Applied patch by Pete Goodeve from ticket #7182. It improves keyboard navigation/tracking of BMenus and BMenuBars, although many issues remain. Should not yet go into alpha, since there is one issue which I am not sure if it's not a regression. The issue is that invoking a menu item with Enter for the first time seems to have no effect, while invoking it subsequently works as expected. I don't know, yet, if that's a regression of this patch. In any case, it's better than before, thanks, Pete! Modified: haiku/trunk/headers/private/interface/MenuPrivate.h =================================================================== --- haiku/trunk/headers/private/interface/MenuPrivate.h 2011-06-07 00:35:31 UTC (rev 42003) +++ haiku/trunk/headers/private/interface/MenuPrivate.h 2011-06-07 12:46:00 UTC (rev 42004) @@ -1,5 +1,5 @@ /* - * Copyright 2006-2008, Haiku, Inc. + * Copyright 2006-2011, Haiku, Inc. * Distributed under the terms of the MIT License. * * Authors: @@ -14,6 +14,8 @@ enum menu_states { MENU_STATE_TRACKING = 0, MENU_STATE_TRACKING_SUBMENU = 1, + MENU_STATE_KEY_TO_SUBMENU = 2, + MENU_STATE_KEY_LEAVE_SUBMENU = 3, MENU_STATE_CLOSED = 5 }; Modified: haiku/trunk/src/kits/interface/Menu.cpp =================================================================== --- haiku/trunk/src/kits/interface/Menu.cpp 2011-06-07 00:35:31 UTC (rev 42003) +++ haiku/trunk/src/kits/interface/Menu.cpp 2011-06-07 12:46:00 UTC (rev 42004) @@ -1,5 +1,5 @@ /* - * Copyright 2001-2009, Haiku Inc. All rights reserved. + * Copyright 2001-2011, Haiku Inc. All rights reserved. * Distributed under the terms of the MIT license. * * Authors: @@ -477,6 +477,13 @@ break; case B_DOWN_ARROW: + { + BMenuBar* bar = dynamic_cast<BMenuBar*>(Supermenu()); + if (bar != NULL && fState == MENU_STATE_CLOSED) { + // tell MenuBar's _Track: + bar->fState = MENU_STATE_KEY_TO_SUBMENU; + } + } if (fLayout == B_ITEMS_IN_COLUMN) _SelectNextItem(fSelected, true); break; @@ -494,8 +501,10 @@ // another top level menu. BMessenger msgr(Supermenu()); msgr.SendMessage(Window()->CurrentMessage()); - } else - _QuitTracking(); + } else { + // tell _Track + fState = MENU_STATE_KEY_LEAVE_SUBMENU; + } } } break; @@ -505,10 +514,11 @@ _SelectNextItem(fSelected, true); else { if (fSelected && fSelected->Submenu()) { - fSelected->Submenu()->_SetStickyMode(true); + fSelected->Submenu()->_SetStickyMode(true); // fix me: this shouldn't be needed but dynamic menus // aren't getting it set correctly when keyboard // navigating, which aborts the attach + fState = MENU_STATE_KEY_TO_SUBMENU; _SelectItem(fSelected, true, true, true); } else if (dynamic_cast<BMenuBar*>(Supermenu())) { // if we have no submenu and we're an @@ -539,13 +549,20 @@ case B_ENTER: case B_SPACE: if (fSelected) { - _InvokeItem(fSelected); + // preserve for exit handling + fChosenItem = fSelected; _QuitTracking(false); } break; case B_ESCAPE: - _QuitTracking(); + _SelectItem(NULL); + if (fState == MENU_STATE_CLOSED && dynamic_cast<BMenuBar*>(Supermenu())) { + // Keyboard may show menu without tracking it + BMessenger msgr(Supermenu()); + msgr.SendMessage(Window()->CurrentMessage()); + } else + _QuitTracking(false); break; default: @@ -1580,8 +1597,8 @@ bigtime_t navigationAreaTime = 0; fState = MENU_STATE_TRACKING; - if (fSuper != NULL) - fSuper->fState = MENU_STATE_TRACKING_SUBMENU; + // we will use this for keyboard selection: + fChosenItem = NULL; BPoint location; uint32 buttons = 0; @@ -1608,11 +1625,15 @@ // The order of the checks is important // to be able to handle overlapping menus: // first we check if mouse is inside a submenu, - // then if the menu is inside this menu, + // then if the mouse is inside this menu, // then if it's over a super menu. bool overSub = _OverSubmenu(fSelected, screenLocation); item = _HitTestItems(location, B_ORIGIN); - if (overSub) { + if (overSub || fState == MENU_STATE_KEY_TO_SUBMENU) { + if (fState == MENU_STATE_TRACKING) { + // not if from R.Arrow + fState = MENU_STATE_TRACKING_SUBMENU; + } navAreaRectAbove = BRect(); navAreaRectBelow = BRect(); @@ -1633,7 +1654,19 @@ if (submenuAction == MENU_STATE_CLOSED) { item = submenuItem; fState = MENU_STATE_CLOSED; - } + } else if (submenuAction == MENU_STATE_KEY_LEAVE_SUBMENU) { + if (LockLooper()) { + BMenuItem *temp = fSelected; + // close the submenu: + _SelectItem(NULL); + // but reselect the item itself for user: + _SelectItem(temp, false); + UnlockLooper(); + } + // cancel key-nav state + fState = MENU_STATE_TRACKING; + } else + fState = MENU_STATE_TRACKING; if (!LockLooper()) break; } else if (item != NULL) { @@ -1641,11 +1674,14 @@ navAreaRectBelow, selectedTime, navigationAreaTime); if (!releasedOnce) releasedOnce = true; - } else if (_OverSuper(screenLocation)) { + } else if (_OverSuper(screenLocation) && fSuper->fState != MENU_STATE_KEY_TO_SUBMENU) { fState = MENU_STATE_TRACKING; UnlockLooper(); break; - } else { + } else if (fState == MENU_STATE_KEY_LEAVE_SUBMENU) { + UnlockLooper(); + break; + } else if (fSuper == NULL || fSuper->fState != MENU_STATE_KEY_TO_SUBMENU) { // Mouse pointer outside menu: // If there's no other submenu opened, // deselect the current selected item @@ -1676,7 +1712,7 @@ uint32 newButtons = buttons; // If user doesn't move the mouse, loop here, - // so we don't interfer with keyboard menu navigation + // so we don't interfere with keyboard menu navigation do { snooze(snoozeAmount); if (!LockLooper()) @@ -1684,7 +1720,8 @@ GetMouse(&newLocation, &newButtons, true); UnlockLooper(); } while (newLocation == location && newButtons == buttons - && !(item && item->Submenu() != NULL)); + && !(item && item->Submenu() != NULL) + && fState == MENU_STATE_TRACKING); if (newLocation != location || newButtons != buttons) { if (!releasedOnce && newButtons == 0 && buttons != 0) @@ -1700,12 +1737,19 @@ if (action != NULL) *action = fState; + + // keyboard Enter will set this + if (fChosenItem != NULL) + item = fChosenItem; + else if (fSelected == NULL) + // needed to cover (rare) mouse/ESC combination + item = NULL; if (fSelected != NULL && LockLooper()) { _SelectItem(NULL); UnlockLooper(); } - + // delete the menu window recycled for all the child menus _DeleteMenuWindow(); @@ -2829,7 +2873,6 @@ if (BMenuBar* menuBar = dynamic_cast<BMenuBar*>(this)) menuBar->_RestoreFocus(); - fChosenItem = NULL; fState = MENU_STATE_CLOSED; // Close the whole menu hierarchy Modified: haiku/trunk/src/kits/interface/MenuBar.cpp =================================================================== --- haiku/trunk/src/kits/interface/MenuBar.cpp 2011-06-07 00:35:31 UTC (rev 42003) +++ haiku/trunk/src/kits/interface/MenuBar.cpp 2011-06-07 12:46:00 UTC (rev 42004) @@ -566,7 +566,7 @@ if (window->Lock()) { if (startIndex != -1) { be_app->ObscureCursor(); - _SelectItem(ItemAt(startIndex), true, true); + _SelectItem(ItemAt(startIndex), true, false); } GetMouse(&where, &buttons); window->Unlock(); @@ -582,7 +582,8 @@ menuItem = ItemAt(0); else menuItem = _HitTestItems(where, B_ORIGIN); - if (_OverSubmenu(fSelected, ConvertToScreen(where))) { + if (_OverSubmenu(fSelected, ConvertToScreen(where)) + || fState == MENU_STATE_KEY_TO_SUBMENU) { // call _Track() from the selected sub-menu when the mouse cursor // is over its window BMenu* menu = fSelected->Submenu(); @@ -645,7 +646,7 @@ if (fState != MENU_STATE_CLOSED) { // If user doesn't move the mouse, loop here, - // so we don't interfer with keyboard menu navigation + // so we don't interfere with keyboard menu navigation BPoint newLocation = where; uint32 newButtons = buttons; do { @@ -654,7 +655,8 @@ break; GetMouse(&newLocation, &newButtons, true); UnlockLooper(); - } while (newLocation == where && newButtons == buttons); + } while (newLocation == where && newButtons == buttons + && fState == MENU_STATE_TRACKING); where = newLocation; buttons = newButtons;