Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Stop multiple refresh on mq change #11

Merged
merged 3 commits into from
Jan 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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