improve email settings progressive enhancement and admin layout
Some checks failed
deploy / deploy (push) Has been cancelled

- semantic HTML: step numbers inside h2, strong provider names, details
  for adapter configs, strong error messages, fieldset drawer toggle hidden
- inline field errors via flash for no-JS controller fallback
- single form POST button for test email (works with and without JS)
- admin sidebar: remove brand/view-shop, move user email to footer nav
- replace inline style with .admin-setup-step-spaced class
- clean up unused CSS (.admin-brand, .admin-sidebar-header, etc.)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
jamey 2026-03-05 15:29:05 +00:00
parent 5dee2ae0be
commit c6636cab65
8 changed files with 91 additions and 126 deletions

View File

@ -1510,6 +1510,9 @@
} }
.card-radio-name { .card-radio-name {
display: flex;
align-items: baseline;
gap: 0.25rem;
font-size: 0.875rem; font-size: 0.875rem;
font-weight: 600; font-weight: 600;
} }
@ -3391,26 +3394,21 @@
margin-inline: auto; margin-inline: auto;
} }
/* Sidebar header (logo + user email) */
.admin-sidebar-header {
padding: 1rem;
border-bottom: 1px solid var(--t-border-default);
& .admin-text-secondary { margin-top: 0.125rem; }
}
/* View shop link below sidebar header */
.admin-sidebar-view-shop {
padding: 0.5rem;
border-bottom: 1px solid var(--t-border-default);
}
/* Sidebar footer (dev tools, log out) */ /* Sidebar footer (dev tools, log out) */
.admin-sidebar-footer { .admin-sidebar-footer {
padding: 0.5rem; padding: 0.5rem;
border-top: 1px solid var(--t-border-default); border-top: 1px solid var(--t-border-default);
} }
.admin-sidebar-email {
display: flex;
align-items: center;
gap: 0.5rem;
padding: 0.5rem 0.75rem;
font-size: 0.75rem;
color: var(--t-text-secondary);
}
/* Dev tools disclosure in sidebar footer */ /* Dev tools disclosure in sidebar footer */
.admin-dev-tools { .admin-dev-tools {
& > summary { & > summary {
@ -3446,14 +3444,6 @@
padding: 0.5rem; padding: 0.5rem;
} }
/* Brand wordmark in sidebar */
.admin-brand {
font-size: 1.125rem;
line-height: 1.75rem;
font-weight: 700;
letter-spacing: -0.025em;
}
/* Secondary text for user email, help text, meta info */ /* Secondary text for user email, help text, meta info */
.admin-text-secondary { .admin-text-secondary {
font-size: 0.75rem; font-size: 0.75rem;
@ -4294,10 +4284,16 @@
gap: 0.5rem; gap: 0.5rem;
} }
.admin-setup-step-spaced { margin-top: 1.5rem; }
.admin-setup-step-header { .admin-setup-step-header {
display: flex; display: flex;
align-items: center; align-items: center;
gap: 0.625rem; gap: 0.625rem;
font-size: 0.9375rem;
font-weight: 600;
color: var(--admin-text-primary);
margin: 0;
} }
.admin-setup-step-number { .admin-setup-step-number {
@ -4314,13 +4310,6 @@
flex-shrink: 0; flex-shrink: 0;
} }
.admin-setup-step-title {
font-size: 0.9375rem;
font-weight: 600;
color: var(--admin-text-primary);
margin: 0;
}
.admin-setup-step-desc { .admin-setup-step-desc {
font-size: 0.8125rem; font-size: 0.8125rem;
color: var(--admin-text-secondary); color: var(--admin-text-secondary);
@ -4379,6 +4368,9 @@
gap: 1.5rem; gap: 1.5rem;
} }
/* Hide the <details> summary — CSS :has(:checked) controls visibility instead */
.admin-adapter-config-summary { display: none; }
/* Show only the adapter config matching the checked radio */ /* Show only the adapter config matching the checked radio */
.admin-adapter-config[data-adapter] { display: none; } .admin-adapter-config[data-adapter] { display: none; }

View File

@ -288,7 +288,7 @@ defmodule BerrypodWeb.CoreComponents do
~H""" ~H"""
<p class="admin-error" id={@id} role="alert"> <p class="admin-error" id={@id} role="alert">
<.icon name="hero-exclamation-circle" class="admin-error-icon" /> <.icon name="hero-exclamation-circle" class="admin-error-icon" />
{render_slot(@inner_block)} <strong>{render_slot(@inner_block)}</strong>
</p> </p>
""" """
end end

View File

@ -1,5 +1,5 @@
<div class="admin-layout"> <div class="admin-layout">
<input id="admin-drawer" type="checkbox" class="admin-layout-toggle" /> <input id="admin-drawer" type="checkbox" class="admin-layout-toggle" hidden />
<%!-- main content area --%> <%!-- main content area --%>
<div class="admin-layout-content"> <div class="admin-layout-content">
@ -20,6 +20,7 @@
<%!-- page content --%> <%!-- page content --%>
<main class="admin-main"> <main class="admin-main">
<.flash_group flash={@flash} />
<div class="admin-container"> <div class="admin-container">
{@inner_content} {@inner_content}
</div> </div>
@ -30,23 +31,6 @@
<div class="admin-sidebar-wrapper"> <div class="admin-sidebar-wrapper">
<label for="admin-drawer" class="admin-sidebar-overlay" aria-label="Close navigation"></label> <label for="admin-drawer" class="admin-sidebar-overlay" aria-label="Close navigation"></label>
<aside class="admin-sidebar"> <aside class="admin-sidebar">
<%!-- sidebar header --%>
<div class="admin-sidebar-header">
<.link navigate={~p"/admin"} class="admin-brand">
Berrypod
</.link>
<p class="admin-text-secondary truncate">
{@current_scope.user.email}
</p>
</div>
<%!-- view shop link --%>
<div class="admin-sidebar-view-shop">
<.link href={~p"/"} class="admin-btn admin-btn-ghost admin-btn-sm admin-btn-block">
<.icon name="hero-arrow-top-right-on-square" class="size-4" /> View shop
</.link>
</div>
<%!-- nav links --%> <%!-- nav links --%>
<nav class="admin-sidebar-nav" aria-label="Admin navigation"> <nav class="admin-sidebar-nav" aria-label="Admin navigation">
<div class="admin-nav-group"> <div class="admin-nav-group">
@ -189,6 +173,9 @@
<%!-- sidebar footer --%> <%!-- sidebar footer --%>
<div class="admin-sidebar-footer"> <div class="admin-sidebar-footer">
<ul class="admin-nav"> <ul class="admin-nav">
<li class="admin-sidebar-email truncate">
<.icon name="hero-user" class="size-5" /> {@current_scope.user.email}
</li>
<li> <li>
<details class="admin-dev-tools"> <details class="admin-dev-tools">
<summary> <summary>
@ -224,5 +211,3 @@
</aside> </aside>
</div> </div>
</div> </div>
<.flash_group flash={@flash} />

View File

@ -21,10 +21,9 @@ defmodule BerrypodWeb.EmailSettingsController do
|> redirect(to: ~p"/admin/settings/email") |> redirect(to: ~p"/admin/settings/email")
{:error, field_errors} when is_map(field_errors) -> {:error, field_errors} when is_map(field_errors) ->
message = field_errors |> Map.values() |> Enum.join(". ")
conn conn
|> put_flash(:error, message) |> put_flash(:field_errors, field_errors)
|> put_flash(:error_adapter, adapter_key)
|> redirect(to: ~p"/admin/settings/email") |> redirect(to: ~p"/admin/settings/email")
end end
end end

View File

@ -11,7 +11,11 @@ defmodule BerrypodWeb.Admin.EmailSettings do
{current_adapter, _current_values} = Mailer.current_config() {current_adapter, _current_values} = Mailer.current_config()
saved_adapter = Settings.get_setting("email_adapter") saved_adapter = Settings.get_setting("email_adapter")
adapter_key = current_adapter || saved_adapter # Controller fallback passes errors via flash after failed validation
flash_adapter = Phoenix.Flash.get(socket.assigns.flash, :error_adapter)
flash_errors = Phoenix.Flash.get(socket.assigns.flash, :field_errors) || %{}
adapter_key = flash_adapter || current_adapter || saved_adapter
grouped = Adapters.grouped() grouped = Adapters.grouped()
all_adapters = Adapters.all() all_adapters = Adapters.all()
all_values = load_all_adapter_values() all_values = load_all_adapter_values()
@ -32,7 +36,7 @@ defmodule BerrypodWeb.Admin.EmailSettings do
|> assign(:test_error, nil) |> assign(:test_error, nil)
|> assign(:test_retryable, false) |> assign(:test_retryable, false)
|> assign(:from_checklist, false) |> assign(:from_checklist, false)
|> assign(:field_errors, %{}) |> assign(:field_errors, flash_errors)
|> assign(:form, to_form(%{}, as: :email))} |> assign(:form, to_form(%{}, as: :email))}
end end
@ -188,10 +192,9 @@ defmodule BerrypodWeb.Admin.EmailSettings do
> >
<%!-- Step 1: Choose a provider --%> <%!-- Step 1: Choose a provider --%>
<div class="admin-setup-step"> <div class="admin-setup-step">
<div class="admin-setup-step-header"> <h2 class="admin-setup-step-header">
<span class="admin-setup-step-number">1</span> <span class="admin-setup-step-number">1</span> Choose a provider
<h2 class="admin-setup-step-title">Choose a provider</h2> </h2>
</div>
<fieldset class="card-radio-fieldset" disabled={@env_locked}> <fieldset class="card-radio-fieldset" disabled={@env_locked}>
<legend class="sr-only">Email provider</legend> <legend class="sr-only">Email provider</legend>
@ -205,7 +208,10 @@ defmodule BerrypodWeb.Admin.EmailSettings do
/> />
</div> </div>
<details class="admin-provider-other"> <details
class="admin-provider-other"
open={@adapter_key in Enum.map(@advanced_adapters, & &1.key)}
>
<summary class="admin-provider-other-toggle"> <summary class="admin-provider-other-toggle">
Already have your own email server? Already have your own email server?
</summary> </summary>
@ -237,10 +243,9 @@ defmodule BerrypodWeb.Admin.EmailSettings do
<div <div
:if={@email_configured} :if={@email_configured}
id="test-email-step" id="test-email-step"
class="admin-setup-step" class="admin-setup-step admin-setup-step-spaced"
style="margin-top: 1.5rem;"
> >
<div class="admin-setup-step-header"> <h2 class="admin-setup-step-header">
<span class={[ <span class={[
"admin-setup-step-number", "admin-setup-step-number",
@test_result == :ok && "admin-setup-step-number-done", @test_result == :ok && "admin-setup-step-number-done",
@ -255,7 +260,6 @@ defmodule BerrypodWeb.Admin.EmailSettings do
{if @selected_adapter && @selected_adapter.url, do: "4", else: "3"} {if @selected_adapter && @selected_adapter.url, do: "4", else: "3"}
<% end %> <% end %>
</span> </span>
<h2 class="admin-setup-step-title">
<%= cond do %> <%= cond do %>
<% @test_result == :ok -> %> <% @test_result == :ok -> %>
Email is working Email is working
@ -265,7 +269,6 @@ defmodule BerrypodWeb.Admin.EmailSettings do
Send a test email Send a test email
<% end %> <% end %>
</h2> </h2>
</div>
<%= if @test_result == :ok do %> <%= if @test_result == :ok do %>
<p class="admin-setup-step-desc"> <p class="admin-setup-step-desc">
@ -308,35 +311,17 @@ defmodule BerrypodWeb.Admin.EmailSettings do
<p class="admin-setup-step-desc"> <p class="admin-setup-step-desc">
Send a test to <strong>{@current_scope.user.email}</strong> to check everything works. Send a test to <strong>{@current_scope.user.email}</strong> to check everything works.
</p> </p>
<div class="admin-row admin-row-sm">
<%!-- JS: async send via LiveView --%>
<span id="test-email-js">
<.button
type="button"
phx-click="send_test"
disabled={@sending_test}
phx-disable-with="Sending..."
>
<.icon name="hero-paper-airplane" class="size-4" /> Send test email
</.button>
</span>
<%!-- No-JS: form POST fallback (hides the JS button above) --%>
<noscript>
<style>
#test-email-js { display: none; }
</style>
<.form <.form
for={%{}} for={%{}}
action={~p"/admin/settings/email/test"} action={~p"/admin/settings/email/test"}
phx-submit="send_test"
method="post" method="post"
style="display:inline" class="admin-row admin-row-sm"
> >
<.button> <.button disabled={@sending_test} phx-disable-with="Sending...">
<.icon name="hero-paper-airplane" class="size-4" /> Send test email <.icon name="hero-paper-airplane" class="size-4" /> Send test email
</.button> </.button>
</.form> </.form>
</noscript>
</div>
<% end %> <% end %>
<% end %> <% end %>
</div> </div>
@ -354,13 +339,13 @@ defmodule BerrypodWeb.Admin.EmailSettings do
defp adapter_config(assigns) do defp adapter_config(assigns) do
~H""" ~H"""
<div hidden={!@selected} class="admin-adapter-config" data-adapter={@adapter.key}> <details open={@selected} class="admin-adapter-config" data-adapter={@adapter.key}>
<summary class="admin-adapter-config-summary">Configure {@adapter.name}</summary>
<%!-- Create an account (providers with sign-up URLs) --%> <%!-- Create an account (providers with sign-up URLs) --%>
<div :if={@adapter.url} class="admin-setup-step"> <div :if={@adapter.url} class="admin-setup-step">
<div class="admin-setup-step-header"> <h2 class="admin-setup-step-header">
<span class="admin-setup-step-number">2</span> <span class="admin-setup-step-number">2</span> Create a free account
<h2 class="admin-setup-step-title">Create a free account</h2> </h2>
</div>
<p class="admin-setup-step-desc"> <p class="admin-setup-step-desc">
<.external_link href={@adapter.url} class="admin-link"> <.external_link href={@adapter.url} class="admin-link">
Sign up at {@adapter.name} &nearr; Sign up at {@adapter.name} &nearr;
@ -371,12 +356,12 @@ defmodule BerrypodWeb.Admin.EmailSettings do
<%!-- Paste your key / server details --%> <%!-- Paste your key / server details --%>
<div class="admin-setup-step"> <div class="admin-setup-step">
<div class="admin-setup-step-header"> <h2 class="admin-setup-step-header">
<span class="admin-setup-step-number"> <span class="admin-setup-step-number">
{if @adapter.url, do: "3", else: "2"} {if @adapter.url, do: "3", else: "2"}
</span> </span>
<h2 class="admin-setup-step-title">{adapter_fields_title(@adapter)}</h2> {adapter_fields_title(@adapter)}
</div> </h2>
<p class="admin-setup-step-desc">{adapter_fields_instruction(@adapter)}</p> <p class="admin-setup-step-desc">{adapter_fields_instruction(@adapter)}</p>
<%= for field <- @adapter.fields do %> <%= for field <- @adapter.fields do %>
<.adapter_field_input <.adapter_field_input
@ -395,7 +380,7 @@ defmodule BerrypodWeb.Admin.EmailSettings do
</div> </div>
<% end %> <% end %>
</div> </div>
</div> </details>
""" """
end end
@ -435,12 +420,12 @@ defmodule BerrypodWeb.Admin.EmailSettings do
disabled={@disabled} disabled={@disabled}
class="card-radio-input" class="card-radio-input"
/> />
<div class="card-radio-name"> <span class="card-radio-name">
{@adapter.name} <strong>{@adapter.name}</strong>
<span :if={@adapter.recommended} class="card-radio-badge card-radio-recommended"> <span :if={@adapter.recommended} class="card-radio-badge card-radio-recommended">
Recommended Recommended
</span> </span>
</div> </span>
<div :if={@adapter.free_tier} class="card-radio-description">{@adapter.free_tier}</div> <div :if={@adapter.free_tier} class="card-radio-description">{@adapter.free_tier}</div>
<div :if={@adapter.setup_hint} class="card-radio-description">{@adapter.setup_hint}</div> <div :if={@adapter.setup_hint} class="card-radio-description">{@adapter.setup_hint}</div>
</label> </label>

View File

@ -33,14 +33,18 @@ defmodule BerrypodWeb.EmailSettingsControllerTest do
assert Settings.get_setting("email_adapter") == "brevo" assert Settings.get_setting("email_adapter") == "brevo"
end end
test "redirects with error on validation failure", %{conn: conn} do test "redirects with field errors on validation failure", %{conn: conn} do
conn = conn =
post(conn, ~p"/admin/settings/email", %{ post(conn, ~p"/admin/settings/email", %{
email: %{adapter: "brevo", brevo: %{api_key: ""}} email: %{adapter: "brevo", brevo: %{api_key: ""}}
}) })
assert redirected_to(conn) =~ ~p"/admin/settings/email" assert redirected_to(conn) =~ ~p"/admin/settings/email"
assert Phoenix.Flash.get(conn.assigns.flash, :error) =~ "required"
field_errors = Phoenix.Flash.get(conn.assigns.flash, :field_errors)
assert is_map(field_errors)
assert field_errors |> Map.values() |> Enum.any?(&String.contains?(&1, "required"))
assert Phoenix.Flash.get(conn.assigns.flash, :error_adapter) == "brevo"
end end
end end

View File

@ -252,7 +252,7 @@ defmodule BerrypodWeb.Admin.EmailSettingsTest do
assert html =~ "sr-only" assert html =~ "sr-only"
end end
test "only selected adapter config is visible (hidden attribute)", %{conn: conn} do test "only selected adapter config is open (details element)", %{conn: conn} do
{:ok, view, _html} = live(conn, ~p"/admin/settings/email") {:ok, view, _html} = live(conn, ~p"/admin/settings/email")
# Select SMTP # Select SMTP
@ -260,12 +260,12 @@ defmodule BerrypodWeb.Admin.EmailSettingsTest do
|> form("form[phx-change=\"form_change\"]", %{email: %{adapter: "smtp"}}) |> form("form[phx-change=\"form_change\"]", %{email: %{adapter: "smtp"}})
|> render_change() |> render_change()
# SMTP should not be hidden # SMTP details should be open
refute has_element?(view, ~s([data-adapter="smtp"][hidden])) assert has_element?(view, ~s(details[data-adapter="smtp"][open]))
# Others should be hidden # Others should not be open
assert has_element?(view, ~s([data-adapter="brevo"][hidden])) refute has_element?(view, ~s(details[data-adapter="brevo"][open]))
assert has_element?(view, ~s([data-adapter="sendgrid"][hidden])) refute has_element?(view, ~s(details[data-adapter="sendgrid"][open]))
assert has_element?(view, ~s([data-adapter="postal"][hidden])) refute has_element?(view, ~s(details[data-adapter="postal"][open]))
end end
end end

View File

@ -43,10 +43,10 @@ defmodule BerrypodWeb.Admin.LayoutTest do
assert html =~ user.email assert html =~ user.email
end end
test "shows view shop and log out links", %{conn: conn} do test "shows shop and log out links", %{conn: conn} do
{:ok, view, _html} = live(conn, ~p"/admin/orders") {:ok, view, _html} = live(conn, ~p"/admin/orders")
assert has_element?(view, ~s(a[href="/"]), "View shop") assert has_element?(view, ~s(a[href="/"]), "Shop")
assert has_element?(view, ~s(a[href="/users/log-out"]), "Log out") assert has_element?(view, ~s(a[href="/users/log-out"]), "Log out")
end end
end end