From 367c6192f10912b5ead0364ff6fcdacdd71f816b Mon Sep 17 00:00:00 2001
From: jos <wjosdejong@gmail.com>
Date: Thu, 17 Apr 2014 17:01:47 +0200
Subject: [PATCH] Fixed label not being cleaned up when group was deleted

---
 src/timeline/component/Group.js    | 25 +++++++++++++++----------
 src/timeline/component/GroupSet.js |  4 ++--
 test/timeline.html                 |  8 ++++----
 test/timeline_groups.html          | 11 +++++++++++
 4 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/src/timeline/component/Group.js b/src/timeline/component/Group.js
index 6595b056..95ddf7ff 100644
--- a/src/timeline/component/Group.js
+++ b/src/timeline/component/Group.js
@@ -1,15 +1,15 @@
 /**
  * @constructor Group
- * @param {Panel} contentPanel  // TODO: replace with a HTML Element
+ * @param {Element} groupFrame
  * @param {Element} labelFrame
  * @param {Number | String} groupId
  * @param {Object} [options]  Options to set initial property values
  *                            // TODO: describe available options
  * @extends Component
  */
-function Group (contentPanel, labelFrame, groupId, options) {
+function Group (groupFrame, labelFrame, groupId, options) {
   this.id = util.randomUUID();
-  this.contentPanel = contentPanel;
+  this.groupFrame = groupFrame;
   this.labelFrame = labelFrame;
 
   this.groupId = groupId;
@@ -87,7 +87,7 @@ Group.prototype.setItems = function setItems(itemSet) {
   if (this.itemSet) {
     // remove current item set
     this.itemSet.setItems();
-    this.contentPanel.removeChild(this.itemSet);
+    this.groupFrame.removeChild(this.itemSet.getFrame());
     this.itemSet = null;
   }
 
@@ -97,7 +97,7 @@ Group.prototype.setItems = function setItems(itemSet) {
     var itemSetOptions = Object.create(this.options);
     this.itemSet = new ItemSet(itemSetOptions);
     this.itemSet.on('change', this.emit.bind(this, 'change')); // propagate change event
-    this.contentPanel.appendChild(this.itemSet);
+    this.groupFrame.appendChild(this.itemSet.getFrame());
 
     if (this.range) this.itemSet.setRange(this.range);
 
@@ -118,8 +118,12 @@ Group.prototype.show = function show() {
     this.labelFrame.appendChild(this.dom.label);
   }
 
-  if (!this.contentPanel.hasChild(this.itemSet)) {
-    this.contentPanel.appendChild(this.itemSet);
+  var itemSetFrame = this.itemSet && this.itemSet.getFrame();
+  if (itemSetFrame) {
+    if (itemSetFrame.parentNode) {
+      itemSetFrame.parentNode.removeChild(itemSetFrame);
+    }
+    this.groupFrame.appendChild(itemSetFrame);
   }
 };
 
@@ -131,7 +135,10 @@ Group.prototype.hide = function hide() {
     this.dom.label.parentNode.removeChild(this.dom.label);
   }
 
-  this.contentPanel.removeChild(this.itemSet);
+  var itemSetFrame = this.itemset && this.itemSet.getFrame();
+  if (itemSetFrame && itemSetFrame.parentNode) {
+    itemSetFrame.parentNode.removeChild(itemSetFrame);
+  }
 };
 
 /**
@@ -172,8 +179,6 @@ Group.prototype.repaint = function repaint() {
 
   var resized = this.itemSet.repaint();
 
-  // TODO: top is redundant, cleanup
-  this.top    = this.itemSet ? this.itemSet.top : 0;
   this.height = this.itemSet ? this.itemSet.height : 0;
 
   this.dom.label.style.height = this.height + 'px';
diff --git a/src/timeline/component/GroupSet.js b/src/timeline/component/GroupSet.js
index e02c6526..d52c166a 100644
--- a/src/timeline/component/GroupSet.js
+++ b/src/timeline/component/GroupSet.js
@@ -272,7 +272,7 @@ GroupSet.prototype.repaint = function repaint() {
 
             // TODO: do not recreate the group with every update
 
-            group = new Group(me, me.labelSet.frame, id, groupOptions);
+            group = new Group(me.frame, me.labelSet.frame, id, groupOptions);
             group.on('change', me.emit.bind(me, 'change')); // propagate change event
             group.setRange(me.range);
             group.setItems(me.itemsData); // attach items data
@@ -289,8 +289,8 @@ GroupSet.prototype.repaint = function repaint() {
 
         case 'remove':
           if (group) {
-            group.hide();
             group.setItems(); // detach items data
+            group.hide(); // FIXME: for some reason when doing setItems after hide, setItems again makes the label visible
             delete groups[id];
           }
 
diff --git a/test/timeline.html b/test/timeline.html
index 417d5808..170c2297 100644
--- a/test/timeline.html
+++ b/test/timeline.html
@@ -40,7 +40,7 @@
 <div id="visualization"></div>
 
 <script>
-  console.time('create dataset')
+  console.time('create dataset');
 
   // create a dataset with items
   var now = moment().minutes(0).seconds(0).milliseconds(0);
@@ -81,11 +81,11 @@
     zoomMax: 1000 * 60 * 60 * 24 * 30 * 6    // 6 months
   };
 
-  console.timeEnd('create dataset')
+  console.timeEnd('create dataset');
 
-  console.time('create timeline')
+  console.time('create timeline');
   var timeline = new vis.Timeline(container, items, options);
-  console.timeEnd('create timeline')
+  console.timeEnd('create timeline');
 
 </script>
 </body>
diff --git a/test/timeline_groups.html b/test/timeline_groups.html
index 210fe45b..63b12c83 100644
--- a/test/timeline_groups.html
+++ b/test/timeline_groups.html
@@ -78,10 +78,21 @@
     groupOrder: 'content'
   };
 
+  console.time('create timeline');
   var timeline = new vis.Timeline(container);
+  console.timeEnd('create timeline');
+
+  console.time('set options');
   timeline.setOptions(options);
+  console.timeEnd('set options');
+
+  console.time('set groups');
   timeline.setGroups(groups);
+  console.timeEnd('set groups');
+
+  console.time('set items');
   timeline.setItems(items);
+  console.timeEnd('set items');
 
 </script>
 </body>