Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

feat(firstrun): Add fx_firstrun_v2 events to support new firstrun flow. #4717

Merged
merged 2 commits into from
Feb 13, 2017
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
1 change: 1 addition & 0 deletions app/scripts/lib/app-start.js
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ define(function (require, exports, module) {
assertionLibrary: this._assertionLibrary,
iframeChannel: this._iframeChannel,
metrics: this._metrics,
notifier: this._notifier,
oAuthClient: this._oAuthClient,
relier: this._relier,
session: Session,
Expand Down
85 changes: 83 additions & 2 deletions app/scripts/models/auth_brokers/fx-firstrun-v2.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,101 @@ define(function (require, exports, module) {
const _ = require('underscore');
const Constants = require('lib/constants');
const FxFirstrunV1AuthenticationBroker = require('./fx-firstrun-v1');
const NotifierMixin = require('views/mixins/notifier-mixin');
Copy link
Author

Choose a reason for hiding this comment

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

I didn't want to add more than this PR than necessary, but it's time to move this mixin into a shared location so it can be used by either views or models without the wonkiness of including a view mixin in a model. I'll do that as a separate PR.

Copy link
Author

Choose a reason for hiding this comment

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

This move was taken care of by @philbooth in #4718


var proto = FxFirstrunV1AuthenticationBroker.prototype;

var FxFirstrunV2AuthenticationBroker = FxFirstrunV1AuthenticationBroker.extend({
type: 'fx-firstrun-v2',

defaultCapabilities: _.extend({}, proto.defaultCapabilities, {
chooseWhatToSyncCheckbox: false,
chooseWhatToSyncWebV1: {
engines: Constants.DEFAULT_DECLINED_ENGINES
}
}),

type: 'fx-firstrun-v2'
initialize (options = {}) {
proto.initialize.call(this, options);

NotifierMixin.initialize.call(this, options);
},

notifications: {
'form.disabled': '_sendFormDisabled',
'form.enabled': '_sendFormEnabled',
'form.engaged': '_sendFormEngaged',
'show-child-view': '_onShowChildView',
'show-view': '_onShowView'
},

_iframeCommands: _.extend({}, proto._iframeCommands, {
FORM_DISABLED: 'form_disabled',
FORM_ENABLED: 'form_enabled',
FORM_ENGAGED: 'form_engaged',
NAVIGATED: 'navigated'
}),

/**
* Notify the parent the form has been modified.
*
* @private
*/
_sendFormEngaged () {
this._iframeChannel.send(this._iframeCommands.FORM_ENGAGED);
},

/**
* Notify the parent the form has been disabled.
*
* @private
*/
_sendFormDisabled () {
this._iframeChannel.send(this._iframeCommands.FORM_DISABLED);
},

/**
* Notify the parent the form has been enabled.
*
* @private
*/
_sendFormEnabled () {
this._iframeChannel.send(this._iframeCommands.FORM_ENABLED);
},

/**
* Called whenever a View is displayed
*
* @param {Function} View constructor
* @param {String} currentPage - URL being navigated to
* @private
*/
_onShowView (View, { currentPage }) {
this._sendNavigated(currentPage);
},

/**
* Notify the parent a view has been navigated to.
*
* @param {Function} ChildView constructor
* @param {Function} ParentView constructor
* @param {String} currentPage - URL being navigated to
* @private
*/
_onShowChildView (ChildView, ParentView, { currentPage }) {
this._sendNavigated(currentPage);
},

/**
* Notify the parent when the URL pathname has changed
*
* @param {String} url - URL being navigated to
* @private
*/
_sendNavigated (url) {
this._iframeChannel.send(this._iframeCommands.NAVIGATED, { url });
}
});

module.exports = FxFirstrunV2AuthenticationBroker;
});

31 changes: 24 additions & 7 deletions app/scripts/views/form.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ define(function (require, exports, module) {
'submit form': preventDefaultThen('validateAndSubmit')
},

_notifiedOfEngaged: false,
onFormChange () {
// the change event can be called after the form is already
// submitted if the user presses "enter" in the form. If the
Expand All @@ -84,6 +85,11 @@ define(function (require, exports, module) {
this.hideError();
this.hideSuccess();
this.enableSubmitIfValid();

if (! this._notifiedOfEngaged) {
this._notifiedOfEngaged = true;
this.notifier.trigger('form.engaged');
}
},

afterRender () {
Expand Down Expand Up @@ -142,25 +148,36 @@ define(function (require, exports, module) {
},

/**
* TODO - this should be called disableSubmit
* Disable the form
*/
disableForm () {
// the disabled class is used instead of the disabled attribute
// so that the submit handler is still called. With the submit attribute
// applied, no submit handler is fired, and the form validation does not
// take place.
this.$('button[type=submit]').addClass('disabled');
this._isFormEnabled = false;
if (this.isFormEnabled()) {
this.$('button[type=submit]').addClass('disabled');
this.notifier.trigger('form.disabled');
}
},

/**
* Enable the form
*/
enableForm () {
this.$('button[type=submit]').removeClass('disabled');
this._isFormEnabled = true;
if (! this.isFormEnabled()) {
Copy link
Author

Choose a reason for hiding this comment

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

To avoid sending form.disabled and form.enabled every time either of these methods is called (which is pretty often), I only send the messages if a state change occurs.

this.$('button[type=submit]').removeClass('disabled');
this.notifier.trigger('form.enabled');
}
},

_isFormEnabled: true,
/**
* Check if the form is enabled
*
* @returns {Boolean}
*/
isFormEnabled () {
return !! this._isFormEnabled;
return ! this.$('button[type=submit]').hasClass('disabled');
Copy link
Author

Choose a reason for hiding this comment

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

This is modified to checking the DOM instead of relying on an internal variable because sometimes the form is disabled from the template.

},

/**
Expand Down
1 change: 1 addition & 0 deletions app/scripts/views/sign_up.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ define(function (require, exports, module) {
el: this.$('#coppa'),
formPrefill: this._formPrefill,
metrics: this.metrics,
notifier: this.notifier,
shouldFocus: autofocusEl === null,
viewName: this.getViewName()
};
Expand Down
3 changes: 1 addition & 2 deletions app/tests/spec/lib/app-start.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ define(function (require, exports, module) {
beforeEach(function () {
appStart = new AppStart({
history: backboneHistoryMock,
notifier,
router: routerMock,
user: userMock,
window: windowMock
Expand Down Expand Up @@ -921,5 +922,3 @@ define(function (require, exports, module) {
});
});
});


58 changes: 49 additions & 9 deletions app/tests/spec/models/auth_brokers/fx-firstrun-v2.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,73 @@
define(function (require, exports, module) {
'use strict';

const chai = require('chai');
const { assert } = require('chai');
const Constants = require('lib/constants');
const FxFirstrunV2AuthenticationBroker = require('models/auth_brokers/fx-firstrun-v2');
const Notifier = require('lib/channels/notifier');
const NullChannel = require('lib/channels/null');
const sinon = require('sinon');
const WindowMock = require('../../../mocks/window');

var assert = chai.assert;
describe('models/auth_brokers/fx-firstrun-v2', () => {
let broker;
let iframeChannel;
let notifier;
let windowMock;

describe('models/auth_brokers/fx-firstrun-v2', function () {
var broker;
var windowMock;

before(function () {
beforeEach(function () {
notifier = new Notifier();
iframeChannel = new NullChannel();
sinon.spy(iframeChannel, 'send');
windowMock = new WindowMock();

broker = new FxFirstrunV2AuthenticationBroker({
iframeChannel,
notifier,
window: windowMock
});
});

it('has all sync content types', function () {
it('has all sync content types', () => {
assert.equal(broker.defaultCapabilities.chooseWhatToSyncWebV1.engines, Constants.DEFAULT_DECLINED_ENGINES);
});

describe('capabilities', function () {
describe('capabilities', () => {
it('has the `chooseWhatToSyncWebV1` capability by default', function () {
assert.isTrue(broker.hasCapability('chooseWhatToSyncWebV1'));
});
});

describe('notifications', () => {
function testNotificationCausesSend(notification, expectedSentMessage) {
assert.isFalse(iframeChannel.send.calledWith(expectedSentMessage));
notifier.trigger(notification);
assert.isTrue(iframeChannel.send.calledWith(expectedSentMessage));
}

it('form.engaged sends a form_engaged message to the iframe parent', () => {
testNotificationCausesSend('form.engaged', 'form_engaged');
});

it('form.disabled sends a form_disabled message to the iframe parent', () => {
testNotificationCausesSend('form.disabled', 'form_disabled');
});

it('form.disabled sends a form_disabled message to the iframe parent', () => {
testNotificationCausesSend('form.enabled', 'form_enabled');
});

it('show-view sends a `navigated` message to the iframe parent', () => {
assert.isFalse(iframeChannel.send.calledWith('navigated'));
notifier.trigger('show-view', null, { currentPage: 'signin' });
assert.isTrue(iframeChannel.send.calledWith('navigated', { url: 'signin' }));
});

it('show-child-view sends a `navigated` message to the iframe parent', () => {
assert.isFalse(iframeChannel.send.calledWith('navigated'));
notifier.trigger('show-child-view', null, null, { currentPage: 'signup' });
assert.isTrue(iframeChannel.send.calledWith('navigated', { url: 'signup' }));
});
});
});
});
12 changes: 6 additions & 6 deletions app/tests/spec/views/coppa/coppa-age-input.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,26 @@ define(function (require, exports, module) {
'use strict';

const $ = require('jquery');
const { assert } = require('chai');
const AuthErrors = require('lib/auth-errors');
const chai = require('chai');
const FormPrefill = require('models/form-prefill');
const Metrics = require('lib/metrics');
const Notifier = require('lib/channels/notifier');
const sinon = require('sinon');
const TestHelpers = require('../../../lib/helpers');
const View = require('views/coppa/coppa-age-input');
const KeyCodes = require('lib/key-codes');

var assert = chai.assert;

describe('views/coppa/coppa-age-input', function () {
var view;
var formPrefill;
var metrics;
var view;

function createView() {
view = new View({
formPrefill: formPrefill,
metrics: metrics,
formPrefill,
metrics,
notifier: new Notifier(),
viewName: 'signup'
});
}
Expand Down
Loading