Fixed: moving of menu items in the editor

Fixed: actions are executed immediately if the queue is empty
Fixed: SetSelectedItem can handle nil with raising an AV
Fixed: memory leak in animation action

Packages/X2CLMenuBarEditor.pas

@@ -49,6 +49,7 @@ type
   private
     FMenuBar:             TX2CustomMenuBar;
     FDesignerAttached:    Boolean;
+    FMoving:              Boolean;
 
     procedure SetMenuBar(const Value: TX2CustomMenuBar);
 
@@ -81,7 +82,7 @@ type
 implementation
 uses
   Contnrs,
-  SysUtils;
+  SysUtils, Dialogs;
 
 
 var
@@ -289,8 +290,8 @@ begin
     { Make sure the group is inserted in the correct position by searching
       for it's sibling group. Note: do NOT use Items[x] in a loop; TTreeView
       emulates this by using GetFirst/GetNext. }
-    if AGroup.Index > 0 then
+    if AGroup.Index < Pred(AGroup.Collection.Count) then
-      siblingGroup  := TX2MenuBarGroup(AGroup.Collection.Items[Pred(AGroup.Index)]);
+      siblingGroup  := TX2MenuBarGroup(AGroup.Collection.Items[Succ(AGroup.Index)]);
 
     if Assigned(siblingGroup) then
     begin
@@ -305,9 +306,9 @@ begin
     end;
 
     if Assigned(siblingNode) then
-      groupNode := tvMenu.Items.Add(siblingNode, '')
+      groupNode  := tvMenu.Items.AddNode(nil, siblingNode, '', nil, naInsert)
     else
-      groupNode := tvMenu.Items.AddFirst(nil, '');
+      groupNode := tvMenu.Items.Add(nil, '');
 
     groupNode.Data  := AGroup;
     UpdateNode(groupNode);
@@ -337,8 +338,8 @@ begin
     siblingNode   := nil;
 
     { See AddGroup }
-    if AItem.Index > 0 then
+    if AItem.Index < Pred(AItem.Collection.Count) then
-      siblingItem := TX2MenuBarItem(AItem.Collection.Items[Pred(AItem.Index)]);
+      siblingItem := TX2MenuBarItem(AItem.Collection.Items[Succ(AItem.Index)]);
 
     if Assigned(siblingItem) then
     begin
@@ -353,9 +354,9 @@ begin
     end;
 
     if Assigned(siblingNode) then
-      itemNode  := tvMenu.Items.Add(siblingNode, '')
+      itemNode  := tvMenu.Items.AddNode(nil, siblingNode, '', nil, naInsert)
     else
-      itemNode  := tvMenu.Items.AddChildFirst(ANode, '');
+      itemNode  := tvMenu.Items.AddChild(ANode, '');
 
     itemNode.Data := AItem;
     UpdateNode(itemNode);
@@ -451,6 +452,9 @@ var
   treeNode:     TTreeNode;
 
 begin
+  if FMoving then
+    Exit;
+
   treeNode  := nil;
 
   if AItem is TX2MenuBarGroup then
@@ -477,6 +481,9 @@ var
   treeNode:     TTreeNode;
 
 begin
+  if FMoving then
+    Exit;
+
   tvMenu.Items.BeginUpdate();
   try
     treeNode  := tvMenu.Items.GetFirstNode();
@@ -496,6 +503,9 @@ var
   treeNode:     TTreeNode;
 
 begin
+  if FMoving then
+    Exit;
+
   treeNode  := GetItemNode(AItem);
   if Assigned(treeNode) then
     tvMenu.Items.Delete(treeNode);
@@ -544,42 +554,50 @@ begin
   if selectedItem is TX2MenuBarItem then
     group := TX2MenuBarItem(selectedItem).Group;
 
-  if ADown then
+  FMoving := True;
-  begin
+  try
-    if selectedItem.Index < Pred(selectedItem.Collection.Count) then
+    if ADown then
     begin
-      selectedItem.Index  := Succ(selectedItem.Index);
+      if selectedItem.Index < Pred(selectedItem.Collection.Count) then
-      refresh             := True;
-    end else if Assigned(group) then
-    begin
-      { Move down to another group }
-      if group.Index < Pred(MenuBar.Groups.Count) then
       begin
