From 34db08453cb9d9b6ee1c4a192febd522532efbb7 Mon Sep 17 00:00:00 2001
From: Ryan Walters <ryan@ryanwalters.dev>
Date: Tue, 1 Aug 2023 11:22:52 -0500
Subject: [PATCH] refactor(Ad): always include empty div

BREAKING CHANGE: The `adUrl` prop has been removed
---
 .../MediaOverlay/MediaOverlayContainer.js     |  9 ++--
 src/components/MediaOverlay/README.md         |  1 -
 src/components/MediaOverlay/Sidebar/Ad/Ad.js  | 49 +++----------------
 .../MediaOverlay/Sidebar/Ad/Ad.module.scss    |  2 +-
 .../MediaOverlay/Sidebar/Sidebar.js           | 12 ++---
 5 files changed, 15 insertions(+), 58 deletions(-)

diff --git a/src/components/MediaOverlay/MediaOverlayContainer.js b/src/components/MediaOverlay/MediaOverlayContainer.js
index 21a09a3..9947560 100644
--- a/src/components/MediaOverlay/MediaOverlayContainer.js
+++ b/src/components/MediaOverlay/MediaOverlayContainer.js
@@ -285,18 +285,17 @@ class MediaOverlayContainer extends Component {
     });
   }
 
-
   // --- Sidebar
 
   toggleSidebar() {
-    this.setState(prevState => ({
+    this.setState((prevState) => ({
       isSidebarVisible: !prevState.isSidebarVisible,
     }));
   }
 
   toggleSidebarAndControls() {
     if (areControlsToggleable()) {
-      this.setState(prevState => ({
+      this.setState((prevState) => ({
         controlsHidden: !prevState.controlsHidden,
         isSidebarVisible: !prevState.isSidebarVisible,
       }));
@@ -345,7 +344,7 @@ class MediaOverlayContainer extends Component {
         betweenWidths(ViewportWidth.LG_MIN, ViewportWidth.LG_MAX),
         betweenWidths(ViewportWidth.MD_MIN, ViewportWidth.MD_MAX),
       ],
-      this.handleBreakpoints
+      this.handleBreakpoints,
     );
 
     onMinWidth(ViewportWidth.LG_MIN, () => this.showSidebarAndControls(true));
@@ -390,7 +389,6 @@ class MediaOverlayContainer extends Component {
 // --- Props
 
 MediaOverlayContainer.propTypes = {
-  adUrl: PropTypes.string,
   assemblies: PropTypes.arrayOf(AssemblyProp).isRequired,
   audioComponent: PropTypes.func,
   baseHref: PropTypes.string,
@@ -427,7 +425,6 @@ MediaOverlayContainer.defaultProps = {
   className: null,
   collapsibleSidebar: false,
   generatePrerollUrl: null,
-  adUrl: null,
   locale: Locale['en-us'],
   title: '',
   videoPlayerId: '',
diff --git a/src/components/MediaOverlay/README.md b/src/components/MediaOverlay/README.md
index a492bea..0f4ea22 100644
--- a/src/components/MediaOverlay/README.md
+++ b/src/components/MediaOverlay/README.md
@@ -16,7 +16,6 @@ MediaOverlay.propTypes = {
 
   hasMediaStrip: PropTypes.bool,
   collapsibleSidebar: PropTypes.bool,
-  adUrl: PropTypes.string,
 
   // An object with all the labels, or a string specifying the language and region. Accepts: 'en-us', 'pt-br'
   locale: PropTypes.oneOfType([PropTypes.shape(), PropTypes.string]),
diff --git a/src/components/MediaOverlay/Sidebar/Ad/Ad.js b/src/components/MediaOverlay/Sidebar/Ad/Ad.js
index 651f1ed..9665e4d 100644
--- a/src/components/MediaOverlay/Sidebar/Ad/Ad.js
+++ b/src/components/MediaOverlay/Sidebar/Ad/Ad.js
@@ -1,50 +1,13 @@
-import React, { Component } from 'react';
 import PropTypes from 'prop-types';
-import withRouter from '../../../../hocs/withRouter';
+import React from 'react';
 import styles from './Ad.module.scss';
 
-function randomString() {
-  return Math.random()
-    .toString(36)
-    .replace(/[^a-z]+/g, '');
-}
-
-class Ad extends Component {
-  shouldComponentUpdate(nextProps) {
-    const { previousMediaId } = this.props;
-    const nextMediaId = nextProps.params.assemblyId;
-
-    return nextMediaId !== previousMediaId;
-  }
-
-  render() {
-    const { adUrl } = this.props;
-
-    return (
-      <div className={styles.Ad}>
-        <iframe
-          src={`${adUrl}?${randomString()}`}
-          title="advertisement"
-          height="100%"
-          width="100%"
-          scrolling="no"
-        />
-      </div>
-    );
-  }
-}
+const Ad = ({ assemblyId }) => (
+  <div className={styles.Ad} id="media-overlay-ad" data-assembly-id={assemblyId} />
+);
 
 Ad.propTypes = {
-  adUrl: PropTypes.string.isRequired,
-  previousMediaId: PropTypes.string,
-
-  // withRouter props
-
-  params: PropTypes.shape().isRequired,
-};
-
-Ad.defaultProps = {
-  previousMediaId: null,
+  assemblyId: PropTypes.number.isRequired,
 };
 
-export default withRouter(Ad);
+export default Ad;
diff --git a/src/components/MediaOverlay/Sidebar/Ad/Ad.module.scss b/src/components/MediaOverlay/Sidebar/Ad/Ad.module.scss
index 97a6b4f..62ce3fd 100644
--- a/src/components/MediaOverlay/Sidebar/Ad/Ad.module.scss
+++ b/src/components/MediaOverlay/Sidebar/Ad/Ad.module.scss
@@ -2,6 +2,6 @@
 
 .Ad {
   margin-bottom: $overlay-gap;
-  height: 250px;
+  max-height: 250px;
   width: 300px;
 }
diff --git a/src/components/MediaOverlay/Sidebar/Sidebar.js b/src/components/MediaOverlay/Sidebar/Sidebar.js
index fb3cc4d..2561ec9 100644
--- a/src/components/MediaOverlay/Sidebar/Sidebar.js
+++ b/src/components/MediaOverlay/Sidebar/Sidebar.js
@@ -26,8 +26,8 @@ function getSidebarPanel(panel, assembly, localeLabels, CaptionPanelAddons, Cite
 const Sidebar = () => (
   <MediaOverlayContext.Consumer>
     {({ overlayProps, overlayState }) => {
-      const { activeSidebarPanel, isSidebarVisible, localeLabels, assembly, previousMediaId } = overlayState;
-      const { adUrl, CaptionPanelAddons, CitePanelAddons, EmailPanel, SidebarTools } = overlayProps;
+      const { activeSidebarPanel, isSidebarVisible, localeLabels, assembly } = overlayState;
+      const { CaptionPanelAddons, CitePanelAddons, EmailPanel, SidebarTools } = overlayProps;
 
       if (!isSidebarVisible) {
         return null;
@@ -59,11 +59,9 @@ const Sidebar = () => (
 
           {/* Show ads if they're enabled, and if we're on md-lg */}
 
-          {adUrl && (
-            <MediaQuery minWidth={ViewportWidth.LG_MIN}>
-              <Ad previousMediaId={previousMediaId} adUrl={adUrl} />
-            </MediaQuery>
-          )}
+          <MediaQuery minWidth={ViewportWidth.MD_MIN}>
+            <Ad />
+          </MediaQuery>
         </div>
       );
     }}