Skip to content

Commit

Permalink
fix: Stop multiple refresh on mq change (#11)
Browse files Browse the repository at this point in the history
* fix: Stop multiple refresh on mq change

Stops multiple calls to instance.refresh() on media query change. This can happen when an instance
is associated with multiple instances.

* Only debounce this.refresh

* Fix comment
  • Loading branch information
andrewb authored Jan 22, 2019
1 parent 57c468a commit 3cff653
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 9 deletions.
19 changes: 19 additions & 0 deletions src/Bling.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import React, {Component} from "react";
import PropTypes from "prop-types";
import ReactDOM from "react-dom";
import {debounce} from "throttle-debounce";
import invariant from "invariant";
import deepEqual from "deep-equal";
import hoistStatics from "hoist-non-react-statics";
Expand Down Expand Up @@ -378,6 +379,10 @@ class Bling extends Component {
: Bling._config.viewableThreshold;
}

_debouncedRefresh = debounce(50, () => {
this.refresh();
});

componentDidMount() {
Bling._adManager.addInstance(this);
Bling._adManager
Expand Down Expand Up @@ -490,6 +495,20 @@ class Bling extends Component {
);
}

onMediaQueryChange() {
// Debounce refresh, since it can be called multiple times if there are
// multiple MQ listeners, e.g. the current screen might match
// `(min-width: 1024px)` AND `(min-width: 768px)`, or an MQ might change
// from matching to not matching, while another does the opposite.
// The slot should only be refreshed once per "resize" otherwise there
// might be unnecessary calls to GPT.
this._debouncedRefresh();
if (this.props.onMediaQueryChange) {
// Call for each change event
this.props.onMediaQueryChange(event);
}
}

getRenderWhenViewable(props = this.props) {
return props.renderWhenViewable !== undefined
? props.renderWhenViewable
Expand Down
5 changes: 1 addition & 4 deletions src/createManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,7 @@ export class AdManager extends EventEmitter {

if (viewportWidth && this._mqls[viewportWidth]) {
this._mqls[viewportWidth].listeners.forEach(instance => {
instance.refresh();
if (instance.props.onMediaQueryChange) {
instance.props.onMediaQueryChange(event);
}
instance.onMediaQueryChange(event);
});
}
};
Expand Down
13 changes: 8 additions & 5 deletions test/createManager.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,13 @@ describe("createManager", () => {
props: {
sizeMapping: [{viewport: [0, 0], slot: [320, 50]}]
},
refresh() {}
onMediaQueryChange() {}
};

const instanceRefresh = sinon.stub(instance, "refresh");
const instanceOnMediaQueryChange = sinon.stub(
instance,
"onMediaQueryChange"
);

adManager.addInstance(instance);
adManager._handleMediaQueryChange({
Expand All @@ -319,19 +322,19 @@ describe("createManager", () => {
media: "(min-width: 0px)"
});

expect(instanceRefresh.calledOnce).to.be.true;
expect(instanceOnMediaQueryChange.calledOnce).to.be.true;

// IE
adManager._handleMediaQueryChange({
media: "all and (min-width:0px)"
});

expect(instanceRefresh.calledTwice).to.be.true;
expect(instanceOnMediaQueryChange.calledTwice).to.be.true;

adManager.removeInstance(instance);

refresh.restore();
instanceRefresh.restore();
instanceOnMediaQueryChange.restore();
});

it("debounces render", done => {
Expand Down

0 comments on commit 3cff653

Please sign in to comment.