Put things where they belong

ampproject · Jul 19, 2018 · b5c9c86 · b5c9c86
1 parent 5b57fcf
commit b5c9c86
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 86 deletions.
diff --git a/extensions/amp-lightbox/0.1/amp-lightbox.js b/extensions/amp-lightbox/0.1/amp-lightbox.js
@@ -28,12 +28,9 @@ import {debounce} from '../../../src/utils/rate-limit';
 import {dev, user} from '../../../src/log';
 import {dict, hasOwn} from '../../../src/utils/object';
 import {getMode} from '../../../src/mode';
+import {htmlFor} from '../../../src/static-template';
 import {isInFie} from '../../../src/friendly-iframe-embed';
 import {removeElement, tryFocus} from '../../../src/dom';
-import {
-  renderCloseButtonHeader,
-  showCloseButtonHeader,
-} from '../../../src/full-overlay-frame-helper';
 import {toArray} from '../../../src/types';
 
 /** @const {string} */
@@ -76,6 +73,26 @@ const AnimationPresets = {
 /** @private @const {string} */
 const DEFAULT_ANIMATION = 'fade-in';
 
+/**
+ * @param {!Element} ctx
+ * @return {!Element}
+ */
+function renderCloseButtonHeader(ctx) {
+  return htmlFor(ctx)`
+    <i-amphtml-ad-close-header role=button tabindex=0 aria-label="Close Ad">
+      <div>Ad</div>
+      <i-amphtml-ad-close-button class="amp-ad-close-button">
+      </i-amphtml-ad-close-button>
+    </i-amphtml-ad-close-header>`;
+}
+
+/**
+ * @param {!Element} header
+ */
+function showCloseButtonHeader(header) {
+  header.classList.add('amp-ad-close-header');
+}
+
 class AmpLightbox extends AMP.BaseElement {
 
   /** @param {!AmpElement} element */

diff --git a/src/full-overlay-frame-helper.js b/src/full-overlay-frame-helper.js
@@ -93,25 +93,3 @@ export function collapseFrameUnderVsyncMutate(iframe) {
     'border',
   ]);
 }
-
-
-/**
- * @param {!Element} ctx
- * @return {!Element}
- * @visibleForTesting
- */
-export function renderCloseButtonHeader(ctx) {
-  return htmlFor(ctx)`
-    <i-amphtml-ad-close-header role=button tabindex=0 aria-label="Close Ad">
-      <div>Ad</div>
-      <i-amphtml-ad-close-button class="amp-ad-close-button">
-      </i-amphtml-ad-close-button>
-    </i-amphtml-ad-close-header>`;
-}
-
-/**
- * @param {!Element} header
- */
-export function showCloseButtonHeader(header) {
-  header.classList.add('amp-ad-close-header');
-}
diff --git a/test/functional/test-friendly-iframe-embed.js b/test/functional/test-friendly-iframe-embed.js
@@ -16,13 +16,11 @@
 
 import * as sinon from 'sinon';
 
-import {ActionTrust} from '../../src/action-constants';
 import {
   FriendlyIframeEmbed,
   getFriendlyIframeEmbedOptional,
   installFriendlyIframeEmbed,
   mergeHtmlForTesting,
-  renderClickableCloseButtonHeader,
   setFriendlyIframeEmbedVisible,
   setSrcdocSupportedForTesting,
   whenContentIniLoad,
@@ -771,17 +769,14 @@ describe('friendly-iframe-embed', () => {
     it('resizes body and fixed container when entering', function* () {
       const bodyElementMock = document.createElement('div');
       const fie = createFie(bodyElementMock);
-      const headerHeight = 60;
-
-      sandbox.stub(fie, 'getHeaderHeight_').returns(headerHeight);
 
       yield fie.enterFullOverlayMode();
 
       expect(bodyElementMock.style.background).to.equal('transparent');
       expect(bodyElementMock.style.position).to.equal('absolute');
       expect(bodyElementMock.style.width).to.equal(`${w}px`);
-      expect(bodyElementMock.style.height).to.equal(`${h - headerHeight}px`);
-      expect(bodyElementMock.style.top).to.equal(`${y - headerHeight}px`);
+      expect(bodyElementMock.style.height).to.equal(`${h}px`);
+      expect(bodyElementMock.style.top).to.equal(`${y}px`);
       expect(bodyElementMock.style.left).to.equal(`${x}px`);
       expect(bodyElementMock.style.right).to.equal('auto');
       expect(bodyElementMock.style.bottom).to.equal('auto');
@@ -791,10 +786,10 @@ describe('friendly-iframe-embed', () => {
       expect(iframe.style.position).to.equal('fixed');
       expect(iframe.style.left).to.equal('0px');
       expect(iframe.style.right).to.equal('0px');
-      expect(iframe.style.top).to.equal(`${headerHeight}px`);
+      expect(iframe.style.top).to.equal('0px');
       expect(iframe.style.bottom).to.equal('0px');
       expect(iframe.style.width).to.equal('100vw');
-      expect(iframe.style.height).to.equal(`calc(100vh - ${headerHeight}px)`);
+      expect(iframe.style.height).to.equal('100vh');
     });
 
     it('should reset body and fixed container when leaving', function* () {
@@ -823,55 +818,4 @@ describe('friendly-iframe-embed', () => {
       expect(iframe.style.height).to.be.empty;
     });
   });
-
-  describe('#renderClickableCloseButtonHeader', () => {
-
-    const win = document.defaultView;
-
-    it('renders', () => {
-      const ampAdParentMock = document.createElement('div');
-      const ampLightboxMock = document.createElement('div');
-
-      const el = renderClickableCloseButtonHeader(
-          win, ampAdParentMock, ampLightboxMock);
-
-      expect(el.tagName.toLowerCase()).to.equal('i-amphtml-ad-close-header');
-      expect(el.getAttribute('role')).to.equal('button');
-      expect(el.getAttribute('aria-label')).to.equal('Close Ad');
-      expect(el.getAttribute('tabindex')).to.equal('0');
-
-      expect(el.firstElementChild.textContent).to.equal('Ad');
-
-      const closeButton = el.lastElementChild;
-
-      expect(closeButton.tagName.toLowerCase())
-          .to.equal('i-amphtml-ad-close-button');
-
-      expect(closeButton).to.have.class('amp-ad-close-button');
-    });
-
-    it('triggers action', () => {
-      const ampAdParentMock = document.createElement('div');
-      const ampLightboxMock = document.createElement('div');
-
-      const el = renderClickableCloseButtonHeader(
-          win, ampAdParentMock, ampLightboxMock);
-
-      const execute = sandbox.spy();
-      const actionServiceMock = {execute};
-
-      sandbox.stub(Services, 'actionServiceForDoc').returns(actionServiceMock);
-
-      el.click();
-
-      expect(execute.withArgs(
-          /* target */ ampLightboxMock,
-          /* method */ 'close',
-          /* args   */ sinon.match.any,
-          /* source */ ampAdParentMock,
-          /* caller */ ampAdParentMock,
-          /* event  */ sinon.match.any,
-          /* trust  */ ActionTrust.HIGH)).to.be.calledOnce;
-    });
-  });
 });