Skip to content

Commit

Permalink
Rename viewportCallbackTemp_ --> viewportCallback (#36245)
Browse files Browse the repository at this point in the history
  • Loading branch information
samouri committed Oct 5, 2021
1 parent 467d732 commit cf71ea8
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 21 deletions.
5 changes: 2 additions & 3 deletions extensions/amp-a4a/0.1/amp-a4a.js
Original file line number Diff line number Diff line change
Expand Up @@ -1316,7 +1316,7 @@ export class AmpA4A extends AMP.BaseElement {
return this.attemptToRenderCreative().then(() => {
this.unobserveIntersections_ = observeIntersections(
this.element,
({isIntersecting}) => this.viewportCallbackTemp(isIntersecting)
({isIntersecting}) => this.viewportCallback(isIntersecting)
);
});
}
Expand Down Expand Up @@ -1500,12 +1500,11 @@ export class AmpA4A extends AMP.BaseElement {
}
}

// TODO: Rename to viewportCallback once BaseElement.viewportCallback has been removed.
/**
* @param {boolean} inViewport
* @protected
*/
viewportCallbackTemp(inViewport) {
viewportCallback(inViewport) {
if (this.xOriginIframeHandler_) {
this.xOriginIframeHandler_.viewportCallback(inViewport);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1234,8 +1234,8 @@ export class AmpAdNetworkDoubleclickImpl extends AmpA4A {
}

/** @override */
viewportCallbackTemp(inViewport) {
super.viewportCallbackTemp(inViewport);
viewportCallback(inViewport) {
super.viewportCallback(inViewport);
if (this.reattemptToExpandFluidCreative_ && !inViewport) {
// If the initial expansion attempt failed (e.g., the slot was within the
// viewport), then we will re-attempt to expand it here whenever the slot
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,9 +365,9 @@ describes.realWin('DoubleClick Fast Fetch Fluid', realWinConfig, (env) => {
impl.isVerifiedAmpCreative_ = true;
impl.reattemptToExpandFluidCreative_ = true;
// Should do nothing
impl.viewportCallbackTemp(true);
impl.viewportCallback(true);
expect(attemptChangeHeightStub).to.not.be.called;
impl.viewportCallbackTemp(false);
impl.viewportCallback(false);
expect(attemptChangeHeightStub).to.be.calledOnce;
});

Expand Down
7 changes: 2 additions & 5 deletions extensions/amp-carousel/0.1/base-carousel.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,11 @@ export class BaseCarousel extends AMP.BaseElement {
this.setControlsState();
}

// TODO(samouri): rename to viewportCallback once
// BaseElement.viewportCallback is deleted

/**
* @param {boolean} inViewport
* @protected
*/
viewportCallbackTemp(inViewport) {
viewportCallback(inViewport) {
if (inViewport) {
this.hintControls();
}
Expand Down Expand Up @@ -236,7 +233,7 @@ export class BaseCarousel extends AMP.BaseElement {
layoutCallback() {
this.unobserveIntersections_ = observeIntersections(
this.element,
({isIntersecting}) => this.viewportCallbackTemp(isIntersecting)
({isIntersecting}) => this.viewportCallback(isIntersecting)
);
return Promise.resolve();
}
Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-carousel/0.1/scrollable-carousel.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export class AmpScrollableCarousel extends BaseCarousel {
layoutCallback() {
this.unobserveIntersections_ = observeIntersections(
this.element,
({isIntersecting}) => this.viewportCallbackTemp(isIntersecting)
({isIntersecting}) => this.viewportCallback(isIntersecting)
);

this.doLayout_(this.pos_);
Expand All @@ -119,8 +119,8 @@ export class AmpScrollableCarousel extends BaseCarousel {
}

/** @override */
viewportCallbackTemp(inViewport) {
super.viewportCallbackTemp(inViewport);
viewportCallback(inViewport) {
super.viewportCallback(inViewport);
this.updateInViewport_(this.pos_, this.pos_);
}

Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-carousel/0.1/slidescroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,8 @@ export class AmpSlideScroll extends BaseCarousel {
}

/** @override */
viewportCallbackTemp(inViewport) {
super.viewportCallbackTemp(inViewport);
viewportCallback(inViewport) {
super.viewportCallback(inViewport);
if (inViewport) {
this.autoplay_();
} else {
Expand Down Expand Up @@ -416,7 +416,7 @@ export class AmpSlideScroll extends BaseCarousel {
layoutCallback() {
this.unobserveIntersections_ = observeIntersections(
this.element,
({isIntersecting}) => this.viewportCallbackTemp(isIntersecting)
({isIntersecting}) => this.viewportCallback(isIntersecting)
);

// TODO(sparhami) #19259 Tracks a more generic way to do this. Remove once
Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-carousel/0.1/test/test-slidescroll-slides.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describes.fakeWin('AmpSlideScroll', {amp: true}, (env) => {
);
viewportCallbackSpy = env.sandbox.spy(
AmpSlideScroll.prototype,
'viewportCallbackTemp'
'viewportCallback'
);
});

Expand Down Expand Up @@ -151,7 +151,7 @@ describes.fakeWin('AmpSlideScroll', {amp: true}, (env) => {
})
);

carousel.viewportCallbackTemp(true);
carousel.viewportCallback(true);
expect(viewportCallbackSpy).to.have.been.calledWith(true);
expect(hintControlsSpy).to.have.been.called;
expect(autoplaySpy).to.have.been.called;
Expand All @@ -166,7 +166,7 @@ describes.fakeWin('AmpSlideScroll', {amp: true}, (env) => {
})
);

carousel.viewportCallbackTemp(false);
carousel.viewportCallback(false);
expect(viewportCallbackSpy).to.have.been.calledWith(false);
expect(hintControlsSpy).to.not.have.been.called;
expect(autoplaySpy).to.not.have.been.called;
Expand Down

0 comments on commit cf71ea8

Please sign in to comment.