Skip to content
This repository has been archived by the owner. 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

Conversation

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Feb 13, 2017

Add several new events that are reported to the parent IFRAME:

  • navigated called whenever the user opens a new page
  • form_enabled called whenever the form is enabled
  • form_disabled called whenever the form is disabled
  • form_engaged called whenever the user modifies an input element

A lot of unit test suites needed to be updated to create the Views with
a notifier reference.

fixes https://github.com/mozilla/fxa-bugzilla-mirror/issues/239

@vladikoff - r?

@@ -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');

This comment has been minimized.

@shane-tomlinson

shane-tomlinson Feb 13, 2017
Author Member

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.

This comment has been minimized.

@shane-tomlinson

shane-tomlinson Feb 13, 2017
Author Member

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

enableForm () {
this.$('button[type=submit]').removeClass('disabled');
this._isFormEnabled = true;
if (! this.isFormEnabled()) {

This comment has been minimized.

@shane-tomlinson

shane-tomlinson Feb 13, 2017
Author Member

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.

isFormEnabled () {
return !! this._isFormEnabled;
return ! this.$('button[type=submit]').hasClass('disabled');

This comment has been minimized.

@shane-tomlinson

shane-tomlinson Feb 13, 2017
Author Member

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

@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Feb 13, 2017

The failing travis tests are the /legal/privacy server tests that @vladikoff fixed in #4713

vladikoff and others added 2 commits Feb 9, 2017
Add several new events that are reported to the parent IFRAME:

* `navigated` called whenever the user opens a new page
* `form_enabled` called whenever the form is enabled
* `form_disabled` called whenever the form is disabled
* `form_engaged` called whenever the user modifies an input element

A lot of unit test suites needed to be updated to create the Views with
a notifier reference.
@vladikoff vladikoff force-pushed the more-firstrun-events branch from da707e3 to 836eaab Feb 13, 2017
@vladikoff vladikoff merged commit f1373ed into train-79 Feb 13, 2017
4 checks passed
4 checks passed
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 98.483%
Details
@ericawright
Copy link

@ericawright ericawright commented Feb 13, 2017

If it's relevant, I don't need the form_engaged message for mozilla/bedrock#4664

@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Feb 14, 2017

Thanks @ericawright, I'll keep the extra event in for now - the code is in production.

@shane-tomlinson shane-tomlinson deleted the more-firstrun-events branch Feb 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants