From 6eb7106bc5e068d8db7f1ae36494e71cc325924f Mon Sep 17 00:00:00 2001
From: Tomer Shvueli <tomer@shvueli.com>
Date: Thu, 4 Mar 2021 09:58:22 -0500
Subject: [PATCH] fix: Nest sub option in Options page, tie auto fill & auto
 copy features together

---
 src/_locales/en/messages.json             |  5 +----
 src/popup/scss/box.scss                   |  5 +++++
 src/popup/settings/options.component.html | 18 +++++++-----------
 src/popup/settings/options.component.ts   |  9 +--------
 src/services/autofill.service.ts          |  6 ++++--
 5 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/src/_locales/en/messages.json b/src/_locales/en/messages.json
index 3c04cfad93..12a4f96fb6 100644
--- a/src/_locales/en/messages.json
+++ b/src/_locales/en/messages.json
@@ -886,10 +886,7 @@
     "message": "If a login form is detected, automatically perform an auto-fill when the web page loads."
   },
   "enableAutoTotpCopyOnAutoFill": {
-    "message": "Enable Copy to Clipboard of TOTP On Page Auto-fill"
-  },
-  "enableAutoTotpCopyOnAutoFillDesc": {
-    "message": "If login auto-fills on page load and TOTP exists for this login, copy to clipboard without prompt"
+    "message": "Enable Copy to Clipboard of TOTP"
   },
   "experimentalFeature": {
     "message": "This is currently an experimental feature. Use at your own risk."
diff --git a/src/popup/scss/box.scss b/src/popup/scss/box.scss
index 91f8db1d5a..8ef64b6b76 100644
--- a/src/popup/scss/box.scss
+++ b/src/popup/scss/box.scss
@@ -9,6 +9,11 @@
         margin-top: 0;
     }
 
+    .box {
+        padding-left: 15px;
+        padding-bottom: 5px;
+    }
+
     .box-header {
         margin: 0 10px 5px 10px;
         text-transform: uppercase;
diff --git a/src/popup/settings/options.component.html b/src/popup/settings/options.component.html
index dc39c2453c..81a9a35598 100644
--- a/src/popup/settings/options.component.html
+++ b/src/popup/settings/options.component.html
@@ -23,19 +23,15 @@
             {{'enableAutoFillOnPageLoadDesc' | i18n}}
             <b>{{'warning' | i18n}}</b>: {{'experimentalFeature' | i18n}}
         </div>
-    </div>
-    <div class="box">
-        <div class="box-content">
-            <div class="box-content-row box-content-row-checkbox" appBoxRow>
-                <label for="autofill">{{'enableAutoTotpCopyOnAutoFill' | i18n}}</label>
-                <input id="autofill" type="checkbox" (change)="updateAutoTotpCopyOnAutoFill()"
-                    [(ngModel)]="enableAutoTotpCopyOnAutoFill" [disabled]="!enableAutoFillOnPageLoad">
+        <div class="box">
+            <div class="box-content">
+                <div class="box-content-row box-content-row-checkbox" appBoxRow>
+                    <label for="autoCopyTotp">{{'enableAutoTotpCopyOnAutoFill' | i18n}}</label>
+                    <input id="autoCopyTotp" type="checkbox" (change)="updateAutoTotpCopyOnAutoFill()"
+                        [(ngModel)]="enableAutoTotpCopyOnAutoFill" [disabled]="!enableAutoFillOnPageLoad">
+                </div>
             </div>
         </div>
-        <div class="box-footer">
-            {{'enableAutoTotpCopyOnAutoFillDesc' | i18n}}
-            <b>{{'warning' | i18n}}</b>: {{'experimentalFeature' | i18n}}
-        </div>
     </div>
     <div class="box">
         <div class="box-content">
diff --git a/src/popup/settings/options.component.ts b/src/popup/settings/options.component.ts
index 80a145e318..c2118634f4 100644
--- a/src/popup/settings/options.component.ts
+++ b/src/popup/settings/options.component.ts
@@ -123,18 +123,11 @@ export class OptionsComponent implements OnInit {
 
     async updateAutoFillOnPageLoad() {
         await this.storageService.save(ConstantsService.enableAutoFillOnPageLoadKey, this.enableAutoFillOnPageLoad);
-        if (!this.enableAutoFillOnPageLoad) {
-            // If we disable Auto Fill on Page Load, also disable Copying of TOTP
-            await this.storageService.save(ConstantsService.enableAutoTotpCopyOnAutoFill, false);
-            // TODO the below reloads the entire extension, I just want to reload the current view, or at least the enable auto totp copy checkbox
-            window.setTimeout(() => window.location.reload(), 200);
-        }
         this.callAnalytics('Auto-fill Page Load', this.enableAutoFillOnPageLoad);
     }
 
     async updateAutoTotpCopyOnAutoFill() {
-        await this.storageService.save(ConstantsService.enableAutoTotpCopyOnAutoFill, this.enableAutoTotpCopyOnAutoFill);
-        this.callAnalytics('Auto Copy TOTP on Page Load', this.enableAutoTotpCopyOnAutoFill);
+        await this.storageService.save(ConstantsService.enableAutoTotpCopyOnAutoFillKey, this.enableAutoTotpCopyOnAutoFill);
     }
 
     async updateDisableFavicon() {
diff --git a/src/services/autofill.service.ts b/src/services/autofill.service.ts
index 9cb3822219..a71c5f6cfb 100644
--- a/src/services/autofill.service.ts
+++ b/src/services/autofill.service.ts
@@ -254,12 +254,14 @@ export default class AutofillService implements AutofillServiceInterface {
             }
         }
 
-        const shouldCopyTotpOnAutoFill = await this.totpService.isAutoCopyOnAutoFillEnabled();
+        const copyTotpOnAutoFill = await this.totpService.isAutoCopyOnAutoFillEnabled();
+        const enableAutoFill = await this.totpService.isAutoCopyEnabled();
+        const shouldCopyTotp = copyTotpOnAutoFill && enableAutoFill;
 
         const totpCode = await this.doAutoFill({
             cipher: cipher,
             pageDetails: pageDetails,
-            skipTotp: !fromCommand && !shouldCopyTotpOnAutoFill,
+            skipTotp: !fromCommand && !shouldCopyTotp,
             skipLastUsed: !fromCommand,
             skipUsernameOnlyFill: !fromCommand,
             onlyEmptyFields: !fromCommand,