oficina / calendar Public

Review nits: dead /mode code, grid floor, Month keeps the date

- theme.go loses Calendar's retired /mode handler, calendar_mode cookie
  and helpers; handlers.go loses safeReturn and secure(), which only
  /mode used. /appearance (estilo) is the one switcher.
- The day and week grids scroll within max(24rem, 100vh - 20rem), so a
  short window keeps a usable grid; html gets scroll-padding-block-start
  of the bar's height (spec §5.3).
- The view switcher carries the day into Month as ?date=, and Month's
  Day, Week and Agenda tabs go back to it (else today in the current
  month, else the 1st), not always to the 1st.
  TestViewTabsKeepTheSelectedDateThroughMonth failed before on six cases.

Fable 5.1 review of #17.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Paul Campbell pushed by paul@keymail.dev 2 weeks ago 075933e157cf1e1d1e5a770e0ab793affec5b1bf parent 1d6483d

9 files changed, +80 −147

internal/calendar/handlers.go +0 −32
internal/calendar/handlers_views.go +18 −5
internal/calendar/static/calendar.css +10 −2
internal/calendar/templates/agenda.html +1 −1
internal/calendar/templates/day.html +1 −1
internal/calendar/templates/month.html +1 −1
internal/calendar/templates/week.html +1 −1
internal/calendar/theme.go +6 −104
internal/calendartest/view_tabs_test.go +42 −0

CI passed — run details

CI log

=== vet ===
go vet ./...
=== fmt ===
=== test ===
go test ./...
?   	amadan.net/oficina/calendar/cmd/calendar	[no test files]
ok  	amadan.net/oficina/calendar/internal/calendar	5.371s
?   	amadan.net/oficina/calendar/internal/calendar/icons	[no test files]
?   	amadan.net/oficina/calendar/internal/calendar/seal	[no test files]
ok  	amadan.net/oficina/calendar/internal/calendartest	3.288s
ok  	amadan.net/oficina/calendar/internal/guardstest	0.212s
?   	amadan.net/oficina/calendar/scripts/writeestilo	[no test files]
=== migration-check ===
go run github.com/carlosframework/rastrillo/cmd/rastrillo migration check
rastrillo migration check: models and migrations agree

diff --git a/internal/calendar/handlers.go b/internal/calendar/handlers.go

index d2fd362..7758479 100644

--- a/internal/calendar/handlers.go

+++ b/internal/calendar/handlers.go

@@ -3,7 +3,6 @@ package calendar

