Visually align shortcuts in menus

In a menu, we use the right side both for submenu arrows and shortcuts.
As a result, when an entry has both a shortcut and a submenu, its
shortcut is not aligned with others, and this does not look so nice.

The spacing for the arrow appears only if there is a submenu in any of
the items in the parent menu.

Change-Id: If91fdcdad36abb0141fb05d1f59141f89540c1db
Reviewed-on: https://review.haiku-os.org/c/haiku/+/355
Reviewed-by: Adrien Destugues <[email protected]>
Reviewed-by: Ryan Leavengood <[email protected]>

headers/os/interface/Menu.h

@@ -313,7 +313,7 @@ private:
 			bool				fStickyMode;
 			bool				fIgnoreHidden;
 			bool				fTriggerEnabled;
-			bool				fRedrawAfterSticky;
+			bool				fHasSubmenus;
 			bool				fAttachAborted;
 };
 

headers/os/interface/MenuItem.h

@@ -91,7 +91,7 @@ private:
 			rgb_color			_HighColor();
 
 			void				_DrawMarkSymbol();
-			void				_DrawShortcutSymbol();
+			void				_DrawShortcutSymbol(bool);
 			void				_DrawSubmenuSymbol();
 			void				_DrawControlChar(char shortcut, BPoint where);
 

headers/private/interface/MenuPrivate.h

@@ -56,6 +56,7 @@ public:
 			void				SetSuperItem(BMenuItem* item);
 			void				InvokeItem(BMenuItem* item, bool now = false);
 			void				QuitTracking(bool thisMenuOnly = true);
+			bool				HasSubmenus() { return fMenu->fHasSubmenus; }
 
 	static	status_t			CreateBitmaps();
 	static	void				DeleteBitmaps();

src/kits/interface/Menu.cpp

@@ -251,7 +251,7 @@ BMenu::BMenu(const char* name, menu_layout layout)
 	fStickyMode(false),
 	fIgnoreHidden(true),
 	fTriggerEnabled(true),
-	fRedrawAfterSticky(false),
+	fHasSubmenus(false),
 	fAttachAborted(false)
 {
 	_InitData(NULL);
@@ -286,7 +286,7 @@ BMenu::BMenu(const char* name, float width, float height)
 	fStickyMode(false),
 	fIgnoreHidden(true),
 	fTriggerEnabled(true),
-	fRedrawAfterSticky(false),
+	fHasSubmenus(false),
 	fAttachAborted(false)
 {
 	_InitData(NULL);
@@ -321,7 +321,7 @@ BMenu::BMenu(BMessage* archive)
 	fStickyMode(false),
 	fIgnoreHidden(true),
 	fTriggerEnabled(true),
-	fRedrawAfterSticky(false),
+	fHasSubmenus(false),
 	fAttachAborted(false)
 {
 	_InitData(archive);
@@ -1109,7 +1109,7 @@ BMenu::AreTriggersEnabled() const
 bool
 BMenu::IsRedrawAfterSticky() const
 {
-	return fRedrawAfterSticky;
+	return false;
 }
 
 
@@ -1323,7 +1323,7 @@ BMenu::BMenu(BRect frame, const char* name, uint32 resizingMode, uint32 flags,
 	fStickyMode(false),
 	fIgnoreHidden(true),
 	fTriggerEnabled(true),
-	fRedrawAfterSticky(false),
+	fHasSubmenus(false),
 	fAttachAborted(false)
 {
 	_InitData(NULL);
@@ -2228,6 +2228,7 @@ BMenu::_ComputeColumnLayout(int32 index, bool bestFit, bool moveItems,
 	bool control = false;
 	bool shift = false;
 	bool option = false;
+	bool submenu = false;
 
 	if (index > 0)
 		frame = ItemAt(index - 1)->Frame();
@@ -2239,6 +2240,8 @@ BMenu::_ComputeColumnLayout(int32 index, bool bestFit, bool moveItems,
 	BFont font;
 	GetFont(&font);
 
+	// Loop over all items to set their top, bottom and left coordinates,
+	// all while computing the width of the menu
 	for (; index < fItems.CountItems(); index++) {
 		BMenuItem* item = ItemAt(index);
 
@@ -2267,12 +2270,13 @@ BMenu::_ComputeColumnLayout(int32 index, bool bestFit, bool moveItems,
 			+ fPad.bottom;
 
 		if (item->fSubmenu != NULL)
-			width += item->Frame().Height() / 2;
+			submenu = true;
 
 		frame.right = std::max(frame.right, width + fPad.left + fPad.right);
 		frame.bottom = item->fBounds.bottom;
 	}
 
+	// Compute the extra space needed for shortcuts and submenus
 	if (command) {
 		frame.right
 			+= BPrivate::MenuPrivate::MenuItemCommand()->Bounds().Width() + 1;
@@ -2289,10 +2293,17 @@ BMenu::_ComputeColumnLayout(int32 index, bool bestFit, bool moveItems,
 		frame.right
 			+= BPrivate::MenuPrivate::MenuItemShift()->Bounds().Width() + 1;
 	}
+	if (submenu) {
+		frame.right += ItemAt(0)->Frame().Height() / 2;
+		fHasSubmenus = true;
+	} else {
+		fHasSubmenus = false;
+	}
 
 	if (fMaxContentWidth > 0)
 		frame.right = std::min(frame.right, fMaxContentWidth);
 
+	// Finally update the "right" coordinate of all items
 	if (moveItems) {
 		for (int32 i = 0; i < fItems.CountItems(); i++)
 			ItemAt(i)->fBounds.right = frame.right;

src/kits/interface/MenuItem.cpp

@@ -469,13 +469,14 @@ BMenuItem::Draw()
 	DrawContent();
 
 	// draw extra symbols
-	const menu_layout layout = MenuPrivate(fSuper).Layout();
+	MenuPrivate privateAccessor(fSuper);
+	const menu_layout layout = privateAccessor.Layout();
 	if (layout == B_ITEMS_IN_COLUMN) {
 		if (IsMarked())
 			_DrawMarkSymbol();
 
 		if (fShortcutChar)
-			_DrawShortcutSymbol();
+			_DrawShortcutSymbol(privateAccessor.HasSubmenus());
 
 		if (Submenu() != NULL)
 			_DrawSubmenuSymbol();
@@ -745,15 +746,17 @@ BMenuItem::_DrawMarkSymbol()
 
 
 void
-BMenuItem::_DrawShortcutSymbol()
+BMenuItem::_DrawShortcutSymbol(bool submenus)
 {
 	BMenu* menu = fSuper;
 	BFont font;
 	menu->GetFont(&font);
 	BPoint where = ContentLocation();
+	// Start from the right and walk our way back
 	where.x = fBounds.right - font.Size();
 
-	if (fSubmenu != NULL)
+	// Leave space for the submenu arrow if any item in the menu has a submenu
+	if (submenus)
 		where.x -= fBounds.Height() / 2;
 
 	const float ascent = MenuPrivate(fSuper).Ascent();