Fixes for removing and adding items to subgroups (#2821)

* Add methods to remove and add items to a subgroup.
Add method to change the subgroup an item is in.
No longer decrement subgroup ID when removing a subgroup.
Fixes #2594

* Moved the orderSubgroups call outside of the add/remove methods.
Added the ability to call the add/remove methods without a subgroupId parameter (takes from item data instead).

lib/timeline/component/Group.js

@@ -438,30 +438,9 @@ Group.prototype.add = function(item) {
 
   // add to
   if (item.data.subgroup !== undefined) {
-    if (this.subgroups[item.data.subgroup] === undefined) {
-      this.subgroups[item.data.subgroup] = {
-        height:0, 
-        top: 0,
-        start: item.data.start,
-        end: item.data.end,
-        visible: false, 
-        index:this.subgroupIndex, 
-        items: []
-      };
-      this.subgroupIndex++;
-    }
-
-
-    if (new Date(item.data.start) < new Date(this.subgroups[item.data.subgroup].start)) { 
-      this.subgroups[item.data.subgroup].start = item.data.start; 
-    }
-    if (new Date(item.data.end) > new Date(this.subgroups[item.data.subgroup].end)) { 
-      this.subgroups[item.data.subgroup].end = item.data.end; 
-    }
-
-    this.subgroups[item.data.subgroup].items.push(item);
+    this._addToSubgroup(item);
+    this.orderSubgroups();
   }
-  this.orderSubgroups();
 
   if (this.visibleItems.indexOf(item) == -1) {
     var range = this.itemSet.body.range; // TODO: not nice accessing the range like this
@@ -469,6 +448,34 @@ Group.prototype.add = function(item) {
   }
 };
 
+
+Group.prototype._addToSubgroup = function(item, subgroupId) {
+  subgroupId = subgroupId || item.data.subgroup;
+  if (subgroupId != undefined && this.subgroups[subgroupId] === undefined) {
+    this.subgroups[subgroupId] = {
+      height:0,
+      top: 0,
+      start: item.data.start,
+      end: item.data.end,
+      visible: false,
+      index:this.subgroupIndex,
+      items: []
+    };
+    this.subgroupIndex++;
+  }
+
+
+  if (new Date(item.data.start) < new Date(this.subgroups[subgroupId].start)) {
+    this.subgroups[subgroupId].start = item.data.start;
+  }
+  if (new Date(item.data.end) > new Date(this.subgroups[subgroupId].end)) {
+    this.subgroups[subgroupId].end = item.data.end;
+  }
+
+  this.subgroups[subgroupId].items.push(item);
+  
+};
+
 Group.prototype._updateSubgroupsSizes = function () {
   var me = this;
   if (me.subgroups) {
@@ -539,22 +546,30 @@ Group.prototype.remove = function(item) {
   if (index != -1) this.visibleItems.splice(index, 1);
 
   if(item.data.subgroup !== undefined){
-    var subgroup = this.subgroups[item.data.subgroup];
+    this._removeFromSubgroup(item);
+    this.orderSubgroups();
+  }
+};
+
+Group.prototype._removeFromSubgroup = function(item, subgroupId) {
+  subgroupId = subgroupId || item.data.subgroup;
+  if (subgroupId != undefined) {
+    var subgroup = this.subgroups[subgroupId];
     if (subgroup){
       var itemIndex = subgroup.items.indexOf(item);
-      subgroup.items.splice(itemIndex,1);
-      if (!subgroup.items.length){
-        delete this.subgroups[item.data.subgroup];
-        this.subgroupIndex--;
-      } else {
-        this._updateSubgroupsSizes();
+      //  Check the item is actually in this subgroup. How should items not in the group be handled?
+      if (itemIndex >= 0) {
+        subgroup.items.splice(itemIndex,1);
+        if (!subgroup.items.length){
+          delete this.subgroups[subgroupId];
+        } else {
+          this._updateSubgroupsSizes();
+        }
       }
-      this.orderSubgroups();
     }
   }
 };
 
-
 /**
  * Remove an item from the corresponding DataSet
  * @param {Item} item
@@ -735,6 +750,10 @@ Group.prototype._checkIfVisibleWithReference = function(item, visibleItems, visi
   }
 };
 
-
+Group.prototype.changeSubgroup = function(item, oldSubgroup, newSubgroup) {
+  this._removeFromSubgroup(item, oldSubgroup);
+  this._addToSubgroup(item, newSubgroup);
+  this.orderSubgroups();
+};
 
 module.exports = Group;

lib/timeline/component/ItemSet.js

@@ -1199,21 +1199,28 @@ ItemSet.prototype._updateItem = function(item, itemData) {
   var oldGroupId = item.data.group;
   var oldSubGroupId = item.data.subgroup;
 
+  if (oldGroupId != itemData.group) {
+    var oldGroup = this.groups[oldGroupId];
+    if (oldGroup) oldGroup.remove(item);
+  }
+
   // update the items data (will redraw the item when displayed)
   item.setData(itemData);
 
   var groupId = this._getGroupId(item.data);
-	var group = this.groups[groupId];
+  var group = this.groups[groupId];
   if (!group) {
-   	item.groupShowing = false;
+    item.groupShowing = false;
   } else if (group && group.data && group.data.showNested) {
-  	item.groupShowing = true;
+    item.groupShowing = true;
   }
   // update group
-  if (oldGroupId != item.data.group || oldSubGroupId != item.data.subgroup) {
-    var oldGroup = this.groups[oldGroupId];
-    if (oldGroup) oldGroup.remove(item);
-    if (group) group.add(item);
+  if (group) {
+    if (oldGroupId != item.data.group) {
+      group.add(item);
+    } else if (oldSubGroupId != item.data.subgroup) {
+      group.changeSubgroup(item, oldSubGroupId);
+    }
   }
 };