import (

"log/slog"

"net/http"

- "strings"

rauth "github.com/carlosframework/rastrillo/auth"

"github.com/carlosframework/rastrillo/mail"

@@ -32,37 +31,6 @@ type handlers struct {

viewerFrom func(*http.Request) (rauth.Identity, bool)

}

-// secure reports whether cookies this app sets should carry Secure. It

-// is derived from the origin rather than from the request, so a

-// misconfigured proxy cannot talk the app into dropping the attribute.

-func (h *handlers) secure() bool {

- return strings.HasPrefix(h.origin, "https://")

-// safeReturn returns a same-site path to redirect back to after a form

-// post, or fallback when there is nothing safe to use.

-//

-// Same-site means starting with exactly one "/" — never "//host",

-// which browsers read as a protocol-relative URL to another origin and

-// which is the whole open-redirect bug in one extra character. A

-// backslash is refused for the same reason: some browsers normalise it

-// to a slash, so "/\evil.com" would otherwise pass this check and

-// leave anyway.

-func safeReturn(r *http.Request, fallback string) string {

- _ = r.ParseForm()

- to := r.FormValue("return_to")

- if to == "" {

- to = r.URL.Query().Get("return_to")

- }

- if len(to) < 1 || to[0] != '/' {

- return fallback

- }

- if strings.HasPrefix(to, "//") || strings.HasPrefix(to, "/\\") {

- return fallback

- }

- return to

// index serves GET / — the calendar itself.

func (h *handlers) index(w http.ResponseWriter, r *http.Request) {

render(w, "index", h.page(r, "Calendar", nil))

diff --git a/internal/calendar/handlers_views.go b/internal/calendar/handlers_views.go

index 671c5cc..624e71a 100644

--- a/internal/calendar/handlers_views.go

+++ b/internal/calendar/handlers_views.go

@@ -29,7 +29,7 @@ func (h *handlers) day(w http.ResponseWriter, r *http.Request) {

p, _ := CurrentPerson(r)

day, err := ParseDate(chi.URLParam(r, "date"), p.Zone)

if err != nil {

- ErrorPage(w, r, http.StatusNotFound, "")

+ h.errorPage(w, r, http.StatusNotFound, "")

return

}

from, to, err := DayBounds(day, p.Zone)

@@ -71,7 +71,7 @@ func (h *handlers) week(w http.ResponseWriter, r *http.Request) {

p, _ := CurrentPerson(r)

anyDay, err := ParseDate(chi.URLParam(r, "date"), p.Zone)

if err != nil {

- ErrorPage(w, r, http.StatusNotFound, "")

+ h.errorPage(w, r, http.StatusNotFound, "")

return

}

start, err := WeekStartFor(anyDay, p.WeekStart, p.Zone)

@@ -140,7 +140,7 @@ func (h *handlers) month(w http.ResponseWriter, r *http.Request) {

p, _ := CurrentPerson(r)

month, err := ParseMonth(chi.URLParam(r, "month"), p.Zone)

if err != nil {

- ErrorPage(w, r, http.StatusNotFound, "")

+ h.errorPage(w, r, http.StatusNotFound, "")

return

}

from, to, err := MonthBounds(month, p.WeekStart, p.Zone)

@@ -158,7 +158,20 @@ func (h *handlers) month(w http.ResponseWriter, r *http.Request) {

h.fail(w, r, "build month", err)

return

}

- render(w, "month", h.page(r, page.Title, map[string]any{"Month": page, "TodayMonth": time.Now().In(month.Location()).Format("2006-01")}))

+ // The day the view switcher carries (?date=, from Day, Week or Agenda),

+ // so Month's Day and Week tabs go back to it. Outside this month, or

+ // absent: today if this is the current month, else the 1st.

+ now := time.Now().In(month.Location())

+ selected := month.Format("2006-01") + "-01"

+ if now.Format("2006-01") == month.Format("2006-01") {

+ selected = now.Format("2006-01-02")

+ }

+ if s := r.URL.Query().Get("date"); s != "" {

+ if d, err := ParseDate(s, p.Zone); err == nil && d.Format("2006-01") == month.Format("2006-01") {

+ selected = d.Format("2006-01-02")

+ }

+ render(w, "month", h.page(r, page.Title, map[string]any{"Month": page, "TodayMonth": now.Format("2006-01"), "SelectedISO": selected}))

}

// agenda lists the next fortnight.

@@ -202,5 +215,5 @@ func (h *handlers) agenda(w http.ResponseWriter, r *http.Request) {

// person or their content goes into it.

func (h *handlers) fail(w http.ResponseWriter, r *http.Request, stage string, err error) {

h.logger.Error("calendar: "+stage, "err", err)

- ErrorPage(w, r, http.StatusInternalServerError, "")

+ h.errorPage(w, r, http.StatusInternalServerError, "")

}

diff --git a/internal/calendar/static/calendar.css b/internal/calendar/static/calendar.css

index 2420df4..bd12658 100644

--- a/internal/calendar/static/calendar.css

+++ b/internal/calendar/static/calendar.css

@@ -26,6 +26,10 @@

* the bar's brand. */

[cal-view] { max-width: 90rem; } /* after tokens.css, so it beats rst-page's 64rem */

+/* The bar is sticky, so an anchor or :target lands below it, not under

+ * it (shared-chrome spec §5.3). */

+html { scroll-padding-block-start: var(--estilo-bar-block-size); }

/* The toolbar (layout.html "calendar-toolbar"): the steps at the start,

* the view switcher at the end, one row wherever it fits. Every label in

* it is the same in every view, so nothing in it moves. */

@@ -208,7 +212,9 @@

display: grid;

gap: 0;

grid-template-columns: 3.5rem 1fr;

- max-block-size: calc(100vh - 20rem);

+ /* The grid scrolls inside the first screen, but never below 24rem: on

+ * a short window (or a phone on its side) 100vh - 20rem is a sliver. */

+ max-block-size: max(24rem, calc(100vh - 20rem));

overflow: auto;

}

@@ -219,7 +225,9 @@

}

[cal-week-scroll] {

- max-block-size: calc(100vh - 20rem);

+ /* The grid scrolls inside the first screen, but never below 24rem: on

+ * a short window (or a phone on its side) 100vh - 20rem is a sliver. */

+ max-block-size: max(24rem, calc(100vh - 20rem));

overflow: auto;

}

diff --git a/internal/calendar/templates/agenda.html b/internal/calendar/templates/agenda.html

index 5b76339..f81e9b7 100644

--- a/internal/calendar/templates/agenda.html

+++ b/internal/calendar/templates/agenda.html

@@ -14,7 +14,7 @@

</div>

</header>

-{{template "calendar-toolbar" (dict "View" "agenda" "Noun" "fortnight" "Prev" (printf "/agenda?from=%s" .Data.PrevISO) "Today" "/agenda" "Next" (printf "/agenda?from=%s" .Data.NextISO) "Day" (printf "/day/%s" .Data.Agenda.FromISO) "Week" (printf "/week/%s" .Data.Agenda.FromISO) "Month" (printf "/month/%s" (slice .Data.Agenda.FromISO 0 7)) "Agenda" (printf "/agenda?from=%s" .Data.Agenda.FromISO))}}

+{{template "calendar-toolbar" (dict "View" "agenda" "Noun" "fortnight" "Prev" (printf "/agenda?from=%s" .Data.PrevISO) "Today" "/agenda" "Next" (printf "/agenda?from=%s" .Data.NextISO) "Day" (printf "/day/%s" .Data.Agenda.FromISO) "Week" (printf "/week/%s" .Data.Agenda.FromISO) "Month" (printf "/month/%s?date=%s" (slice .Data.Agenda.FromISO 0 7) .Data.Agenda.FromISO) "Agenda" (printf "/agenda?from=%s" .Data.Agenda.FromISO))}}

{{/* The agenda's scope, below the toolbar like the day's sentence. */}}

<p cal-sentence cal-sentence-muted>The next fortnight, and only the days with something in them.</p>

diff --git a/internal/calendar/templates/day.html b/internal/calendar/templates/day.html

index 28a0651..3184cb5 100644

--- a/internal/calendar/templates/day.html

+++ b/internal/calendar/templates/day.html

@@ -14,7 +14,7 @@

</div>

</header>

-{{template "calendar-toolbar" (dict "View" "day" "Noun" "day" "Prev" (printf "/day/%s" .Data.PrevISO) "Today" (printf "/day/%s" .Data.TodayISO) "Next" (printf "/day/%s" .Data.NextISO) "Day" (printf "/day/%s" .Data.Day.ISO) "Week" (printf "/week/%s" .Data.Day.ISO) "Month" (printf "/month/%s" (slice .Data.Day.ISO 0 7)) "Agenda" (printf "/agenda?from=%s" .Data.Day.ISO))}}

+{{template "calendar-toolbar" (dict "View" "day" "Noun" "day" "Prev" (printf "/day/%s" .Data.PrevISO) "Today" (printf "/day/%s" .Data.TodayISO) "Next" (printf "/day/%s" .Data.NextISO) "Day" (printf "/day/%s" .Data.Day.ISO) "Week" (printf "/week/%s" .Data.Day.ISO) "Month" (printf "/month/%s?date=%s" (slice .Data.Day.ISO 0 7) .Data.Day.ISO) "Agenda" (printf "/agenda?from=%s" .Data.Day.ISO))}}

{{/* The day's own sentence. Not a caption — it is the answer to the

question somebody opened this page to ask. Below the toolbar, so its

diff --git a/internal/calendar/templates/month.html b/internal/calendar/templates/month.html

index a8f75b8..d21a2e0 100644

--- a/internal/calendar/templates/month.html

+++ b/internal/calendar/templates/month.html

@@ -14,7 +14,7 @@

</div>

</header>

-{{template "calendar-toolbar" (dict "View" "month" "Noun" "month" "Prev" (printf "/month/%s" .Data.Month.PrevISO) "Today" (printf "/month/%s" .Data.TodayMonth) "Next" (printf "/month/%s" .Data.Month.NextISO) "Day" (printf "/day/%s-01" .Data.Month.ISO) "Week" (printf "/week/%s-01" .Data.Month.ISO) "Month" (printf "/month/%s" .Data.Month.ISO) "Agenda" (printf "/agenda?from=%s-01" .Data.Month.ISO))}}

+{{template "calendar-toolbar" (dict "View" "month" "Noun" "month" "Prev" (printf "/month/%s" .Data.Month.PrevISO) "Today" (printf "/month/%s" .Data.TodayMonth) "Next" (printf "/month/%s" .Data.Month.NextISO) "Day" (printf "/day/%s" .Data.SelectedISO) "Week" (printf "/week/%s" .Data.SelectedISO) "Month" (printf "/month/%s?date=%s" .Data.Month.ISO .Data.SelectedISO) "Agenda" (printf "/agenda?from=%s" .Data.SelectedISO))}}

{{/* A real table. A month IS tabular data — seven weekday columns

against week rows — so the headers come free for a screen reader,

diff --git a/internal/calendar/templates/week.html b/internal/calendar/templates/week.html

index 033ff83..eabef19 100644

--- a/internal/calendar/templates/week.html

+++ b/internal/calendar/templates/week.html

@@ -15,7 +15,7 @@

</div>

</header>

-{{template "calendar-toolbar" (dict "View" "week" "Noun" "week" "Prev" (printf "/week/%s" .Data.PrevISO) "Today" (printf "/week/%s" .Data.TodayISO) "Next" (printf "/week/%s" .Data.NextISO) "Day" (printf "/day/%s" .Data.StartISO) "Week" (printf "/week/%s" .Data.StartISO) "Month" (printf "/month/%s" (slice .Data.StartISO 0 7)) "Agenda" (printf "/agenda?from=%s" .Data.StartISO))}}

+{{template "calendar-toolbar" (dict "View" "week" "Noun" "week" "Prev" (printf "/week/%s" .Data.PrevISO) "Today" (printf "/week/%s" .Data.TodayISO) "Next" (printf "/week/%s" .Data.NextISO) "Day" (printf "/day/%s" .Data.StartISO) "Week" (printf "/week/%s" .Data.StartISO) "Month" (printf "/month/%s?date=%s" (slice .Data.StartISO 0 7) .Data.StartISO) "Agenda" (printf "/agenda?from=%s" .Data.StartISO))}}

{{/* The week keeps its two dimensions on a phone and scrolls sideways.

Flattening it to a list would make it the agenda, which already

diff --git a/internal/calendar/theme.go b/internal/calendar/theme.go

index 5789b64..7fa054b 100644

--- a/internal/calendar/theme.go

+++ b/internal/calendar/theme.go

@@ -5,110 +5,12 @@ import (

"net/http"

)

-// modeCookie carries the reader's light/dark choice. A cookie, not a

-// row: the choice belongs to a browser rather than to an account, it

-// must be readable server-side before the first byte of HTML is

-// written, and a signed-out visitor on the sign-in page has no row to

-// read it from.

-const modeCookie = "calendar_mode"

-// Three states, not two. "auto" is the absence of a stored choice and

-// renders as the ABSENCE of the data-theme attribute — that is the

-// mechanism, not an omission. With no attribute the browser resolves

-// against the operating system through the theme's own

-// `color-scheme: light dark`; with one, the two rules at the foot of

-// theme.css re-resolve every light-dark() in the file and the explicit

-// choice beats the OS in both directions.

-//

-// The bug this names: data-theme takes the light/dark SCHEME, never

-// rastrillo's theme NAME. "day", "plain" and "signal" are palette

-// files picked once at scaffold time; feeding one of those to the

-// attribute matches neither toggle rule, so the switcher silently does

-// nothing. Sheets shipped exactly that and wrote a test to stop it

-// coming back.

-const (

- modeAuto = "auto"

- modeLight = "light"

- modeDark = "dark"

-// themeAttr maps a stored mode onto the value of <html data-theme>.

-// Auto — and anything unrecognised, including a hand-edited cookie —

-// maps to the empty string, which the layout renders as no attribute

-// at all.

-func themeAttr(mode string) string {

- switch mode {

- case modeLight:

- return modeLight

- case modeDark:

- return modeDark

- default:

- return ""

- }

-// readMode returns the mode this request should render in. A missing or

-// unreadable cookie is auto, which is the correct answer for a first

-// visit and for a hostile value alike.

-func readMode(r *http.Request) string {

- c, err := r.Cookie(modeCookie)

- if err != nil {

- return modeAuto

- }

- switch c.Value {

- case modeLight, modeDark:

- return c.Value

- default:

- return modeAuto

- }

-// mode handles POST /mode: the whole switcher, as a plain form. No

-// script, deliberately — somebody who needs a high-contrast theme is

-// not well served by being told to enable JavaScript first.

-//

-// It answers 303 back to where the person was, so the choice takes

-// effect on the page they were reading rather than dropping them at

-// the root.

-func (h *handlers) mode(w http.ResponseWriter, r *http.Request) {

- _ = r.ParseForm()

- mode := r.FormValue("mode")

- switch mode {

- case modeLight, modeDark:

- http.SetCookie(w, &http.Cookie{

- Name: modeCookie,

- Value: mode,

- Path: "/",

- MaxAge: int(oneYearSeconds),

- HttpOnly: true,

- SameSite: http.SameSiteLaxMode,

- Secure: h.secure(),

- })

- default:

- // Auto is stored as the absence of the cookie, so choosing it

- // deletes rather than writes. One representation of "no

- // choice", so a cookie can never disagree with its own absence.

- http.SetCookie(w, &http.Cookie{

- Name: modeCookie,

- Value: "",

- Path: "/",

- MaxAge: -1,

- HttpOnly: true,

- SameSite: http.SameSiteLaxMode,

- Secure: h.secure(),

- })

- }

- http.Redirect(w, r, safeReturn(r, "/"), http.StatusSeeOther)

-// appearance is the shared account menu's three-state preference. The old

-// Calendar-only /mode and calendar_mode cookie deliberately do not survive:

-// the suite cookie gives every adopter the same validated return behaviour.

+// appearance handles POST /appearance, the account menu's Light / Dark /

+// System group (shared-chrome spec §8.3). estilo validates the mode and the

+// return path and sets the suite's estilo_appearance cookie; the layout

+// writes <html data-theme> from it with chrome.ThemeAttr, so "System" is

+// the ABSENCE of the attribute. Calendar's own /mode, calendar_mode cookie

+// and "Auto" label are retired.

func (h *handlers) appearance(w http.ResponseWriter, r *http.Request) {

chrome.HandleAppearance(w, r)

}

-// oneYearSeconds is the mode cookie's lifetime. A display preference

-// with no expiry to speak of; a year is simply longer than anyone

-// leaves a browser profile alone.

-const oneYearSeconds = 365 * 24 * 60 * 60

diff --git a/internal/calendartest/view_tabs_test.go b/internal/calendartest/view_tabs_test.go

new file mode 100644

index 0000000..4530574

--- /dev/null

+++ b/internal/calendartest/view_tabs_test.go

@@ -0,0 +1,42 @@

+package calendartest

+import (

+ "regexp"

+ "testing"

+// The view switcher keeps the date you were looking at, through Month too.

+// A month URL names only the month, so the switcher carries the day as

+// ?date= and Month's Day and Week tabs go back to it, not to the 1st.

+func TestViewTabsKeepTheSelectedDateThroughMonth(t *testing.T) {

+ app, session := signedIn(t, "someone@example.com")

+ h := app.Handler()

+ tab := func(path, label string) string {

+ t.Helper()

+ rec := getAs(t, h, session, path)

+ if rec.Code != 200 {

+ t.Fatalf("GET %s: %d", path, rec.Code)

+ }

+ m := regexp.MustCompile(`<a cal-tab="` + label + `" href="([^"]*)"`).FindStringSubmatch(rec.Body.String())

+ if m == nil {

+ t.Fatalf("GET %s: no %s tab", path, label)

+ }

+ return m[1]

+ }

+ for _, c := range []struct{ path, label, want string }{

+ {"/day/2026-09-24", "Month", "/month/2026-09?date=2026-09-24"},

+ {"/week/2026-09-24", "Month", "/month/2026-09?date=2026-09-21"},

+ {"/agenda?from=2026-09-24", "Month", "/month/2026-09?date=2026-09-24"},

+ {"/month/2026-09?date=2026-09-24", "Day", "/day/2026-09-24"},

+ {"/month/2026-09?date=2026-09-24", "Week", "/week/2026-09-24"},

+ {"/month/2026-09?date=2026-09-24", "Agenda", "/agenda?from=2026-09-24"},

+ // A date outside the month, or none, falls back to the 1st (this

+ // month is not the current one, where it would be today).

+ {"/month/2026-09?date=2026-10-02", "Day", "/day/2026-09-01"},

+ {"/month/2020-02", "Week", "/week/2020-02-01"},

+ } {

+ if got := tab(c.path, c.label); got != c.want {

+ t.Errorf("%s: %s tab = %s, want %s", c.path, c.label, got, c.want)

+ }