[Blocked] Local video stapling

static/css/chat.css

@@ -135,13 +135,14 @@ body {
   z-index: 1;
 }
 
-/* XXX once we get the local-video stapled to the remote-video,
+/* YYYaudit once we get the local-video stapled to the remote-video,
    much of this stuff may be irrelevant.  Audit this as part
    of that landing. */
 #call .local-video {
   position: absolute;
   bottom: 10px; /* 10px seems to give stablest stapling across most sizes */
-  right: 5px;
+  right: 5px; /* Adjusted dynamically, currently by callView resize handler
+               * If you change this; edit that handler */
   width: 20vw;
   min-width: 80px;
   max-width: 200px;

static/js/chat/views.js

@@ -397,6 +397,7 @@
 
       this.call.on('change:state', this.render, this);
 
+      window.addEventListener("resize", this._onWindowResize.bind(this));
       this.render();
     },
 
@@ -442,6 +443,32 @@
         this.$el.show();
       else
         this.$el.hide();
+    },
+
+    // be sure to change the localVideo.right property in chat.css when
+    // changing this!
+    _localVideoGutterWidth: 5,
+
+    _onWindowResize: function(event) {
+
+      // (re)positions localVideo to be stapled inside right of remote-video
+
+      var localVideo = this.$('#local-video').get(0);
+      var remoteVideo = this.$('#remote-video').get(0);
+
+      var streamHeight = remoteVideo.videoHeight;
+      var streamWidth = remoteVideo.videoWidth;
+
+      if (!streamHeight || !streamWidth) {
+        return;
+      }
+
+      var pillarboxWidth = app.utils.getPillarboxWidth(
+        [remoteVideo.clientWidth, remoteVideo.clientHeight],
+        [streamWidth, streamHeight]);
+
+      localVideo.style.right =
+        this._localVideoGutterWidth + pillarboxWidth + "px";
     }
   });
 

static/js/utils.js

@@ -183,4 +183,27 @@
     return [scaleFactor * streamSize[0], scaleFactor * streamSize[1]];
   };
 
+  /**
+   * Returns the width of the pillarbox (blank vertical column to the left
+   * or right of the video when preserving the video stream's aspect ratio
+   * requires the displayed video to be smaller than its containing box.
+   *
+   * XXX need to define behavior when width is not an integer
+   *
+   * @param {Array} boxSize -- size of entire <video> element: [width, height]
+   * @param {Array} streamSize -- size of the media stream: [width, height]
+   *
+   * @return {Number} the width of one of the pillarboxes (left and right
+   *                  pillarboxes should be sized identically
+   */
+  app.utils.getPillarboxWidth = function(boxSize, streamSize) {
+
+    var boxWidth = boxSize[0];
+
+    var displayedSize =
+      app.utils.computeDisplayedVideoSize(boxSize, streamSize);
+    var displayedWidth = displayedSize[0];
+
+    return (boxWidth - displayedWidth) / 2;
+  };
 })(app, _);

test/frontend/chat/callview_test.js

@@ -54,6 +54,17 @@ describe("CallView", function() {
       expect(shouldExplode).to.Throw(Error, /missing parameter: el/);
     });
 
+    it("should attach a resize handler to the window", function() {
+
+      sandbox.stub(window, "addEventListener");
+
+      new app.views.CallView({el: el, call: call});
+
+      sinon.assert.calledOnce(window.addEventListener);
+      sinon.assert.calledWith(window.addEventListener, "resize",
+        sinon.match.func);
+    });
+
     describe("Change events", function() {
       var callView;
 
@@ -201,4 +212,84 @@ describe("CallView", function() {
       });
     });
   });
