Update auth pages visually#6479
Conversation
|
- Create new auth layout context that looks visually cleaner, and hides the header and footer - Transform all auth pages to use the new context. This includes: - Login page - Signup page - Password reset page - Password reset request page - Password reset request success page - Password reset error page - Activate account page - Verify 2FA page - Verify 2FA recovery code page - SSO login page - SSO provision notice page - SSO provision issue page - Invitation expired page - Tighten copy across auth pages to be more consistent and concise - Remove password conformation field from signup page - Extract password field to a new component with visibility toggle - Extract Plausible logo to a new component
4319128 to
081c808
Compare
| oninput_js = | ||
| "this.value=this.value.replace(/[^0-9]/g, '');" <> | ||
| if assigns.autosubmit? do | ||
| " if (this.value.length >= #{assigns.length}) this.form.requestSubmit();" | ||
| else | ||
| "" | ||
| end |
There was a problem hiding this comment.
nitpick, non-blocking: With the autosubmit feature in Activate account feature, it's quite easy to hit the rate limiter, which leads to a frustrating "Rate limited. Go to homepage (actually lands you back on Activate account feature)" experience. Can we treat autosubmitting activation codes as a separate extra feature from the bulk of the changes?
There was a problem hiding this comment.
@apata I reverted to the original behaviour, and I also changed the flow to an inline message instead of a separate page for a less frustrating experience:
| post(conn, "/login", email: user.email, password: "wrong") | ||
| end) | ||
|
|
||
| assert logs =~ "[warning] [login] wrong password for #{user.email}" |
| ) | ||
|
|
||
| assert html_response(response, 429) =~ "Too many activation attempts" | ||
| assert html_response(response, 200) =~ |
There was a problem hiding this comment.
What's the reason for changing from 429 to 200?
There was a problem hiding this comment.
@aerosol we switched the rate-limit from a full-page 429 error to an inline error on the /activate page itself for better UX (see this convo). Since inline errors are rendered as a normal form render rather than an error page, I thought the status is 200 instead of 429. Would you prefer if we keep 429 instead?
| end | ||
| } | ||
| onclick="this.select();" | ||
| <div class={[@class, "flex flex-col gap-y-6 items-center"]}> |
| render(conn, "activate.html", | ||
| error: "Incorrect activation code", | ||
| render_auth_page(conn, "activate.html", | ||
| error: "That code didn't work. Please try again.", |
There was a problem hiding this comment.
This is a bit inconsistent - we're rendering wrong activation code error inline, under the input, but now wrong 2FA code at login is indicated by the flash. Same with wrong recovery code.
| </:footer> | ||
| </.focus_box> | ||
| <% else %> | ||
| <.styled_link method="delete" href="/me?redirect=/register">change email</.styled_link> |
There was a problem hiding this comment.
are we deliberately dropping the confirmation dialog?
There was a problem hiding this comment.
Yeah deliberate, at this point in the flow it's not really a destructive action. Clicking 'change email' and then being asked 'Deleting your account cannot be reversed. Are you sure?' sounds quite alarming for what's essentially 'let me fix a typo/use a different email'.
RobertJoonas
left a comment
There was a problem hiding this comment.
Nice work! 👏 Happy to see much less classname duplication and more extracted reusable components :)
| else | ||
| {:error, :wrong_password} -> | ||
| Auth.log_failed_login_attempt("wrong password for #{email}") | ||
| Auth.log_failed_login_attempt("Incorrect password for #{email}") |
There was a problem hiding this comment.
This message is not exposed to the user so I would revert this change. There might be a Grafana graph somewhere that depends on the word "wrong".
| <%= if ee?() do %> | ||
| <%= on_ee do %> |
There was a problem hiding this comment.
I think the outer if block can be removed here.
| scope alias: Live, | ||
| assigns: %{ | ||
| connect_live_socket: true, | ||
| hide_header?: true, | ||
| hide_footer?: true, | ||
| disable_global_notices?: true | ||
| } do |
There was a problem hiding this comment.
Since the new AuthLayoutContext is responsible for putting the :auth layout, what do you think if we move these layout-related assigns there too?
def on_mount(_arg, _params, _session, socket) do
socket =
assign(socket,
hide_header?: true,
hide_footer?: true,
disable_global_notices?: true
)
{:cont, socket, layout: {PlausibleWeb.LayoutView, :auth}}
end| end) | ||
|
|
||
| assert logs =~ "[warning] [login] wrong password for #{user.email}" | ||
| assert logs =~ "[warning] [login] Incorrect password for #{user.email}" |
There was a problem hiding this comment.
Ditto: this log is for internal use. Best to revert.

Changes
Tests
Changelog
Documentation
Dark mode