fix internals of stateless components, bulbs-video autoplay (#294)

* fix internals of stateless components * updating bulbs-video autoplay attribute will now cause the player to redispatch * fix tests
theonion · Apr 6, 2017 · 6c84d74 · 6c84d74
1 parent 1920540
commit 6c84d74
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 5 deletions.
diff --git a/elements/bulbs-video/bulbs-video.js b/elements/bulbs-video/bulbs-video.js
@@ -26,7 +26,9 @@ export default class BulbsVideo extends BulbsElement {
   }
 
   componentDidUpdate (prevProps) {
-    if (this.props.src !== prevProps.src) {
+    if (this.props.src !== prevProps.src ||
+        this.props.autoplay !== prevProps.autoplay) {
+
       this.store.actions.resetController();
       this.store.actions.setVideoField(null); // eslint-disable-line no-undefined
       requestAnimationFrame(() => {

diff --git a/elements/bulbs-video/bulbs-video.test.js b/elements/bulbs-video/bulbs-video.test.js
@@ -7,9 +7,11 @@ import fetchMock from 'fetch-mock';
 import { scrollingElement } from 'bulbs-elements/util';
 
 describe('<bulbs-video>', () => {
+  let autoplay = '';
   let src = '//example.org/video-src.json';
   let subject;
   let props = {
+    autoplay,
     src,
     disableLazyLoading: true,
   };
@@ -54,16 +56,17 @@ describe('<bulbs-video>', () => {
     let fetchSpy;
     let resetSpy;
     let newSrc;
+    let newAutoplay;
 
     beforeEach(() => {
       subject = new BulbsVideo(props);
     });
 
-    context('src did not change', () => {
+    context('src and autoplay did not change', () => {
       beforeEach(() => {
         fetchSpy = sinon.spy(subject.store.actions, 'fetchVideo');
         resetSpy = sinon.spy(subject.store.actions, 'resetController');
-        subject.componentDidUpdate({ src });
+        subject.componentDidUpdate({ autoplay, src });
         newSrc = src;
       });
 
@@ -100,6 +103,31 @@ describe('<bulbs-video>', () => {
         });
       });
     });
+
+    context('autoplay did change', () => {
+      beforeEach(() => {
+        fetchSpy = sinon.spy(subject.store.actions, 'fetchVideo');
+        resetSpy = sinon.spy(subject.store.actions, 'resetController');
+        newAutoplay = false;
+        fetchMock.mock(src, {});
+        subject.props.autoplay = newAutoplay;
+        subject.componentDidUpdate({ autoplay, src });
+      });
+
+      it('fetches video data', (done) => {
+        setImmediate(() => {
+          expect(fetchSpy).to.have.been.calledWith(newSrc);
+          done();
+        });
+      });
+
+      it('resets the controller', (done) => {
+        setImmediate(() => {
+          expect(resetSpy).to.have.been.called;
+          done();
+        });
+      });
+    });
   });
 
   describe('lazy loading', () => {

diff --git a/lib/bulbs-elements/register.js b/lib/bulbs-elements/register.js
@@ -61,7 +61,7 @@ export function registerReactElement (tagName, ReactComponent) {
       skip the rendering step.
     */
     attachedCallback () {
-      if (!this.reactInstance) {
+      if (!this.rendered) {
         this.doRender();
       }
     }
@@ -76,7 +76,7 @@ export function registerReactElement (tagName, ReactComponent) {
     }
 
     attributeChangedCallback () {
-      if (this.reactInstance) {
+      if (this.rendered) {
         this.doRender();
       }
     }
@@ -86,6 +86,13 @@ export function registerReactElement (tagName, ReactComponent) {
         ReactComponent,
         this.attributesHash
       );
+      this.rendered = true;
+
+      // NOTE: this will only fill in for elements that are stateful, stateless
+      //  components will have this property as `null`. From ReactDOM.render
+      //  docs, "Render a React element into the DOM in the supplied
+      //  `container` and return a reference to the component (or returns
+      //  `null` for stateless components."
       this.reactInstance = ReactDOM.render(
         renderableReactElement,
         this.mountPoint