+
+  // Sure would be nice to test this using synthetic events, but those don't
+  // seem to work well enough just yet.  Oh well, one of these years!
+  describe("#_onWindowResize", function() {
+
+    var el, callView, localElement, fakeEvent, remoteElement,
+      fakePillarboxWidth;
+
+    beforeEach(function() {
+
+      // note that we specify width and height here, even though they don't
+      // get laid out, and will therefore be _different_ than clientWidth
+      // and clientWidth (which happens for different reasons with <video>
+      // in the production code).  This is intentional so that our tests later
+      // can ensure the implementation will compute the correct results by
+      // using client* for its math.
+      el = $(['<div>',
+        '  <div id="local-video" width="20" height="20"></div>',
+        '  <div id="remote-video" width="320" height="200"></div>',
+        '</div>'].join(''));
+      $("#fixtures").append(el);
+      var $remoteElement = el.find('#remote-video');
+      remoteElement = $remoteElement.get(0);
+      remoteElement.videoHeight = 300;
+      remoteElement.videoWidth = 180;
+
+      callView = new app.views.CallView({el: el, call: call});
+
+      fakeEvent = {};
+
+      fakePillarboxWidth = 40;
+      sandbox.stub(app.utils, "getPillarboxWidth").returns(fakePillarboxWidth);
+    });
+
+    afterEach(function() {
+      $("#fixtures").empty();
+    });
+
+    // XXX
+    it("should not fire when the view is not small");
+
+    // XXX adjust test to also check that the view is small
+    it("should get the pillarbox width of the remoteVideo element",
+      function() {
+        callView._onWindowResize(fakeEvent);
+
+        // Note that we are testing against clientWidth and clientHeight,
+        // because using width & height will not work in production.
+        sinon.assert.calledOnce(app.utils.getPillarboxWidth);
+        sinon.assert.calledWith(app.utils.getPillarboxWidth,
+          [remoteElement.clientWidth, remoteElement.clientHeight],
+          [remoteElement.videoWidth, remoteElement.videoHeight]);
+      });
+
+    it("should set the CSS |right| property on the localVideo element to" +
+      " pillarboxWidth + gutterWidth",
+      function() {
+        var $localElement = el.find('#local-video');
+        localElement = $localElement.get(0);
+        localElement.style.right = "";
+
+        callView._onWindowResize(fakeEvent);
+
+        expect(localElement.style.right).to.
+          equal(fakePillarboxWidth + callView._localVideoGutterWidth + "px");
+      });
+
+    // there's no meaningful thing to resize in this case, and making the call
+    // would violate the getPillarboxWidth API and cause an exception
+    it("should not call getPillarboxWidth if the remote stream's height" +
+      " or width is 0",
+      function() {
+        remoteElement.videoHeight = 0;
+
+        callView._onWindowResize(fakeEvent);
+
+        sinon.assert.notCalled(app.utils.getPillarboxWidth);
+      });
+  });
+
 });

test/frontend/utils_test.js

@@ -301,5 +301,36 @@ describe('Utils', function() {
      */
     it("should have defined behavior when sizes are non-integer multiples");
     it("should have defined behavior on zero-box-size elements");
-  });   
+  });
+
+  describe("#getPillarboxWidth", function () {
+
+    // We're stubbing out computeDisplayedVideoSize even though it's
+    // in module because testing this API only would be very hard to
+    // ensure we got sufficient coverage.  I.e. this is a "humble function",
+    // analogous to the "humble object" pattern.
+
+    it("should return half the difference between the box width and" +
+      " the displayed video width",
+      function() {
+        var boxWidth=400;
+        var fakeDisplayedWidth = 271;
+        var fakeDimension = 1;
+        sandbox.stub(app.utils, "computeDisplayedVideoSize").
+          returns([fakeDisplayedWidth, fakeDimension]);
+
+        var pbWidth = app.utils.getPillarboxWidth([boxWidth, fakeDimension],
+          [fakeDimension, fakeDimension]);
+
+        expect(pbWidth).to.equal((boxWidth - fakeDisplayedWidth) / 2);
+      });
+
+
+    // XXX need to write test to define behavior when returned value would
+    // not be an integer, make the test pass, and remove the similar
+    // comment in the impl code
+    it("should behave in a defined way when the return value would not" +
+      " be an int");
+
+  });
 });