-        selectedItem.Collection := MenuBar.Groups[Succ(group.Index)].Items;
+        selectedItem.Index  := Succ(selectedItem.Index);
-        refresh                 := True;
+        refresh             := True;
+      end else if Assigned(group) then
+      begin
+        { Move down to another group
+            The AddItem is triggered by moving between groups, no need
+            to add here. }
+        if group.Index < Pred(MenuBar.Groups.Count) then
+        begin
+          selectedItem.Collection := MenuBar.Groups[Succ(group.Index)].Items;
+          selectedItem.Index      := 0;
+          refresh                 := True;
+        end;
+      end;
+    end else
+    begin
+      if selectedItem.Index > 0 then
+      begin
+        selectedItem.Index  := Pred(selectedItem.Index);
+        refresh             := True;
+      end else if Assigned(group) then
+      begin
+        { Move up to another group }
+        if group.Index > 0 then
+        begin
+          selectedItem.Collection := MenuBar.Groups[Pred(group.Index)].Items;
+          refresh                 := True;
+        end;
       end;
     end;
-  end else
+  finally
-  begin
+    FMoving := False;
-    if selectedItem.Index > 0 then
-    begin
-      selectedItem.Index  := Pred(selectedItem.Index);
-      refresh             := True;
-    end else if Assigned(group) then
-    begin
-      { Move up to another group }
-      if group.Index > 0 then
-      begin
-        selectedItem.Collection := MenuBar.Groups[Pred(group.Index)].Items;
-        refresh                 := True;
-      end;
-    end;
-  end;
 
-  if refresh then
+    if refresh then
-  begin
+    begin
-    ItemDeleting(selectedItem);
+      ItemDeleting(selectedItem);
-    ItemAdded(selectedItem);
+      ItemAdded(selectedItem);
+    end;
   end;
 end;
 

Source/X2CLMenuBar.pas

@@ -21,10 +21,14 @@ uses
   Graphics,
   ImgList,
   Messages,
+  SysUtils,
   Types,
   Windows;
   
+
 type
+  EInvalidItem                = class(Exception);
+
   TX2MenuBarAnimationStyle    = (asNone, asSlide, asDissolve, asFade,
                                  asSlideFade, asCustom);
 
@@ -181,7 +185,7 @@ type
     :$ Abstract action class.
 
     :: Provides a base for menu bar actions which need to be performed
-    :: asynchronous and in sequence.
+    :: asynchronous and/or in sequence.
   }
   TX2CustomMenuBarAction = class(TObject)
   private
@@ -457,6 +461,8 @@ type
     procedure CMMouseLeave(var Msg: TMessage); message CM_MOUSELEAVE;
 
     procedure WMVScroll(var Msg: TWMVScroll); message WM_VSCROLL;
+//    procedure WMMouseWheel(var Message: TWMMouseWheel); message WM_MOUSEWHEEL;
+//    procedure CMMouseWheel(var Message: TCMMouseWheel); message CM_MOUSEWHEEL;
 
     procedure TestMousePos(); virtual;
     function GetMenuHeight(): Integer; virtual;
@@ -593,6 +599,8 @@ type
     property HideScrollbar;
     property Images;
     property ParentFont;
+    property TabOrder;
+    property TabStop default True;
     property OnClick;
     property OnCollapsed;
     property OnCollapsing;
@@ -628,16 +636,17 @@ const
 
 implementation
 uses
-  SysUtils,
-
   X2CLGraphics,
   X2CLMenuBarActions,
   X2CLMenuBarAnimators;
 
+
 const
   SDefaultItemCaption   = 'Menu Item';
   SDefaultGroupCaption  = 'Group';
   SNoPainter            = 'Painter property not set';
+  SInvalidItem          = 'Item does not belong to this MenuBar';
+
   
 type
   TProtectedCollection  = class(TCollection);
@@ -1392,6 +1401,7 @@ begin
   FGroups.OnUpdate  := GroupsUpdate;
   FHideScrollbar    := True;
   FScrollbar        := True;
+  TabStop           := True;
 end;
 
 
@@ -1506,6 +1516,23 @@ begin
       begin
         currentAction.Stop();
         PopCurrentAction();
