fix(cad): Fixes based on @vladikoff's feedback.

* Rename `_areSubjectPrereqsMet` to `_isValidSubject`
* Remove the `instructions` class.
* Add docs about views/base.js->hasNavigated
* Fix some typos.
* Remove the temporary strings from strings.js

app/scripts/lib/experiments/grouping-rules/connect-another-device-on-signin.js

@@ -25,7 +25,7 @@ define((require, exports, module) => {
      * @returns {Any}
      */
     choose (subject) {
-      if (! this._areSubjectPrereqsMet(subject)) {
+      if (! this._isValidSubject(subject)) {
         return false;
       }
 
@@ -37,13 +37,13 @@ define((require, exports, module) => {
     }
 
     /**
-     * Are the subject pre-requisites met?
+     * Is the subject valid?
      *
      * @param {Object} subject
      * @returns {Boolean}
      * @private
      */
-    _areSubjectPrereqsMet (subject) {
+    _isValidSubject (subject) {
       return subject &&
              subject.account &&
              subject.account.get('email');

app/scripts/lib/experiments/grouping-rules/email-first.js

@@ -24,7 +24,7 @@ define((require, exports, module) => {
     choose (subject) {
       const GROUPS = ['control', 'treatment'];
 
-      if (! this._areSubjectPrereqsMet(subject)) {
+      if (! this._isValidSubject(subject)) {
         return false;
       } else if (! subject.isEmailFirstSupported) {
         // isEmailFirstSupported is `true` for brokers that support the email-first flow.
@@ -39,13 +39,13 @@ define((require, exports, module) => {
     }
 
     /**
-     * Are the subject pre-requisites met?
+     * Is the subject valid?
      *
      * @param {Object} subject
      * @returns {Boolean}
      * @private
      */
-    _areSubjectPrereqsMet (subject) {
+    _isValidSubject (subject) {
       return subject && subject.uniqueUserId && subject.experimentGroupingRules;
     }
 

app/scripts/lib/strings.js

@@ -45,11 +45,6 @@ define(function (require, exports, module) {
   // translated but have not yet translated the contextualized variant.
   t('Sign in');
 
-  // PR #5408 has these strings
-  t('Signin confirmed');
-  t('Still adding devices?');
-  t('Why is more than one device required?');
-
   /**
    * Replace instances of %s and %(name)s with their corresponding values in
    * the context

app/scripts/templates/connect_another_device.mustache

@@ -15,7 +15,7 @@
     <div class="graphic graphic-connect-another-device">{{#t}}Success{{/t}}</div>
 
     {{#canSignIn}}
-      <p class="instructions">
+      <p>
         {{#t}}Sign in to this Firefox to complete set-up{{/t}}
       </p>
 
@@ -30,7 +30,7 @@
         <!-- User verified in Fx for Android - they are using an old Fennec
         or are already signed in. Ignore old browsers, assume already signed in.
         Encourage installation on another device. -->
-        <p id="connect-other-firefox-from-android" class="instructions">
+        <p id="connect-other-firefox-from-android">
           {{#isSignIn}}
             {{#t}}Still adding devices?{{/t}}
           {{/isSignIn}}
@@ -38,7 +38,7 @@
         </p>
       {{/isFirefoxAndroid}}
       {{#isFirefoxDesktop}}
-        <p id="install-mobile-firefox-desktop" class="instructions">
+        <p id="install-mobile-firefox-desktop">
           {{#isSignIn}}
             {{#t}}Still adding devices?{{/t}}
           {{/isSignIn}}
@@ -47,25 +47,25 @@
       {{/isFirefoxDesktop}}
       {{#isFirefoxIos}}
         <!-- user verifies in Fx for iOS, assume they are not signed in -->
-        <p id="signin-fxios" class="instructions">
+        <p id="signin-fxios">
           {{#t}}Open settings and select Sign in to Firefox to complete set-up{{/t}}
         </p>
       {{/isFirefoxIos}}
       {{#isOtherAndroid}}
         <!-- Another android browser, encourage Fx for Android installation -->
-        <p id="install-mobile-firefox-android" class="instructions">
+        <p id="install-mobile-firefox-android">
           {{#t}}Sign in to Firefox for Android to complete set-up{{/t}}
         </p>
       {{/isOtherAndroid}}
       {{#isOtherIos}}
         <!-- Safari or Chrome for iOS, encourage installation of Fx -->
-        <p id="install-mobile-firefox-ios" class="instructions">
+        <p id="install-mobile-firefox-ios">
           {{#t}}Sign in to Firefox for iOS to complete set-up{{/t}}
         </p>
       {{/isOtherIos}}
       {{#isOther}}
         <!-- probably some desktop browser -->
-        <p id="install-mobile-firefox-other" class="instructions">
+        <p id="install-mobile-firefox-other">
           {{#isSignIn}}
             {{#t}}Still adding devices?{{/t}}
           {{/isSignIn}}

app/scripts/templates/sms_send.mustache

@@ -3,10 +3,10 @@
     <div class="success visible">{{#t}}This Firefox is connected{{/t}}</div>
     <div class="error"></div>
 
-    <section>
+    <section class="send-sms">
       <div class="graphic graphic-connect-another-device">{{#t}}Success{{/t}}</div>
 
-      <p class="instructions">
+      <p>
         {{#isSignIn}}
           {{#t}}Still adding devices?{{/t}}
         {{/isSignIn}}

app/scripts/views/base.js

@@ -216,7 +216,7 @@ define(function (require, exports, module) {
         .then((shouldRender) => {
           // rendering is opt out, should not occur if the view
           // has already navigated.
-          if (shouldRender === false || this._hasNavigated) {
+          if (shouldRender === false || this.hasNavigated()) {
             return false;
           }
 
@@ -236,7 +236,7 @@ define(function (require, exports, module) {
           return this.afterRender();
         })
         .then((shouldDisplay) => {
-          return shouldDisplay !== false && ! this._hasNavigated;
+          return shouldDisplay !== false && ! this.hasNavigated();
         });
     },
 
@@ -824,7 +824,9 @@ define(function (require, exports, module) {
     },
 
     /**
-     * Has the view already navigated?
+     * Has the view navigated? Useful to check when a view should
+     * perform an action, but only if the view hasn't already
+     * navigated.
      *
      * @returns {Boolean}
      */

app/scripts/views/mixins/connect-another-device-mixin.js

@@ -41,10 +41,10 @@ define((require, exports, module) => {
     ],
 
     /**
-     * Is the user eligible for the CAD on signin?
+     * Is `account` eligible for he connect another device on signin?
      *
      * @param {any} account
-     * @returns {Booelean}
+     * @returns {Boolean}
      */
     isEligibleForConnectAnotherDeviceOnSignin (account) {
       const isEligibleForCadOnSignin = !! this.getExperimentGroup('cadOnSignin', { account });

app/tests/spec/views/sms_send.js

@@ -138,7 +138,7 @@
          sinon.stub(view, 'isSignIn').callsFake(() => true);
          return view.render()
            .then(() => {
-             assert.include(view.$('.instructions').text().toLowerCase(), 'still adding devices');
+             assert.include(view.$('.send-sms > p').text().toLowerCase(), 'still adding devices');
            });
        });
      });