Skip to content

Commit

Permalink
Merge pull request #1499 from erwinmombay/remove-from-canary
Browse files Browse the repository at this point in the history
remove amp-user-notification from experiment
  • Loading branch information
erwinmombay committed Jan 21, 2016
2 parents af6015e + 87a13cb commit 7092547
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 46 deletions.
52 changes: 19 additions & 33 deletions extensions/amp-user-notification/0.1/amp-user-notification.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,13 @@ import {assertHttpsUrl, addParamsToUrl} from '../../../src/url';
import {assert} from '../../../src/asserts';
import {cidFor} from '../../../src/cid';
import {getService} from '../../../src/service';
import {isExperimentOn} from '../../../src/experiments';
import {log} from '../../../src/log';
import {urlReplacementsFor} from '../../../src/url-replacements';
import {viewerFor} from '../../../src/viewer';
import {whenDocumentReady} from '../../../src/document-state';
import {xhrFor} from '../../../src/xhr';


/** @const */
const EXPERIMENT = 'amp-user-notification';

/**
* @export
* @typedef {{
Expand Down Expand Up @@ -82,14 +78,6 @@ class NotificationInterface {
*/
export class AmpUserNotification extends AMP.BaseElement {

/**
* @return {boolean}
* @private
*/
isExperimentOn_() {
return isExperimentOn(this.getWin(), EXPERIMENT);
}

/** @override */
buildCallback() {

Expand All @@ -110,33 +98,31 @@ export class AmpUserNotification extends AMP.BaseElement {
this.dialogResolve_ = resolve;
});

if (this.isExperimentOn_()) {
/** @private @const {!UserNotificationManager} */
this.userNotificationManager_ = getUserNotificationManager_(this.win_);
/** @private @const {!UserNotificationManager} */
this.userNotificationManager_ = getUserNotificationManager_(this.win_);

this.elementId_ = assert(this.element.id,
'amp-user-notification should have an id.');
this.elementId_ = assert(this.element.id,
'amp-user-notification should have an id.');

assert(this.element.hasAttribute('data-show-if-href'),
`"amp-user-notification" (${this.elementId_}) ` +
'should have "data-show-if-href" attribute.');
/** @private @const {string} */
this.showIfHref_ = assertHttpsUrl(
this.element.getAttribute('data-show-if-href'), this.element);
assert(this.element.hasAttribute('data-show-if-href'),
`"amp-user-notification" (${this.elementId_}) ` +
'should have "data-show-if-href" attribute.');
/** @private @const {string} */
this.showIfHref_ = assertHttpsUrl(
this.element.getAttribute('data-show-if-href'), this.element);

assert(this.element.hasAttribute('data-dismiss-href'),
`"amp-user-notification" (${this.elementId_}) ` +
'should have "data-dismiss-href" attribute.');
assert(this.element.hasAttribute('data-dismiss-href'),
`"amp-user-notification" (${this.elementId_}) ` +
'should have "data-dismiss-href" attribute.');

/** @private @const {string} */
this.dismissHref_ = assertHttpsUrl(
this.element.getAttribute('data-dismiss-href'), this.element);
/** @private @const {string} */
this.dismissHref_ = assertHttpsUrl(
this.element.getAttribute('data-dismiss-href'), this.element);

this.userNotificationManager_
.registerUserNotification(this.elementId_, this);
this.userNotificationManager_
.registerUserNotification(this.elementId_, this);

this.registerAction('dismiss', this.dismiss.bind(this));
}
this.registerAction('dismiss', this.dismiss.bind(this));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ describe('amp-user-notification', () => {
let stub;
let stub1;
let stub2;
let notifStub;
let dftAttrs;

function getUserNotification(attrs = {}) {
Expand Down Expand Up @@ -57,13 +56,9 @@ describe('amp-user-notification', () => {
'data-dismiss-href': 'https://www.ampproject.org/post/here',
'layout': 'nodisplay',
};
notifStub = sinon.stub(AmpUserNotification.prototype, 'isExperimentOn_')
.returns(true);
});

afterEach(() => {
notifStub.restore();

if (stub) {
stub.restore();
stub = null;
Expand Down
8 changes: 0 additions & 8 deletions tools/experiments/experiments.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,6 @@ const EXPERIMENTS = [
'amp-access/amp-access-spec.md',
},

// Amp User Notification
{
id: 'amp-user-notification',
name: 'Amp User Notification',
spec: 'https://github.com/ampproject/amphtml/blob/master/extensions/' +
'amp-user-notification/amp-user-notification.md',
},

// Dynamic CSS Classes
{
id: 'dynamic-css-classes',
Expand Down

0 comments on commit 7092547

Please sign in to comment.