From 616aa3a934951260d5daa81ab3d0f93fbc94ebee Mon Sep 17 00:00:00 2001 From: grm Date: Thu, 17 Sep 2026 18:54:33 +0300 Subject: Keep a chosen image older than the picker's list, and the layout on a rejected save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The image picker's select lists the newest recentImages library images and was meant to add the chosen one when it is older than those, but the fallback option was gated on the filename being unknown, which designData makes known: the option never rendered, the browser selected "None", and every save of the design form cleared a logo or favicon older than the 48 newest images (the post form's featured image the same way). The partial now checks whether the choice is listed and otherwise emits it, named. handleDesign ran the uploads before parsing the module and menu rows and gave up at the first problem, so a rejected upload rendered the form without any rows — and, since a missing row means delete, the next save would have wiped the layout and menu. Everything is read before the first problem is reported. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Sd8UPWrvyYCLj97JexNw3A --- AGENTS.md | 5 ++- internal/web/handlers_design.go | 60 +++++++++++++++----------- internal/web/templates/partials/imagepick.html | 14 +++--- internal/web/web_test.go | 36 ++++++++++++++++ 4 files changed, 83 insertions(+), 32 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5753d26..b7bd557 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -144,7 +144,10 @@ internal/web/ server.go (host router, middleware, render helpers) the modules of every area (with their settings inline, folded in a `
`) and the menu — stored atomically by `BlogStore.SaveDesign` (theme + full module list + full menu in one transaction: rows with an id - are updated, without one inserted, missing ones deleted). Sections are cards + are updated, without one inserted, missing ones deleted). Because a missing + row means delete, `handleDesign` reads *everything* — modules, menu, then + the uploads — before reporting the first problem, and the rejected form is + rendered from all of it; never bail out half-way. Sections are cards with ids (`designSections`) a sticky side index jumps to; the hidden `at` field says which one was in view so the flash returns there. Module rows are `mod..{id,area,kind,title,body,count,pos,del}` and menu rows diff --git a/internal/web/handlers_design.go b/internal/web/handlers_design.go index 5853436..acc7569 100644 --- a/internal/web/handlers_design.go +++ b/internal/web/handlers_design.go @@ -141,39 +141,49 @@ func (s *Server) handleDesign(w http.ResponseWriter, r *http.Request) { if p, ok := PresetByKey(r.FormValue("preset")); ok { v.Theme = v.Theme.WithPreset(p) } - // The form is shown again with everything that was sent, plus the message. - fail := func(msg string) { - d, err := s.designData(r, v) - if err != nil { - s.serverError(w, err) - return - } - d["error"] = msg - s.renderStatus(w, r, http.StatusBadRequest, "dashboard/design.html", d) - } - // Optional direct uploads from the pickers. - for field, dst := range map[string]*string{"bg_image_file": &v.Theme.BgImage, "header_image_file": &v.Theme.HeaderImage, "logo_file": &v.Theme.Logo, "favicon_file": &v.Theme.Favicon} { - img, err := s.readUpload(r, field, true) - if err != nil { - fail(err.Error()) - return - } - if img != nil { - *dst = img.ID.String() + // Every part of the form is read before any problem is reported, so the + // form shown again carries all of it: a module or menu row that was left + // out here would be deleted by the next save. The first problem is the + // one reported. + var problem error + note := func(err error) { + if problem == nil { + problem = err } } var err error - if v.Modules, err = parseModules(r.Form, lang, blog.Language); err != nil { - fail(err.Error()) - return - } + v.Modules, err = parseModules(r.Form, lang, blog.Language) + note(err) pages, err := blogStore(r).ListPages(r.Context()) if err != nil { s.serverError(w, err) return } - if v.Menu, err = parseMenu(r.Form, lang, pages); err != nil { - fail(err.Error()) + v.Menu, err = parseMenu(r.Form, lang, pages) + note(err) + // Optional direct uploads from the pickers. One that is stored stays + // chosen in the form even when something else is rejected. + for _, u := range []struct { + field string + dst *string + }{{"bg_image_file", &v.Theme.BgImage}, {"header_image_file", &v.Theme.HeaderImage}, {"logo_file", &v.Theme.Logo}, {"favicon_file", &v.Theme.Favicon}} { + img, err := s.readUpload(r, u.field, true) + if err != nil { + note(err) + continue + } + if img != nil { + *u.dst = img.ID.String() + } + } + if problem != nil { + d, err := s.designData(r, v) + if err != nil { + s.serverError(w, err) + return + } + d["error"] = problem.Error() + s.renderStatus(w, r, http.StatusBadRequest, "dashboard/design.html", d) return } if err := blogStore(r).SaveDesign(r.Context(), v.Theme.JSON(), v.Modules, v.Menu); err != nil { diff --git a/internal/web/templates/partials/imagepick.html b/internal/web/templates/partials/imagepick.html index ed9368c..5062d42 100644 --- a/internal/web/templates/partials/imagepick.html +++ b/internal/web/templates/partials/imagepick.html @@ -1,9 +1,11 @@ {{/* One image field of a form (the design page, a post's featured image). - The select lists the recent images (plus the chosen one) so it works - without the script; with it the select is hidden and "Choose from - library…" opens the form's imagelib panel, which fetches the library - page by page and marks the pickers with .js. */}} -{{define "imagepick"}}{{$cur := .current}}
+ The select lists the recent images so it works without the script; a + chosen image older than those gets its own option (named through .names, + or "Current image" once the file is gone), so the select always carries + the choice and a save keeps it. With the script the select is hidden and + "Choose from library…" opens the form's imagelib panel, which fetches the + library page by page and marks the pickers with .js. */}} +{{define "imagepick"}}{{$cur := .current}}{{$listed := false}}{{range .images}}{{if eq .ID.String $cur}}{{$listed = true}}{{end}}{{end}}
{{if .hint}}

{{.hint}}

{{end}}
@@ -12,7 +14,7 @@ diff --git a/internal/web/web_test.go b/internal/web/web_test.go index b707339..fcd1e41 100644 --- a/internal/web/web_test.go +++ b/internal/web/web_test.go @@ -691,3 +691,39 @@ func TestPostSummary(t *testing.T) { t.Error("summary: expected a cut") } } + +// A chosen image older than the recent ones the picker lists must still be +// the select's selected option, else saving the form clears it (the logo of +// a blog with more than recentImages images vanished on every save). +func TestImagePickKeepsOlderChoice(t *testing.T) { + tpl, err := newTemplates(false).get("en", "dashboard/design.html") + if err != nil { + t.Fatal(err) + } + recent := uuid.New() + old := uuid.New() + for _, tc := range []struct { + name string + names map[string]string + want string + }{ + {"known name", map[string]string{recent.String(): "new.png", old.String(): "logo.png"}, ``}, + {"unknown name", map[string]string{recent.String(): "new.png"}, ``}, + } { + var b strings.Builder + err := tpl.ExecuteTemplate(&b, "imagepick", map[string]any{ + "name": "logo", "current": old.String(), "sub": "x", "label": "Logo", + "images": []store.File{{ID: recent, Filename: "new.png"}}, "names": tc.names, + }) + if err != nil { + t.Fatal(err) + } + out := b.String() + if !strings.Contains(out, tc.want) { + t.Errorf("%s: the chosen image is not the selected option:\n%s", tc.name, out) + } + if strings.Contains(out, `