Skip to content

Commit de4f40f

Browse files
author
tznind
committed
Revert changes to MoveMenuItemRightOperation
Test menu bar more
1 parent c73a29d commit de4f40f

4 files changed

Lines changed: 145 additions & 77 deletions

File tree

Showcase/Menus.Designer.cs

Lines changed: 116 additions & 27 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Showcase/Menus.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
// You can make changes to this file and they will not be overwritten when saving.
88
// </auto-generated>
99
// -----------------------------------------------------------------------------
10-
namespace Showcase{
10+
namespace YourNamespace {
1111
using Terminal.Gui;
1212

1313

src/Operations/MenuOperations/MoveMenuItemRightOperation.cs

Lines changed: 13 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -73,51 +73,39 @@ protected override bool DoImpl()
7373
return false;
7474
}
7575

76+
// When user hits shift right
7677
var children = this.Parent.GetMenuItems(out _);
7778
var currentItemIdx = children.IndexOf(this.OperateOn);
7879
var aboveIdx = currentItemIdx - 1;
7980

81+
// and there is an item above
8082
if (aboveIdx < 0)
8183
{
8284
return false;
8385
}
8486

87+
// Get or create a submenu on the item above
8588
var itemAbove = children[aboveIdx];
89+
if (itemAbove.SubMenu == null)
90+
{
91+
itemAbove.SubMenu = new Menu();
92+
}
8693

87-
// Remove us from current menu first
94+
// Remove us from current menu
8895
this.Parent.RemoveMenuItem(this.OperateOn);
8996

90-
if (itemAbove is MenuBarItem existingMbi)
97+
// Add us to the submenu of the item above
98+
if (this.InsertionIndex != null)
9199
{
92-
// Item above already has a sub-menu — add to it
93-
if (this.InsertionIndex != null)
94-
{
95-
existingMbi.InsertMenuItem(this.InsertionIndex.Value, this.OperateOn);
96-
}
97-
else
98-
{
99-
var subItems = existingMbi.GetMenuItems(out _);
100-
subItems.Add(this.OperateOn);
101-
existingMbi.SetMenuItems(subItems);
102-
}
100+
itemAbove.InsertMenuItem(this.InsertionIndex.Value, this.OperateOn);
103101
}
104102
else
105103
{
106-
// Convert plain MenuItem to MenuBarItem with OperateOn as first child.
107-
// Re-read children since RemoveMenuItem may have shifted indexes.
108-
children = this.Parent.GetMenuItems(out _);
109-
var newAboveIdx = children.IndexOf(itemAbove);
110-
111-
var newMbi = new MenuBarItem(itemAbove.Title, new MenuItem[] { this.OperateOn });
112-
newMbi.Data = itemAbove.Data;
113-
newMbi.Key = itemAbove.Key;
114-
115-
children.RemoveAt(newAboveIdx);
116-
children.Insert(newAboveIdx, newMbi);
117-
this.Parent.SetMenuItems(children);
104+
itemAbove.SubMenu.Add(this.OperateOn);
118105
}
119106

120107
this.Bar?.SetNeedsDraw();
108+
121109
return true;
122110
}
123111

tests/MenuBarTests.cs

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -293,30 +293,28 @@ public void MoveMenuItemRight_CannotMoveElementZero( )
293293
Assert.That( impossibleMoveRightOpSucceeded, Is.False );
294294

295295
// can move element 1
296-
// This is a destructive action, so references will change.
297296
MoveMenuItemRightOperation validMoveRightOp = new( App, fileMenu.GetMenuItems(out _)[1] );
298297
Assert.That( validMoveRightOp.IsImpossible, Is.False );
299298
bool validMoveRightOpSucceeded = false;
300299
Assert.That( ( ) => validMoveRightOpSucceeded = validMoveRightOp.Do( ), Throws.Nothing );
301300
Assert.That( validMoveRightOpSucceeded );
302301

