fix(accounts): address jimchamp + Mek review feedback on OTP login

mekarpeles · mekarpeles · commit cfd076901608 · 2026-05-24T16:33:00.000-06:00
- Move @internetarchive/elements to devDependencies (matches OL convention;
  all other JS deps are devDependencies)
- Button: use OL primary-blue CSS var, full width, margin-top spacing,
  and correct hover state to match existing Log In button
- Keep modal open on overlay click when user is in code-entry step —
  avoids losing flow when switching back from email client
- Rename redeem_otp() parameter password→otp for clarity; update
  xauth() kwarg and fake xauth endpoint check to match
- Move OTP login section above password form (right after Google sign-in)
  per Mek's request
diff --git a/openlibrary/accounts/model.py b/openlibrary/accounts/model.py
@@ -866,12 +866,12 @@ def issue_otp(cls, email, service="ol", originating_ip=None):
         )
 
     @classmethod
-    def redeem_otp(cls, email, password, originating_ip=None):
+    def redeem_otp(cls, email, otp, originating_ip=None):
         headers = {"X-Originating-IP": originating_ip} if originating_ip else None
         return cls.xauth(
             "redeem_otp",
             email=email.strip().lower(),
-            password=password,
+            otp=otp,
             headers=headers,
         )
 
diff --git a/openlibrary/components/lit/OpenLibraryOTP.js b/openlibrary/components/lit/OpenLibraryOTP.js
@@ -48,19 +48,20 @@ export class OpenLibraryOTP extends LitElement {
         }
 
         .trigger-btn {
-            background: var(--ia-button-primary-bg, #4b4bdf);
+            background: var(--primary-blue, hsl(202, 96%, 37%));
             color: var(--ia-button-primary-color, #fff);
             border: none;
             border-radius: 4px;
             padding: 8px 16px;
             font-size: 1rem;
             font-family: inherit;
             cursor: pointer;
-            width: fit-content;
+            width: 100%;
+            margin-top: 10px;
         }
         .trigger-btn:hover,
         .trigger-btn:focus {
-            background: var(--ia-button-primary-bg-hover, #3a3abf);
+            background: hsl(202, 96%, 17%);
             outline: 2px solid currentColor;
             outline-offset: 2px;
         }
@@ -288,6 +289,10 @@ export class OpenLibraryOTP extends LitElement {
     }
 
     _handleOverlayClick(e) {
+        // Keep the modal open while the user is entering their code — they are
+        // likely switching back from their email client and an accidental click
+        // should not lose their place in the flow.
+        if (this._step === 'code') return;
         if (e.target === e.currentTarget) this._closeModal();
     }
 
diff --git a/openlibrary/plugins/upstream/account.py b/openlibrary/plugins/upstream/account.py
@@ -161,7 +161,7 @@ def POST(self):
             result = {"success": True, "version": 1}
         elif i.op == "redeem_otp":
             # Accept "123456" as the dev OTP code
-            if body.get("password") == "123456":
+            if body.get("otp") == "123456":
                 result = {
                     "success": True,
                     "version": 1,
diff --git a/openlibrary/templates/login.html b/openlibrary/templates/login.html
@@ -42,6 +42,8 @@ <h1 class="ol-signup-hero__title">$_("Log In")</h1>
         <p>$:_('If you\'d like to create a new, different Open Library account, you\'ll need to <a href="javascript:;" onclick="document.forms[\'hamburger-logout\'].submit()">log out</a> and start the signup process afresh.')</p>
 
     $else:
+        <ol-otp-login redirect="$(form.redirect.value or '/account/books')"></ol-otp-login>
+        <div class="ol-signup-form__big-or">$_('OR')</div>
         <form id="register" class="login olform" name="login" method="post" data-i18n="$json_encode(i18n_strings)">
             $if form.note:
                 <div class="flash-messages">
@@ -94,7 +96,5 @@ <h1 class="ol-signup-hero__title">$_("Log In")</h1>
             <p class="ol-signup-form__login-hint">$:_('Don\'t have an account? <a href="/account/create">Sign up now</a>.')</p>
         </form>
 
-        <div class="ol-signup-form__big-or">$_('OR')</div>
-        <ol-otp-login redirect="$(form.redirect.value or '/account/books')"></ol-otp-login>
     </div>
 </div>
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -28,6 +28,7 @@
     "build-storybook": "cd stories && npm install --no-audit && npx storybook build"
   },
   "devDependencies": {
+    "@internetarchive/elements": "^0.2.5",
     "@babel/core": "^7.24.7",
     "@babel/eslint-parser": "^7.28.6",
     "@babel/preset-env": "^7.29.3",
@@ -130,8 +131,5 @@
     "Safari >= 11.1",
     "iOS >= 11.3",
     "Android >= 5"
-  ],
-  "dependencies": {
-    "@internetarchive/elements": "^0.2.5"
-  }
+  ]
 }

Original file line number	Diff line number	Diff line change
`@@ -866,12 +866,12 @@ def issue_otp(cls, email, service="ol", originating_ip=None):`
`866`	`866`	`)`
`867`	`867`
`868`	`868`	`@classmethod`
`869`		`- def redeem_otp(cls, email, password, originating_ip=None):`
	`869`	`+ def redeem_otp(cls, email, otp, originating_ip=None):`
`870`	`870`	`headers = {"X-Originating-IP": originating_ip} if originating_ip else None`
`871`	`871`	`return cls.xauth(`
`872`	`872`	`"redeem_otp",`
`873`	`873`	`email=email.strip().lower(),`
`874`		`- password=password,`
	`874`	`+ otp=otp,`
`875`	`875`	`headers=headers,`
`876`	`876`	`)`
`877`	`877`