+
+        { Start the next action in the queue, continue until we find an
+          action which doesn't terminate immediately. See PushAction. }
+        currentAction := GetCurrentAction();
+        while Assigned(currentAction) do
+        begin
+          currentAction.Start();
+
+          if currentAction.Terminated then
+          begin
+            currentAction.Stop();
+            PopCurrentAction();
+
+            currentAction := GetCurrentAction();
+          end else
+            Break;
+        end;
       end;
     end;
   end
@@ -1949,8 +1976,29 @@ end;
 
 
 procedure TX2CustomMenuBar.PushAction(AAction: TX2CustomMenuBarAction);
+var
+  action:   TX2CustomMenuBarAction;
+
 begin
-  ActionQueue.Add(AAction);
+  action  := AAction;
+
+  if ActionQueue.Count = 0 then
+  begin
+    { Start the action; if it's terminated immediately don't add it to the
+      queue. This enables actions like selecting an item without requiring
+      animation to fire straight away. }
+    action.Start();
+
+    if action.Terminated then
+    begin
+      action.Stop();
+      FreeAndNil(action);
+    end;
+  end;
+
+  if Assigned(action) then
+    ActionQueue.Add(action);
+
   Invalidate();
 end;
 
@@ -1967,16 +2015,31 @@ procedure TX2CustomMenuBar.InternalSetExpanded(AGroup: TX2MenuBarGroup;
 begin
   AGroup.InternalSetExpanded(AExpanded);
   DoExpandedChanged(AGroup);
+
+  Invalidate();
 end;
 
 
 procedure TX2CustomMenuBar.InternalSetSelected(AItem: TX2CustomMenuBarItem);
+var
+  group:    TX2MenuBarGroup;
+
 begin
   FSelectedItem := AItem;
   DoSelectedChanged();
 
-  if Assigned(SelectedItem) and Assigned(SelectedItem.Action) then
+  if Assigned(AItem) then
-    SelectedItem.ActionLink.Execute(Self);
+  begin
+    if (AItem is TX2MenuBarItem) then
+    begin
+      group := TX2MenuBarItem(AItem).Group;
+      if Assigned(group) then
+        group.SelectedItem  := AItem.Index;
+    end;
+
+    if Assigned(AItem) and Assigned(AItem.Action) then
+      AItem.ActionLink.Execute(Self);
+  end;
 end;
 
 
@@ -2902,33 +2965,51 @@ var
 begin
   if Value <> FSelectedItem then
   begin
+    if Assigned(Value) and (Value.MenuBar <> Self) then
+      raise EInvalidItem.Create(SInvalidItem);
+
+
     allowed := (not Assigned(Value)) or ItemEnabled(Value);
     if allowed then
       DoSelectedChanging(Value, allowed);
 
+      
     if allowed then
     begin
       selectItem  := Value;
 
-      if selectItem is TX2MenuBarGroup then
+      if Assigned(selectItem) then
       begin
-        group := TX2MenuBarGroup(selectItem);
+        if selectItem is TX2MenuBarGroup then
+        begin
+          group := TX2MenuBarGroup(selectItem);
 
-        { Check if the group should be collapsed }
+          { Check if the group should be collapsed }
-        if group.Expanded and (not AutoCollapse) then
+          if group.Expanded and (not AutoCollapse) then
-        begin
-          PerformExpand(group, False);
-        end else
-        begin
-          if group.Items.Count > 0 then
           begin
-            PerformExpand(group, True);
+            PerformExpand(group, False);
-            PerformAutoSelectItem(group);
           end else
           begin
-            if PerformAutoCollapse(group) then
+            if group.Items.Count > 0 then
-              PerformSelectItem(group);
+            begin
+              PerformExpand(group, True);
+              PerformAutoSelectItem(group);
+            end else
+            begin
+              if PerformAutoCollapse(group) then
+                PerformSelectItem(group);
+            end;
           end;
+        end else
+        begin
+          if (selectItem is TX2MenuBarItem) then
+          begin
+            group := TX2MenuBarItem(selectItem).Group;
+            if Assigned(group) and (not group.Expanded) then
+              PerformExpand(group, True);
+          end;
+
+          PerformSelectItem(selectItem);
         end;
       end else
         PerformSelectItem(selectItem);
@@ -2936,5 +3017,15 @@ begin
   end;
 end;
 
+//procedure TX2CustomMenuBar.WMMouseWheel(var Message: TWMMouseWheel);
+//begin
+////  MessageBox(0, 'I gots a mousewheel', '', 0);
+//end;
+//
+//procedure TX2CustomMenuBar.CMMouseWheel(var Message: TCMMouseWheel);
+//begin
+////  MessageBox(0, 'I gots a mousewheel', '', 0);
+//end;
+
 end.
 

Source/X2CLMenuBarActions.pas

@@ -25,6 +25,7 @@ type
   public
     constructor Create(AMenuBar: TX2CustomMenuBar; AGroup: TX2MenuBarGroup;
                        AAnimator: TX2CustomMenuBarAnimator);
+    destructor Destroy(); override;
 
     procedure Start(); override;
 
@@ -130,6 +131,14 @@ begin
 end;
 
 
+destructor TX2MenuBarAnimateAction.Destroy();
+begin
+  FreeAndNil(FAnimator);
+
+  inherited;
+end;
+
+
 procedure TX2MenuBarAnimateAction.Start();
 begin
   inherited;
@@ -340,7 +349,6 @@ begin
   inherited;
 
   TProtectedX2CustomMenuBar(MenuBar).InternalSetExpanded(FGroup, FExpanding);
-  MenuBar.Invalidate();
   Terminate();
 end;