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

[DRAFT] <amp-embed type="insticator"> #23928

Merged
merged 54 commits into from
Oct 9, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
ce8a869
UNSTABLE: added new amp-embed type, insticator
ortemcheg Aug 2, 2019
5360271
Merged in updates
ortemcheg Aug 7, 2019
39a0491
Merge branch 'master' into amp-embed-type-insticaor-initial-setup
ortemcheg Aug 13, 2019
6384f77
Merge pull request #5 from Insticator/amp-embed-type-insticaor-initia…
vladdeev Aug 13, 2019
4b12980
Utilizing our standard header and body code to generate ads (instead …
triso07 Aug 28, 2019
f154c66
Commenting out extraneous helper functions now that we're generating …
triso07 Aug 28, 2019
285ec22
commented out logs
triso07 Aug 28, 2019
793d332
Merge pull request #6 from Insticator/sandbox
triso07 Aug 28, 2019
732d146
- Added site id param
triso07 Aug 28, 2019
e8ff916
further cleaning the code
triso07 Aug 28, 2019
232585e
Merge pull request #7 from Insticator/sandbox
triso07 Aug 28, 2019
6bf457a
Changed append location of markup to be more inline with other code. …
triso07 Aug 28, 2019
bef3245
minor text change to documentation
triso07 Aug 28, 2019
50d46fb
hooking up to available API and passing top level domain to insticato…
triso07 Aug 28, 2019
461cba4
Merge pull request #8 from Insticator/sandbox
triso07 Aug 28, 2019
a94add8
Passing AMP embed API to insticator code so we can resize when needed
triso07 Sep 4, 2019
5902f0b
Merge pull request #9 from Insticator/sandbox
triso07 Sep 4, 2019
1d884c4
initial round of AMP requested adjustments to our code
triso07 Sep 11, 2019
1e83788
Merge pull request #10 from Insticator/sandbox
triso07 Sep 11, 2019
78209cf
No longer creating script tags then appending them, instead just exec…
triso07 Sep 11, 2019
6c6ff83
Merge pull request #11 from Insticator/sandbox
triso07 Sep 11, 2019
3217f7e
removed unnecessary function
triso07 Sep 11, 2019
7c5b757
Merge pull request #12 from Insticator/sandbox
triso07 Sep 11, 2019
c15d775
removed extraneous helper function
triso07 Sep 11, 2019
b00b29a
Merge pull request #13 from Insticator/sandbox
triso07 Sep 11, 2019
b1e86f7
changed var to const
triso07 Sep 11, 2019
a7a6f79
Merge pull request #14 from Insticator/sandbox
triso07 Sep 11, 2019
7a18c14
Removing unnecessary library call. Using dummy embed instead of real …
triso07 Sep 12, 2019
eb877b8
Merge pull request #15 from Insticator/sandbox
triso07 Sep 12, 2019
ca32ebe
Implemented renderStart method per AMP request
triso07 Sep 16, 2019
253be0d
Merge pull request #16 from Insticator/sandbox
triso07 Sep 16, 2019
9b14ed6
Fixed all reported linting issues
triso07 Oct 1, 2019
b3b6a4e
Merge pull request #17 from Insticator/sandbox
triso07 Oct 1, 2019
80d2610
Embed was not loading correctly before. Using a to reference Insticat…
triso07 Oct 1, 2019
12cf618
Merge pull request #18 from Insticator/sandbox
triso07 Oct 1, 2019
5d092de
Updated Copyright year
triso07 Oct 1, 2019
06f9928
Merge pull request #19 from Insticator/sandbox
triso07 Oct 1, 2019
6d54912
removed loadScript (thought this was necessary from AMPs side)
triso07 Oct 1, 2019
ce4f748
Merge pull request #20 from Insticator/sandbox
triso07 Oct 1, 2019
9df25d4
removed import for loadScript since no longer using it
triso07 Oct 1, 2019
7d9f9bb
added /*OK*/ to template as requested
triso07 Oct 1, 2019
2e1361c
Merge pull request #21 from Insticator/sandbox
triso07 Oct 1, 2019
b73a7e3
Removed URL helper var since no longer being used in multiple places.…
triso07 Oct 1, 2019
d184d3a
Using /*OK*/ for innerHTML as AMP requires this
triso07 Oct 3, 2019
44f0cae
Merge pull request #22 from Insticator/sandbox
triso07 Oct 3, 2019
328d501
Starting at 850 as that is our standard for embed with ads (resize wi…
triso07 Oct 3, 2019
e43fb55
Merge pull request #23 from Insticator/sandbox
triso07 Oct 3, 2019
cc27047
Capitalized E in element
triso07 Oct 4, 2019
fa492de
using bracket notation to reference data attributes per AMP request
triso07 Oct 4, 2019
feb6772
Additional bracket instead of dot notation reference
triso07 Oct 4, 2019
eeb8e18
forgot the tick marks when accessing property via brackets
triso07 Oct 4, 2019
10ae65b
Merge pull request #24 from Insticator/sandbox
triso07 Oct 4, 2019
11edc2f
Another NIT linter issue
triso07 Oct 9, 2019
8eed75d
Merge pull request #25 from Insticator/sandbox
triso07 Oct 9, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions 3p/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ import {imonomy} from '../ads/imonomy';
import {improvedigital} from '../ads/improvedigital';
import {inmobi} from '../ads/inmobi';
import {innity} from '../ads/innity';
import {insticator} from '../ads/insticator';
import {invibes} from '../ads/invibes';
import {ix} from '../ads/ix';
import {jubna} from '../ads/jubna';
Expand Down Expand Up @@ -285,6 +286,7 @@ const AMP_EMBED_ALLOWED = {
epeex: true,
forkmedia: true,
idealmedia: true,
insticator: true,
jubna: true,
kuadio: true,
'mantis-recommend': true,
Expand Down Expand Up @@ -418,6 +420,7 @@ register('improvedigital', improvedigital);
register('industrybrains', industrybrains);
register('inmobi', inmobi);
register('innity', innity);
register('insticator', insticator);
register('invibes', invibes);
register('ix', ix);
register('jubna', jubna);
Expand Down
8 changes: 8 additions & 0 deletions ads/_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,14 @@ const adConfig = jsonConfiguration({
renderStartImplemented: true,
},

'insticator': {
prefetch: 'https://testthisshit.online/amp-embed-lib/insticator.js',
preconnect: [
'https://drhn9v8cwg89y.cloudfront.net',
'https://d3lcz8vpax4lo2.cloudfront.net',
]
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you please also implement renderStart API?

in your remote loaded insticator.js, it should call context.renderStart once content is ready to show. This basically gives AMP runtime signal when to hide loading indicator and show your content.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renderStart implemented

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍verified amp runtime is getting this signal.


'invibes': {
prefetch: 'https://k.r66net.com/GetAmpLink',
renderStartImplemented: true,
Expand Down
91 changes: 91 additions & 0 deletions ads/insticator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// !!!!!!!!!! ---------- AMP EMBED - INSTICATOR ---------- !!!!!!!!!! //
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add the amp license header to this file.


import { validateData, loadScript } from '../3p/3p';

// ------- HELPER VARIABLES ------- //
//reusable URL references to ads, embed and the library
//Don't forget to preconnect and prefetch as it's described in the docs
//https://github.com/ampproject/amphtml/blob/master/ads/README.md
const url = {
ads: 'https://drhn9v8cwg89y.cloudfront.net',
embed: 'https://d3lcz8vpax4lo2.cloudfront.net',
lib: 'https://testthisshit.online/amp-embed-lib/insticator.js'
};

// ------- HELPER FUNCTIONS ------- //
function createInitialMarkup() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for flexibility in a long run, I suggest you only leave config related logic in this file, and move logic to /amp-embed-lib/insticator.js as much as possible.

return `
<div id="insticator-container">
<div id="div-insticator-ad-1"></div>
<div id="insticator-embed">
<iframe id="insticator-iframe" scrolling="no" frameborder="0" allowtransparency="true"></iframe>
</div>
<div id="div-insticator-ad-2"></div>
</div>
`;
}

function createTemplate(domString) {
const template = document.createElement('template');
template.innerHTML = domString;
return template.content;
}

function appendElement(location, el) {
location.appendChild(el);
}

function createElement(location, el, attrs) {
const newEl = location.createElement(el);
Object.entries(attrs).forEach(attr => newEl.setAttribute(attr[0], attr[1]));
return newEl;
}

function getRequest(file, callback, componentContainer) {
const xhr = new XMLHttpRequest();
xhr.onreadystatechange = () => xhr.readyState == 4 && xhr.status == 200 ? callback(JSON.parse(xhr.responseText), componentContainer) : null;
xhr.open('GET', file, true);
xhr.send();
}

function appendAds(ads, componentContainer) {
Object.entries(ads).forEach(ad => appendElement(componentContainer.querySelector(`#div-insticator-ad-${ad[0][ad[0].length -1]}`), createElement(componentContainer.ownerDocument, 'amp-ad', ad[1])));
}

/*
* @param {!Object} componentContainer
*/
function createComponent(componentContainer) {
// get data attribute from the amp-insticator tag
const embedId = componentContainer.getAttribute('data-embed-id');

// store DOM elements
const insticatorContainer = componentContainer.querySelector('#insticator-container');
const embedIframe = componentContainer.querySelector('#insticator-iframe');
const embedApp = createElement(componentContainer.ownerDocument, 'div', {
id: 'app'
});
const embedScript = createElement(componentContainer.ownerDocument, 'script', {
type: 'text/javascript',
src: `${url.embed}/embed-code/${embedId}.js`
});

// append content to iframe
appendElement(embedIframe.contentWindow.document.body, embedApp);
appendElement(embedIframe.contentWindow.document.head, embedScript);
// append ads
getRequest(`${url.ads}/test/ad_settings/${embedId}.js`, (ads, componentContainer) => appendAds(ads, componentContainer), componentContainer);
}

/*
* @param {!Window} global
* @param {!Object} data
*/
export function insticator(global, data) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code style: public function at top before all private functions

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

validateData(data, ['embedId']);
const componentContainer = global.document.getElementById('c');
componentContainer.setAttribute('data-embed-id', data.embedId);
appendElement(componentContainer, createTemplate(createInitialMarkup()));
createComponent(componentContainer);
loadScript(global, url.lib);
}
35 changes: 35 additions & 0 deletions ads/insticator.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<!---
Copyright 2019 The AMP HTML Authors. All Rights Reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS-IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->

# Insticator

## Example of Insticator's widget implementation


```html
<amp-embed
type="insticator"
data-embed-id="ACCOUNT-ID">
</amp-embed>
```

## Configuration

For details on the configuration semantics, please contact [Insticator](https://www.insticator.com/contact/).

### Required parameters

- `data-embed-id`: Uniquie ID of your Insticator account
6 changes: 6 additions & 0 deletions examples/ads.amp.html
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@
<option>industrybrains</option>
<option>inmobi</option>
<option>innity</option>
<option>insticator</option>
<option>invibes</option>
<option>ix</option>
<option>jubna</option>
Expand Down Expand Up @@ -1264,6 +1265,11 @@ <h2>Innity</h2>
data-channel="NEWS">
</amp-ad>

<h2>Insticator</h2>
<amp-embed type="insticator" width="450" height="750"
data-embed-id="5bf0548f-a332-4934-9d35-c9ca9e93d4ff">
</amp-embed>

<h2>Invibes</h2>
<amp-ad width="300" height="250"
type="invibes"
Expand Down
1 change: 1 addition & 0 deletions extensions/amp-ad/amp-ad.md
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ See [amp-ad rules](https://github.com/ampproject/amphtml/blob/master/extensions/
- [Imonomy](../../ads/imonomy.md)
- [Imedia](../../ads/imedia.md)
- [Improve Digital](../../ads/improvedigital.md)
- [Insticator](../../ads/insticator.md)
- [Index Exchange](../../ads/ix.md)
- [Industrybrains](../../ads/industrybrains.md)
- [InMobi](../../ads/inmobi.md)
Expand Down