From 639521047408333d0a19cb3e180a34fe3b26a07f Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sat, 3 Oct 2026 01:30:02 +0200 Subject: [PATCH] Show each page's own title in the browser tab (closes #117) Every browser tab read "Webhooker": parsePageTemplate parsed each page before htmlheader.html, whose {{block "title"}} fallback then redefined the page's {{define "title"}}. The page file is now parsed last, so its title replaces the fallback (a later definition of a template name replaces an earlier one, and an empty one never does). A test renders every page template and checks its browser tab title. Model: opus-5-5 --- internal/handlers/handlers.go | 13 ++-- internal/handlers/page_title_test.go | 107 +++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 internal/handlers/page_title_test.go diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 9decb63..caf4e0c 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -110,22 +110,25 @@ type Handlers struct { // parsePageTemplate parses a page-specific template set from the // embedded FS. Each page template is combined with the shared // base, htmlheader, navbar and notice templates, and with any further -// files the page includes. The page file must be listed first so that -// its root action ({{template "base" .}}) becomes the template set's -// entry point. +// files the page includes. The set is named after the page file, so +// the page's root action ({{template "base" .}}) is its entry point. +// +// The page file is parsed last because a later definition of a name +// replaces an earlier one: the page's {{define "title"}} must replace +// the {{block "title"}} fallback in htmlheader.html. func parsePageTemplate( pageFile string, included ...string, ) *template.Template { files := append([]string{ - pageFile, "base.html", "htmlheader.html", "navbar.html", "notice.html", }, included...) + files = append(files, pageFile) return template.Must( - template.ParseFS(templates.Templates, files...), + template.New(pageFile).ParseFS(templates.Templates, files...), ) } diff --git a/internal/handlers/page_title_test.go b/internal/handlers/page_title_test.go new file mode 100644 index 0000000..81bd9a6 --- /dev/null +++ b/internal/handlers/page_title_test.go @@ -0,0 +1,107 @@ +package handlers_test + +import ( + "html/template" + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/webhooker/internal/database" + "sneak.berlin/go/webhooker/internal/handlers" + "sneak.berlin/go/webhooker/internal/session" + "sneak.berlin/go/webhooker/templates" +) + +// TestEveryPageRendersItsOwnTitle renders each page template and checks +// the browser tab title is the one the page declares, not the +// "Webhooker" fallback in htmlheader.html. A page that fails to render +// shows the error page's title instead, and fails here too. +func TestEveryPageRendersItsOwnTitle(t *testing.T) { + t.Parallel() + + var h *handlers.Handlers + + var sess *session.Session + + app := newTestApp(t, &h, &sess) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + // A pointer, as in the handlers: some pages call + // Webhook.RetentionLabel, a pointer method. + webhook := &database.Webhook{Name: "orders", RetentionDays: 14} + webhook.ID = testWebhookID + + pages := []struct { + page string + data map[string]any + title string + }{ + {"login.html", map[string]any{}, "Login - Webhooker"}, + {"profile.html", map[string]any{}, "Profile - Webhooker"}, + {"settings.html", map[string]any{}, "Settings - Webhooker"}, + {"sources_list.html", map[string]any{}, "Webhooks - Webhooker"}, + {"sources_new.html", map[string]any{}, "New Webhook - Webhooker"}, + { + "source_detail.html", + map[string]any{dataKeyWebhook: webhook}, + "orders - Webhooker", + }, + { + "source_edit.html", + map[string]any{dataKeyWebhook: webhook}, + "Edit orders - Webhooker", + }, + { + "source_logs.html", + map[string]any{dataKeyWebhook: webhook, "TotalEvents": int64(0)}, + "Full Event Log - orders - Webhooker", + }, + { + "event_detail.html", + map[string]any{dataKeyWebhook: webhook}, + "Event - orders - Webhooker", + }, + { + "target_edit.html", + map[string]any{ + dataKeyWebhook: webhook, + "Target": map[string]any{"Name": "alerts", "Type": "slack"}, + }, + "Edit alerts - Webhooker", + }, + { + "error.html", + map[string]any{"StatusText": http.StatusText(http.StatusNotFound)}, + "Not Found - Webhooker", + }, + } + + for _, p := range pages { + body := renderPage(t, h, sess, p.page, p.data) + + _, afterOpen, _ := strings.Cut(body, "") + title, _, _ := strings.Cut(afterOpen, "") + + assert.Equal(t, p.title, title, p.page) + } +} + +// TestTitleFallbackIsWebhooker checks the title htmlheader.html gives a +// page that declares none. Every page declares one, so it is checked on +// htmlheader.html alone. +func TestTitleFallbackIsWebhooker(t *testing.T) { + t.Parallel() + + header := template.Must( + template.ParseFS(templates.Templates, "htmlheader.html"), + ) + + var buf strings.Builder + + require.NoError(t, header.ExecuteTemplate(&buf, "htmlheader", nil)) + assert.Contains(t, buf.String(), "Webhooker") +}