feat(initial): initial bare bones implementation #2
Conversation
no unit tests currently in place
| { | ||
| "preset": "google", | ||
| "maximumLineLength": null, | ||
| "validateQuoteMarks": { "mark": "'", "escape": true } |
There was a problem hiding this comment.
Does this mean we don't use "s even if we've got to escape?
There was a problem hiding this comment.
per http://jscs.info/rule/validateQuoteMarks:
validateQuoteMarks
Requires all quote marks to be either the supplied value, or consistent if true
Types: Boolean, String or Object
Values:
- """: all strings require double quotes
- "'": all strings require single quotes
- true: all strings require the quote mark first encountered in the source code
- Object:
- escape: allow the "other" quote mark to be used, but only to avoid having to escape
- mark: the same effect as the non-object values
- ignoreJSX: ignore JSX nodes
| @@ -0,0 +1,3 @@ | |||
| /** | |||
| * @typedef {SingleSize|MultiSize} GeneralSize | |||
There was a problem hiding this comment.
Would be easier to understand if we had a description of purpose here.
| * queue callbacks for when GPT is ready. These callbacks do not have to check | ||
| * {@link #apiReady} as they are guaranteed to execute once the API is set up. | ||
| * | ||
| * @property pubadsReady {boolean|undefined} Flag indicating that Pubads service |
There was a problem hiding this comment.
Nit - you've flipped the order of the type and name
| * This property will be simply `undefined` until {@link #enableServices()} | ||
| * is called and Pubads service is loaded and initialized. | ||
| * | ||
| * @property cmd {!Array<function()>|!CommandArray} Reference to the global |
| this._addService(new CompanionAdsService(this)); | ||
| this._addService(new ContentService(this)); | ||
| this._addService(new PubAdsService(this)); | ||
| this._title = null; |
There was a problem hiding this comment.
Weird that this is null and others undefined. Guessing that's just how it is in the real one.
| * | ||
| * @param {string} key The targeting key to look for. | ||
| * @returns {!Array<string>} The values associated with this key, or an empty | ||
| * array if there is no such key. |
There was a problem hiding this comment.
Nit - technically it returns an empty array if the value is undefined.
| } | ||
|
|
||
| _getTargetingMap() { | ||
| return this._targeting; |
|
|
||
| /** | ||
| * Sets the slot-level preferences for SafeFrame configuration. Any unrecognized | ||
| * keys in the config object will be ignored. The entire config will be ignored |
| @@ -0,0 +1,17 @@ | |||
| /** | |||
| * This event is fired when a slot on the page has finished rendering. The event | |||
| * is fired by the service that rendered the slot. Example: To listen to | |||
| @@ -2,3 +2,5 @@ import GoogleTag from './GoogleTag.js'; | |||
|
|
|||
| module.exports = new GoogleTag(); | |||
|
|
|||
| // Load this 1ms after this. | |||
There was a problem hiding this comment.
Nit - this should really say Defer loading until next tick. setTimeout just tries to ensure that at least that amount of time has passed
There was a problem hiding this comment.
I think I'll actually just return the class from the library rather than an instance, so the caller can control when the _load event happens.
|
LGTM just nits |
|
Merging on LGTM |
What does this PR do?
This is the first "real" implementation of the mock library. There's still some work to do, namely actually putting in place a unit test framework for the mock library itself. However, this does some things and adheres to the API documented here.
Why is this change being made?
This is the first step in implementing the code.
Where should the reviewer start?
GoogleTag.js
How was this tested? How can the reviewer verify your testing?
Untested currently :-(
What gif best describes this PR or how it makes you feel?
Completion checklist