Sign-in and registration exist twice, and neither route leads back to the shop #50
No Assignees
bermudalamb
bermudalamb
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: bermudalamb/redefined-designs#50
Reference in New Issue
Block a user
Problem
Two separate things are wrong with the auth entry points.
They are dead ends.
/login,/registerand/forgot-passwordrender bare cards with no site header. They link to each other, and/registerlinks to/privacy, but none of them offers a route back to the storefront. A customer who clicks "Log in" from the header and then changes their mind has no way back except the browser's back button.The same flows exist twice.
AuthPromptModalalready provides login and registration as a modal, opened when an item is added to the cart while signed out. So the app has two independent implementations of signing in, which can drift apart in validation, copy, consent wording, and error handling. The registration consent text is the part most worth keeping identical, since it is recorded verbatim against the customer's consent record.Options
AuthPromptModalover the current page, closing back to it. Removes the duplication and matches the pattern being adopted for the account view./loginand/registerwould still need to work as URLs, since/reset-passwordlinks to/loginand customers may have bookmarks.Whichever is chosen, the two implementations should end up sharing one form rather than maintaining two.
Note
/forgot-passwordhas no modal counterpart today, so it needs handling either way.Related
Same root cause as the sibling issues: only
/,/adminand/cartrenderLayout+Header.Design questions resolved
Modal routes, or pages with chrome?
Modal routes, reusing the backdrop-location pattern #51 introduced. The header's Log in / Sign up open a modal over the current page; a direct visit or bookmark opens it over the storefront. Rejected keeping them as pages with chrome: it is the smaller change but leaves the second half of this issue — two independent sign-in implementations — in place. Also rejected adopting the modal presentation while keeping the forms separate, for the same reason.
This means the route and
AuthPromptModalrender one shared form, which is the point the issue makes regardless of which option is chosen. The registration consent wording especially: it is recorded verbatim against the customer's consent record, so two copies of it is a correctness risk, not just duplication.Where does the customer land after signing in?
Back where they were, now signed in — not
/account. Someone who clicks Log in while browsing wants to keep browsing. This also makes the route behave the same wayAuthPromptModalalready does when it resumes an interrupted add-to-cart or favorite. It is a deliberate change from today's behaviour and existing end-to-end tests assert the old destination, so those will be updated.Scope
All four auth routes:
/login,/register,/forgot-password,/reset-password. The first three are what this issue names;/reset-passwordis included because it arrives from an email with a token and most needs somewhere to go afterwards, and because leaving it out would make the reset flow half modal and half page.Left for #52:
/privacy,/verify-email,/admin, and the shared-layout root cause.Implemented on
feature/50-auth-modal-routesAll four auth routes are now modals over the page behind, reusing the backdrop-location arrangement from #51. They remain real routes, so
/reset-password's link to/loginand any bookmarks keep working. Signing in returns the customer to where they were rather than to/account.The duplication was worse than the issue described
The issue anticipated that the two implementations could drift in consent wording. They already had. Three different strings were in play:
/registerpageAuthPromptModalThe server keeps
MARKETING_CONSENT_TEXTverbatim against the consent record precisely so the record says what the customer saw. None of the three matched, so no consent record was accurate regardless of which path was used. Both callers now render oneAuthFormwhose checkbox is the exact string the server records, and an end-to-end test asserts that equality — the only thing spanning the frontend/backend boundary, since there is no shared module. Logged in the project context next to theTAG_COLORSduplication.Other decisions worth recording
replace. Switching tabs or stepping to recovery keeps the whole detour to one history entry, so closing returns to where it started rather than walking back through every tab that was looked at. Covered by a test./privacyhas no way back of its own until #52.Cart's redirect to/logindeliberately does not pass a backdrop. Cart redirects whenever there is no customer, so rendering it as the backdrop would re-fire its own redirect. Its login modal opens over the storefront instead.Verification
83 end-to-end tests, all passing, type checking clean, no backend changes. Nineteen assertions across seven specs waited for
/accountafter signing in; they now assert the header shows a signed-in customer, which is the condition actually being waited for.Two of the changes were forced by real problems rather than being cosmetic: modal submit buttons had to be scoped to their dialog, because the storefront behind now offers a "Log in" button of its own and an unscoped locator matched both; and assertions following a server round-trip needed a realistic timeout, since the 5s default is too tight for a bcrypt hash plus re-rendering the storefront behind the modal.
The accumulated local database (~457 items) is still making the suite slow and contention-prone — it needs a reset, which is a local-environment call rather than mine.
Released to production.
Verified in QA: sign-in and registration open as modals over the page behind, the forgot-password flow is complete, and the marketing consent wording shown is the exact string the server records — which is the correctness fix in this change, not just a cosmetic one. Three different wordings were in circulation before the form was shared, and none matched what was stored.
Worth noting that the consent labels on accounts created before this release were recorded against the old, non-matching text. Nothing needs fixing for those records to be valid — the stored string is still what the server used — but they will not match what the form shows today.