fix(Masonry): Changing Masonry parent leads to removal of its items (#…

…139) * fix(Masonry): Changing Masonry parent leads to removal of its items Whenever we tries to change the masonry parent element from one element to another or reattach to same parent lead to removal of its child items. * fix(Masonry): Changing Masonry parent leads to removal of its items * fix(Masonry): Changing Masonry parent leads to removal of its items * fix(Masonry): Changing Masonry parent leads to removal of its items Co-authored-by: Pareesh Gupta <paregupt@adobe.com>
adobe · Oct 22, 2020 · 46ae499 · 46ae499
1 parent a8ffe46
commit 46ae499
Show file tree

Hide file tree

Showing 4 changed files with 161 additions and 4 deletions.
diff --git a/coral-component-masonry/src/scripts/MasonryItem.js b/coral-component-masonry/src/scripts/MasonryItem.js
@@ -150,8 +150,11 @@ class MasonryItem extends BaseComponent(HTMLElement) {
 
   /** @ignore */
   connectedCallback() {
+    if (!this.isConnected) {
+      return;
+    }
+
     super.connectedCallback();
-
     // Inform masonry immediately
     this.trigger('coral-masonry-item:_connected');
   }
@@ -177,8 +180,12 @@ class MasonryItem extends BaseComponent(HTMLElement) {
 
   /** @ignore */
   disconnectedCallback() {
+    if (this.isConnected) {
+      return;
+    }
+
     super.disconnectedCallback();
-  
+
     // Handle it in masonry immediately
     const masonry = this._masonry;
     if (masonry) {

diff --git a/coral-component-masonry/src/tests/snippets/Masonry.with.div.wrapper.html b/coral-component-masonry/src/tests/snippets/Masonry.with.div.wrapper.html
@@ -0,0 +1,6 @@
+<div>
+  <coral-masonry layout="fixed-centered" columnwidth="250">
+    <coral-masonry-item>item1</coral-masonry-item>
+    <coral-masonry-item>item2</coral-masonry-item>
+  </coral-masonry>
+</div>
diff --git a/coral-component-masonry/src/tests/test.Masonry.Item.js b/coral-component-masonry/src/tests/test.Masonry.Item.js
@@ -12,6 +12,7 @@
 
 import {helpers} from '../../../coral-utils/src/tests/helpers';
 import {Masonry} from '../../../coral-component-masonry';
+import {commons} from '../../../coral-utils';
 
 describe('Masonry.Item', function() {
   describe('Namespace', function() {
@@ -186,7 +187,75 @@ describe('Masonry.Item', function() {
       });
     });
   });
-
+
+  describe('Attach/Detach', function() {
+    describe('replacing masonry item', function() {
+      it('with another masonry item using replaceChild', function(done) {
+        const el = helpers.build(window.__html__['Masonry.with.div.wrapper.html']);
+        const masonry = el.querySelector("coral-masonry");
+        const oldItem = masonry.querySelector("coral-masonry-item");
+
+        const newItem = new Masonry.Item();
+        newItem.content.innerHTML = "Hi";
+
+        masonry.replaceChild(newItem, oldItem);
+
+        // Here we cannot test oldItem _disconnected and isConnected
+        // because we again attach item with removing attribute to show transition.
+        expect(oldItem.hasAttribute("_removing")).to.be.true;
+
+        expect(newItem.isConnected).to.be.true;
+        expect(newItem.parentElement).to.equal(masonry);
+        expect(masonry.items.getAll().length).to.equal(2);
+
+        const item = masonry.querySelector("coral-masonry-item");
+        //wait for transition to end
+        commons.transitionEnd(oldItem, () => {
+          helpers.next(function() {
+            expect(masonry.items.getAll().length).to.equal(2);
+            // After transition, we test for oldItem _disconnected and isConnected
+            expect(oldItem._disconnected).to.be.true;
+            expect(oldItem.isConnected).to.be.false;
+            expect(oldItem.parentElement).to.equal(null);
+            done();
+          });
+        });
+      });
+
+      it('with another masonry item using replaceWith', function(done) {
+        const el = helpers.build(window.__html__['Masonry.with.div.wrapper.html']);
+        const masonry = el.querySelector("coral-masonry");
+        const oldItem = masonry.querySelector("coral-masonry-item");
+
+        const newItem = new Masonry.Item();
+        newItem.content.innerHTML = "Hi";
+
+        oldItem.replaceWith(newItem);
+
+        // Here we cannot test oldItem _disconnected and isConnected
+        // because we again attach item with removing attribute to show transition.
+        expect(oldItem.hasAttribute("_removing")).to.be.true;
+
+        expect(newItem.isConnected).to.be.true;
+        expect(newItem.parentElement).to.equal(masonry);
+        expect(masonry.items.getAll().length).to.equal(2);
+
+        const item = masonry.querySelector("coral-masonry-item");
+        //wait for transition to end
+        commons.transitionEnd(oldItem, () => {
+          helpers.next(function() {
+            expect(masonry.items.getAll().length).to.equal(2);
+            // after actual removal _disconnected is false and isConnected is true
+            expect(oldItem._disconnected).to.be.true;
+            expect(oldItem.isConnected).to.be.false;
+            expect(oldItem.parentElement).to.equal(null);
+            done();
+          });
+        });
+      });
+    });
+  });
+
   describe('Accessibility', function() {
     it('should have an aria attribute for selection', function() {
       const el = new Masonry.Item();

diff --git a/coral-component-masonry/src/tests/test.Masonry.js b/coral-component-masonry/src/tests/test.Masonry.js
@@ -12,8 +12,9 @@
 
 import {helpers} from '../../../coral-utils/src/tests/helpers';
 import {Masonry} from '../../../coral-component-masonry';
+import {commons} from '../../../coral-utils';
 
-describe('Masonry.Layout', function() {
+describe('Masonry', function() {
 
   describe('Instantiation', function() {
 
@@ -532,6 +533,80 @@ describe('Masonry.Layout', function() {
       expect(el.parentElement.getAttribute('aria-label')).to.equal('Masonry Label', 'Masonry parent element should receive same aria-label as Masonry');
       expect(el.parentElement.getAttribute('aria-labelledby')).to.equal('Masonry Labelledby', 'Masonry parent element should receive same aria-labelledby as Masonry');
     });
+  });
+
+  describe('Attach/Detch', function() {
+    it('changing masonry parent should keep child intact', function(done) {
+      const el = helpers.build(window.__html__['Masonry.with.div.wrapper.html']);
+      const masonry = el.querySelector("coral-masonry");
+
+      expect(masonry.items.getAll().length).to.equal(2);
+      // appending to same parent still calls disconnected and connected callbacks
+      el.appendChild(masonry);
+      helpers.next(function() {
+        expect(masonry.items.getAll().length).to.equal(2);
+        done();
+      });
+    });
+
+    it('item removed should be disconnected with masonry', function(done) {
+      const el = helpers.build(window.__html__['Masonry.with.div.wrapper.html']);
+      const masonry = el.querySelector("coral-masonry");
+      const item =  masonry.querySelector("coral-masonry-item");
+
+      expect(masonry.items.getAll().length).to.equal(2);
+
+      item.remove();
+      // Here we cannot test item _disconnected and isConnected
+      // because we again attach item with removing attribute to show transition.
+      expect(item.hasAttribute("_removing")).to.be.true;
+
+      //wait for transition to end
+      commons.transitionEnd(item, () => {
+        helpers.next(function() {
+          expect(masonry.items.getAll().length).to.equal(1);
+          // After transition, we test for oldItem _disconnected and isConnected
+          expect(item._disconnected).to.be.true;
+          expect(item.isConnected).to.be.false;
+          done();
+        });
+      });
+    });
+
+    it('removed child should be disconnected with masonry', function(done) {
+      const el = helpers.build(window.__html__['Masonry.with.div.wrapper.html']);
+      const masonry = el.querySelector("coral-masonry");
+      const item =  masonry.querySelector("coral-masonry-item");
+
+      expect(masonry.items.getAll().length).to.equal(2);
+
+      masonry.removeChild(item);
+      // Here we cannot test item _disconnected and isConnected
+      // because we again attach item with removing attribute to show transition.
+      expect(item.hasAttribute("_removing")).to.be.true;
+
+      //wait for transition to end
+      commons.transitionEnd(item, () => {
+        helpers.next(function() {
+          expect(masonry.items.getAll().length).to.equal(1);
+          // After transition, we test for oldItem _disconnected and isConnected
+          expect(item._disconnected).to.be.true;
+          expect(item.isConnected).to.be.false;
+          done();
+        });
+      });
+    });
 
+    it('added item should be connected with masonry', function() {
+      const el = helpers.build(window.__html__['Masonry.with.div.wrapper.html']);
+      const masonry = el.querySelector("coral-masonry");
+      const item = document.createElement("coral-masonry-item");
+      item.content.innerHTML = "Hi";
+
+      masonry.appendChild(item);
+
+      expect(masonry.items.getAll().length).to.equal(3);
+      expect(item.parentElement).to.equal(masonry);
+    });
   });
 });