Avoid video embeds being cut off in narrow viewports

Previously video embeds were given a fixed with of 369px, which is close
to the width available to annotation content at the sidebar's default width
(~380px). On narrower viewports the video would get cut off.

Fix this by setting the width of video embeds to `100%` and using a CSS
hack [1] to set the height of the video such that the embed has a 16:9
aspect ratio, which is what YouTube optimizes for.

 - Set `width: 100%` on `excerpt__content` and `markdown-view` so that
   the annotation content always fills the available width.

 - Add support for specifying a class name for embed containers when
   calling `replaceLinksWithEmbeds` and use that to give embeds rendered
   in `MarkdownView` a `width` of 100%.

 - Change `replaceLinksWithEmbeds` to wrap iframes with an aspect-ratio
   box (see [1]) which causes the iframe's height to be adjusted as the
   width changes to have a 16:9 aspect ratio.

   Adding this container required changes in `media-embedder-test` to
   allow the `<iframe>` to be wrapped in a container element.

[1] https://css-tricks.com/aspect-ratio-boxes/

src/sidebar/components/markdown-view.js

@@ -14,10 +14,12 @@ export default function MarkdownView({ markdown = '', textClass = {} }) {
   const html = useMemo(() => (markdown ? renderMarkdown(markdown) : ''), [
     markdown,
   ]);
-  const content = useRef(null);
+  const content = useRef(/** @type {HTMLDivElement|null} */ (null));
 
   useEffect(() => {
-    replaceLinksWithEmbeds(content.current);
+    replaceLinksWithEmbeds(content.current, {
+      className: 'markdown-view__embed',
+    });
   }, [markdown]);
 
   // Use a blank string to indicate that the content language is unknown and may be

src/sidebar/components/test/markdown-view-test.js

@@ -13,10 +13,7 @@ describe('MarkdownView', () => {
   beforeEach(() => {
     fakeRenderMarkdown = markdown => `rendered:${markdown}`;
     fakeMediaEmbedder = {
-      replaceLinksWithEmbeds: el => {
-        // Tag the element as having been processed
-        el.dataset.replacedLinksWithEmbeds = 'yes';
-      },
+      replaceLinksWithEmbeds: sinon.stub(),
     };
 
     $imports.$mock({
@@ -50,7 +47,9 @@ describe('MarkdownView', () => {
   it('replaces links with embeds in rendered output', () => {
     const wrapper = mount(<MarkdownView markdown="**test**" />);
     const rendered = wrapper.find('.markdown-view').getDOMNode();
-    assert.equal(rendered.dataset.replacedLinksWithEmbeds, 'yes');
+    assert.calledWith(fakeMediaEmbedder.replaceLinksWithEmbeds, rendered, {
+      className: 'markdown-view__embed',
+    });
   });
 
   it('applies `textClass` class to container', () => {

src/sidebar/media-embedder.js

@@ -11,16 +11,47 @@ function audioElement(src) {
   return html5audio;
 }
 
+/**
+ * Wrap an element in a container that causes the element to be displayed at
+ * a given aspect ratio.
+ *
+ * See https://css-tricks.com/aspect-ratio-boxes/.
+ *
+ * @param {HTMLElement} element
+ * @param {number} aspectRatio - Aspect ratio as `width/height`
+ * @return {HTMLElement}
+ */
+function wrapInAspectRatioContainer(element, aspectRatio) {
+  element.style.position = 'absolute';
+  element.style.top = '0';
+  element.style.left = '0';
+  element.style.width = '100%';
+  element.style.height = '100%';
+
+  const container = document.createElement('div');
+  container.style.paddingBottom = `${(1 / aspectRatio) * 100}%`;
+  container.style.position = 'relative';
+  container.appendChild(element);
+
+  return container;
+}
+
 /**
  * Return an iframe DOM element with the given src URL.
+ *
+ * @param {string} src
  */
 function iframe(src) {
   const iframe_ = document.createElement('iframe');
   iframe_.src = src;
-  iframe_.classList.add('annotation-media-embed');
   iframe_.setAttribute('frameborder', '0');
   iframe_.setAttribute('allowfullscreen', '');
-  return iframe_;
+
+  // 16:9 is the aspect ratio that YouTube videos are optimized for.
+  // We assume here that this works for other embed types as well.
+  const aspectRatio = 16 / 9;
+
+  return wrapInAspectRatioContainer(iframe_, aspectRatio);
 }
 
 /**
@@ -297,6 +328,7 @@ function embedForLink(link) {
  *
  * If the link is not a link to an embeddable media it will be left untouched.
  *
+ * @return {HTMLElement|null}
  */
 function replaceLinkWithEmbed(link) {
   // The link's text may or may not be percent encoded. The `link.href` property
@@ -306,12 +338,13 @@ function replaceLinkWithEmbed(link) {
     link.href !== link.textContent &&
     decodeURI(link.href) !== link.textContent
   ) {
-    return;
+    return null;
   }
   const embed = embedForLink(link);
   if (embed) {
     link.parentElement.replaceChild(embed, link);
   }
+  return embed;
 }
 
 /**
@@ -320,8 +353,13 @@ function replaceLinkWithEmbed(link) {
  * All links to YouTube videos or other embeddable media will be replaced with
  * embeds of the same media.
  *
+ * @param {HTMLElement} element
+ * @param {Object} options
+ *   @param {string} [options.className] -
+ *     Class name to apply to embed containers. An important function of this class is to set
+ *     the width of the embed.
  */
-export function replaceLinksWithEmbeds(element) {
+export function replaceLinksWithEmbeds(element, { className } = {}) {
   let links = element.getElementsByTagName('a');
 
   // `links` is a "live list" of the <a> element children of `element`.
@@ -332,6 +370,14 @@ export function replaceLinksWithEmbeds(element) {
 
   let i;
   for (i = 0; i < links.length; i++) {
-    replaceLinkWithEmbed(links[i]);
+    const embed = replaceLinkWithEmbed(links[i]);
+    if (embed) {
+      if (className) {
+        embed.className = className;
+      } else {
+        // Default width.
+        embed.style.width = '350px';
+      }
+    }
   }
 }

src/sidebar/test/media-embedder-test.js

@@ -17,6 +17,28 @@ describe('media-embedder', function () {
     clock.restore();
   });
 
+  /**
+   * Find all the media embed elements in a container.
+   */
+  function findEmbeds(element) {
+    return [...element.querySelectorAll('iframe,audio')];
+  }
+
+  /**
+   * Return the URL of the single media embed in `element`.
+   */
+  function embedUrl(element) {
+    const embeds = findEmbeds(element);
+    assert.equal(embeds.length, 1);
+    return embeds[0].src;
+  }
+
+  function assertStyle(element, expectedProperties) {
+    Object.entries(expectedProperties).forEach(([prop, value]) => {
+      assert.equal(element.style[prop], value);
+    });
+  }
+
   it('replaces YouTube watch links with iframes', function () {
     const urls = [
       'https://www.youtube.com/watch?v=QCkm0lL-6lc',
@@ -30,10 +52,8 @@ describe('media-embedder', function () {
 
       mediaEmbedder.replaceLinksWithEmbeds(element);
 
-      assert.equal(element.childElementCount, 1);
-      assert.equal(element.children[0].tagName, 'IFRAME', url);
       assert.equal(
-        element.children[0].src,
+        embedUrl(element),
         'https://www.youtube.com/embed/QCkm0lL-6lc'
       );
     });
@@ -50,14 +70,12 @@ describe('media-embedder', function () {
 
       mediaEmbedder.replaceLinksWithEmbeds(element);
 
-      assert.equal(element.childElementCount, 1);
-      assert.equal(element.children[0].tagName, 'IFRAME', url);
+      assert.equal(
+        embedUrl(element),
         // queryString's #stringify sorts keys, so resulting query string
         // will be reliably as follows, regardless of original ordering
         // Note also `v` param is handled elsewhere and is not "allowed" in
         // queryString.
-      assert.equal(
-        element.children[0].src,
         'https://www.youtube.com/embed/QCkm0lL-6lc?end=10&start=5'
       );
     });
@@ -74,10 +92,8 @@ describe('media-embedder', function () {
 
       mediaEmbedder.replaceLinksWithEmbeds(element);
 
-      assert.equal(element.childElementCount, 1);
-      assert.equal(element.children[0].tagName, 'IFRAME', url);
       assert.equal(
-        element.children[0].src,
+        embedUrl(element),
         'https://www.youtube.com/embed/QCkm0lL-6lc?end=10&start=5'
       );
     });
@@ -131,9 +147,7 @@ describe('media-embedder', function () {
 
       mediaEmbedder.replaceLinksWithEmbeds(element);
 
-      assert.equal(element.childElementCount, 1);
-      assert.equal(element.children[0].tagName, 'IFRAME', url[0]);
-      assert.equal(element.children[0].src, url[1]);
+      assert.equal(embedUrl(element), url[1]);
     });
   });
 
@@ -147,12 +161,10 @@ describe('media-embedder', function () {
 
       mediaEmbedder.replaceLinksWithEmbeds(element);
 
-      assert.equal(element.childElementCount, 1);
-      assert.equal(element.children[0].tagName, 'IFRAME');
+      assert.equal(
+        embedUrl(element),
         // queryString's #stringify sorts keys, so resulting query string
         // will be reliably as follows, regardless of original ordering
-      assert.equal(
-        element.children[0].src,
         'https://www.youtube.com/embed/QCkm0lL-6lc?end=10&start=5'
       );
     });
@@ -168,10 +180,8 @@ describe('media-embedder', function () {
 
       mediaEmbedder.replaceLinksWithEmbeds(element);
 
-      assert.equal(element.childElementCount, 1);
-      assert.equal(element.children[0].tagName, 'IFRAME');
       assert.equal(
-        element.children[0].src,
+        embedUrl(element),
         'https://www.youtube.com/embed/QCkm0lL-6lc'
       );
     });
@@ -187,12 +197,10 @@ describe('media-embedder', function () {
 
       mediaEmbedder.replaceLinksWithEmbeds(element);
 
-      assert.equal(element.childElementCount, 1);
-      assert.equal(element.children[0].tagName, 'IFRAME');
+      assert.equal(
+        embedUrl(element),
         // queryString's #stringify sorts keys, so resulting query string
         // will be reliably as follows, regardless of original ordering
-      assert.equal(
-        element.children[0].src,
         'https://www.youtube.com/embed/QCkm0lL-6lc?end=10&start=5'
       );
     });
@@ -208,10 +216,8 @@ describe('media-embedder', function () {
 
       mediaEmbedder.replaceLinksWithEmbeds(element);
 
-      assert.equal(element.childElementCount, 1);
-      assert.equal(element.children[0].tagName, 'IFRAME', url);
       assert.equal(
-        element.children[0].src,
+        embedUrl(element),
         'https://www.youtube.com/embed/QCkm0lL-6lc?end=10&start=5'
       );
     });
@@ -227,12 +233,8 @@ describe('media-embedder', function () {
 
       mediaEmbedder.replaceLinksWithEmbeds(element);
 
-      assert.equal(element.childElementCount, 1);
-      assert.equal(element.children[0].tagName, 'IFRAME');
-      // queryString's #stringify sorts keys, so resulting query string
-      // will be reliably as follows, regardless of original ordering
       assert.equal(
-        element.children[0].src,
+        embedUrl(element),
         'https://www.youtube.com/embed/QCkm0lL-6lc?end=10&start=5'
       );
     });
@@ -252,10 +254,8 @@ describe('media-embedder', function () {
 
       mediaEmbedder.replaceLinksWithEmbeds(element);
 
-      assert.equal(element.childElementCount, 1);
-      assert.equal(element.children[0].tagName, 'IFRAME');
       assert.equal(
-        element.children[0].src,
+        embedUrl(element),
         'https://player.vimeo.com/video/149000090'
       );
     });
@@ -276,10 +276,8 @@ describe('media-embedder', function () {
 
       mediaEmbedder.replaceLinksWithEmbeds(element);
 
-      assert.equal(element.childElementCount, 1);
-      assert.equal(element.children[0].tagName, 'IFRAME');
       assert.equal(
-        element.children[0].src,
+        embedUrl(element),
         'https://player.vimeo.com/video/148845534'
       );
     });
@@ -304,9 +302,7 @@ describe('media-embedder', function () {
 
       mediaEmbedder.replaceLinksWithEmbeds(element);
 
-      assert.equal(element.children[0].tagName, 'IFRAME');
-
-      const actual = element.children[0].src;
+      const actual = embedUrl(element);
       const expected =
         url.indexOf('start') !== -1
           ? 'https://archive.org/embed/PATH?start=360&end=420.3'
@@ -406,16 +402,59 @@ describe('media-embedder', function () {
 
     mediaEmbedder.replaceLinksWithEmbeds(element);
 
-    assert.equal(element.childElementCount, 2);
-    assert.equal(element.children[0].tagName, 'IFRAME');
-    assert.equal(
-      element.children[0].src,
-      'https://www.youtube.com/embed/QCkm0lL-6lc'
-    );
-    assert.equal(element.children[1].tagName, 'IFRAME');
-    assert.equal(
-      element.children[1].src,
-      'https://www.youtube.com/embed/abcdefg'
-    );
+    const embeds = findEmbeds(element);
+
+    assert.equal(embeds.length, 2);
+    assert.equal(embeds[0].src, 'https://www.youtube.com/embed/QCkm0lL-6lc');
+    assert.equal(embeds[1].src, 'https://www.youtube.com/embed/abcdefg');
+  });
+
+  it('applies `className` option to video embeds', () => {
+    const url = 'https://www.youtube.com/watch?v=QCkm0lL-6lc';
+    const element = domElement('<a href="' + url + '">' + url + '</a>');
+
+    mediaEmbedder.replaceLinksWithEmbeds(element, {
+      className: 'widget__video',
+    });
+
+    assert.equal(element.childElementCount, 1);
+    assert.equal(element.children[0].className, 'widget__video');
+  });
+
+  it('sets a default width on video embeds if no `className` if provided', () => {
+    const url = 'https://www.youtube.com/watch?v=QCkm0lL-6lc';
+    const element = domElement('<a href="' + url + '">' + url + '</a>');
+
+    mediaEmbedder.replaceLinksWithEmbeds(element);
+
+    assert.equal(element.childElementCount, 1);
+    assert.equal(element.children[0].style.width, '350px');
+  });
+
+  it('wraps video embeds in an aspect-ratio box', () => {
+    const url = 'https://www.youtube.com/watch?v=QCkm0lL-6lc';
+    const element = domElement('<a href="' + url + '">' + url + '</a>');
+
+    mediaEmbedder.replaceLinksWithEmbeds(element);
+
+    assert.equal(element.childElementCount, 1);
+    assert.equal(element.children[0].tagName, 'DIV');
+
+    const aspectRatioBox = element.children[0];
+    assertStyle(aspectRatioBox, {
+      position: 'relative',
+      paddingBottom: '56.25%' /* 9/16 as a percentage */,
+    });
+    assert.equal(aspectRatioBox.childElementCount, 1);
+
+    const embed = aspectRatioBox.children[0];
+    assert.equal(embed.tagName, 'IFRAME');
+    assertStyle(embed, {
+      position: 'absolute',
+      top: '0px',
+      left: '0px',
+      width: '100%',
+      height: '100%',
+    });
   });
 });

src/styles/sidebar/components/annotation.scss

@@ -32,9 +32,3 @@
     margin-left: auto;
   }
 }
-
-// FIXME move me
-.annotation-media-embed {
-  width: 369px;
-  height: 208px;
-}

src/styles/sidebar/components/excerpt.scss

@@ -20,6 +20,7 @@
     // See https://developer.mozilla.org/en-US/docs/Web/Guide/CSS/Block_formatting_context
     // and https://github.com/hypothesis/client/issues/1518.
     display: inline-block;
+    width: 100%;
   }
 
   // inline controls for expanding and collapsing

src/styles/sidebar/components/markdown-view.scss

 @use "../styled-text";
 
 .markdown-view {
+  width: 100%;
+
   @include styled-text.styled-text;
 
   cursor: text;
@@ -24,4 +26,8 @@
   p:last-child {
     margin-bottom: 1px;
   }
+
+  &__embed {
+    width: 100%;
+  }
 }