303-
// We will have changed from a MenuItem to a MenuBarItem
304-
// so element 0 will not be us. In Terminal.Gui there is
305-
// a different class for a menu item and one with submenus.
306-
Assert.That( fileMenu.GetMenuItems(out _)[0], Is.Not.Null.And.InstanceOf<MenuBarItem>( ) );
307-
MenuBarItem miConvertedToMenuBarItem = (MenuBarItem)fileMenu.GetMenuItems(out _)[0];
302+
// In Terminal.Gui v2, nested items with sub-menus remain plain MenuItem instances
303+
// (with SubMenu set) rather than being converted to MenuBarItem. MenuBarItem is
304+
// exclusively for top-level MenuBar entries. The same object reference (mi) is
305+
// retained at index 0 — it is not replaced with a new object.
306+
Assert.That( fileMenu.GetMenuItems(out _), Has.Exactly( 1 ).InstanceOf<MenuItem>( ) );
307+
MenuItem miWithSubMenu = fileMenu.GetMenuItems(out _)[0];
308308

309-
// Check that the references are unequal but values are equal
310309
Assert.Multiple( ( ) =>
311310
{
312-
Assert.That( miConvertedToMenuBarItem, Is.Not.SameAs( mi ) );
313-
Assert.That( miConvertedToMenuBarItem.Title, Is.EqualTo( mi.Title ) );
314-
Assert.That( miConvertedToMenuBarItem.Data, Is.EqualTo( mi.Data ) );
315-
Assert.That( miConvertedToMenuBarItem.GetMenuItems(out _), Has.Exactly( 1 ).InstanceOf<MenuItem>( ) );
311+
Assert.That( miWithSubMenu, Is.SameAs( mi ) );
312+
Assert.That( miWithSubMenu.Title, Is.EqualTo( mi.Title ) );
313+
Assert.That( miWithSubMenu.Data, Is.EqualTo( mi.Data ) );
314+
Assert.That( miWithSubMenu.GetMenuItems(out _), Has.Exactly( 1 ).InstanceOf<MenuItem>( ) );
316315
} );
317316

318317
// Now undo it.
319-
// This is destructive as well.
320318
Assert.That( validMoveRightOp.Undo, Throws.Nothing );
321319

322320
Assert.That( fileMenu.GetMenuItems(out _), Has.Exactly( 2 ).InstanceOf<MenuItem>( ) );
@@ -326,23 +324,16 @@ public void MoveMenuItemRight_CannotMoveElementZero( )
326324
Assert.That( fileMenu.GetMenuItems(out _)[1], Is.Not.Null.And.InstanceOf<MenuItem>( ) );
327325
} );
328326
MenuItem firstChildAfterUndo = fileMenu.GetMenuItems(out _)[0];
329-
MenuItem secondChildAfterUndo = fileMenu.GetMenuItems(out _)[1];
330327

331328
Assert.Multiple( ( ) =>
332329
{
333-
// All the previous references are gone forever through this process.
334-
// So, neither element should be mi.
335-
Assert.That( firstChildAfterUndo, Is.Not.SameAs( mi ) );
336-
Assert.That( secondChildAfterUndo, Is.Not.SameAs( mi ) );
337-
338-
// Neither element should be miConvertedToMenuBarItem either
339-
Assert.That( firstChildAfterUndo, Is.Not.SameAs( miConvertedToMenuBarItem ) );
340-
Assert.That( secondChildAfterUndo, Is.Not.SameAs( miConvertedToMenuBarItem ) );
330+
// The same object reference is retained after undo.
331+
Assert.That( firstChildAfterUndo, Is.SameAs( mi ) );
341332

342-
// And mi still should not be miConvertedToMenuBarItem
343-
Assert.That( mi, Is.Not.SameAs( miConvertedToMenuBarItem ) );
333+
// The sub-menu should have been cleared after undo.
334+
Assert.That( firstChildAfterUndo.GetMenuItems(out _), Is.Empty );
344335

345-
// But the values need to be preserved
336+
// Values need to be preserved.
346337
Assert.That( firstChildAfterUndo.Title, Is.EqualTo( mi.Title ) );
347338
Assert.That( firstChildAfterUndo.Data, Is.EqualTo( mi.Data ) );
348339
Assert.That( firstChildAfterUndo.Key, Is.EqualTo( mi.Key) );

0 commit comments

Comments
 (0)