diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-09-20 01:32:44 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-09-20 01:32:44 +0000 |
| commit | b970be11ee895c34e019231022e9e5244e4aec2f (patch) | |
| tree | b6b21179c4eecb017843b5fc737c06ee2cbfbdc9 /internal | |
| parent | 36a82d934366cc2ddbd92ee3885b1c547eeb8244 (diff) | |
Stop per-store add forms jumping to the bottom of the page
The inline "Add to <store>..." forms submitted with a bare
hx-swap="innerHTML" against #shopping-items-list. Those forms live *inside*
that target, so the swap destroys the form that submitted it and the page ends
up scrolled to the bottom rather than staying on the store being added to. The
quick-add form at the top never had this because it sits outside the target and
already carried an explicit scroll: modifier.
Each store section now has a stable id and its inline form scrolls back to it,
so adding to a store keeps that store in view.
Adds a `slug` template function for the id: store names are user data
("Trader Joe's", "Hardware Store") going straight into element ids, so anything
outside [a-z0-9-] is collapsed to a dash and an empty result becomes "unnamed"
rather than an invalid, shared anchor.
Tests render the real template with the real func map, and assert every inline
add form has a scroll target — the bug is trivially reintroduced by copying a
section.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
Diffstat (limited to 'internal')
| -rw-r--r-- | internal/handlers/handlers.go | 28 | ||||
| -rw-r--r-- | internal/handlers/shopping_template_test.go | 127 |
2 files changed, 155 insertions, 0 deletions
diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index e70650d..c5df00d 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -1251,6 +1251,34 @@ func templateFuncMap() template.FuncMap { return template.FuncMap{ "subtract": func(a, b int) int { return a - b }, "add": func(a, b int) int { return a + b }, + // slug turns a free-text name into an id-safe fragment, so a template + // can give a section a stable anchor and target it by selector. Store + // names are user data ("Trader Joe's", "Hardware Store") and go + // straight into element ids, so anything outside [a-z0-9-] has to go. + "slug": func(s string) string { + var b strings.Builder + lastDash := true // leading dashes are suppressed + for _, r := range strings.ToLower(s) { + switch { + case (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9'): + b.WriteRune(r) + lastDash = false + default: + // Collapse any run of separators/punctuation into one dash. + if !lastDash { + b.WriteByte('-') + lastDash = true + } + } + } + out := strings.Trim(b.String(), "-") + if out == "" { + // An empty id would make every such section share the same + // (invalid) anchor, so give it something addressable. + return "unnamed" + } + return out + }, // multiDayLabel returns the "(starts/ends HH:MM)" suffix for a // multi-day calendar event's row on the day it's rendered, or "" // for a normal item or a "spans" (pass-through) day, which shows diff --git a/internal/handlers/shopping_template_test.go b/internal/handlers/shopping_template_test.go new file mode 100644 index 0000000..06f63b3 --- /dev/null +++ b/internal/handlers/shopping_template_test.go @@ -0,0 +1,127 @@ +package handlers + +import ( + "bytes" + "html/template" + "path/filepath" + "strings" + "testing" + + "task-dashboard/internal/models" +) + +// renderShoppingTab renders the real shopping-tab template with the real func +// map, so these assertions cover what actually ships rather than a stand-in. +func renderShoppingTab(t *testing.T, data any) string { + t.Helper() + dir := filepath.Join("..", "..", "web", "templates") + + tmpl, err := template.New("").Funcs(templateFuncMap()).ParseGlob(filepath.Join(dir, "*.html")) + if err != nil { + t.Fatalf("parsing top-level templates: %v", err) + } + if _, err := tmpl.ParseGlob(filepath.Join(dir, "partials", "*.html")); err != nil { + t.Fatalf("parsing partials: %v", err) + } + + var buf bytes.Buffer + if err := tmpl.ExecuteTemplate(&buf, "shopping-tab", data); err != nil { + t.Fatalf("executing shopping-tab: %v", err) + } + return buf.String() +} + +func twoStoreTabData() any { + return struct { + Stores []models.ShoppingStore + Grouped bool + }{ + Stores: []models.ShoppingStore{ + {Name: "Costco", Categories: []models.ShoppingCategory{ + {Name: "", Items: []models.UnifiedShoppingItem{ + {ID: "user-1", Name: "Milk", Store: "Costco", Source: "user"}, + }}, + }}, + {Name: "Hardware Store", Categories: []models.ShoppingCategory{ + {Name: "", Items: []models.UnifiedShoppingItem{ + {ID: "user-2", Name: "Screws", Store: "Hardware Store", Source: "user"}, + }}, + }}, + }, + Grouped: true, + } +} + +// The inline per-store add form used to submit with a bare +// hx-swap="innerHTML". Because that form lives *inside* the swap target +// (#shopping-items-list), submitting it destroys the form mid-request and the +// page ends up scrolled to the bottom instead of staying where the user was. +// The quick-add form at the top already pinned this with an explicit +// scroll: modifier; the per-store forms did not. +func TestShoppingTab_InlineAddScrollsToItsOwnStore(t *testing.T) { + html := renderShoppingTab(t, twoStoreTabData()) + + for _, want := range []string{ + `id="store-costco"`, + `id="store-hardware-store"`, + `scroll:#store-costco:top`, + `scroll:#store-hardware-store:top`, + } { + if !strings.Contains(html, want) { + t.Errorf("rendered shopping tab is missing %q", want) + } + } +} + +// Every inline add form must carry a scroll target. A form without one is the +// bug, and it is easy to reintroduce by adding a new store section by copy. +func TestShoppingTab_EveryInlineAddFormHasAScrollTarget(t *testing.T) { + html := renderShoppingTab(t, twoStoreTabData()) + + // Count the class attribute specifically — a bare "inline-add-form" also + // matches the string literal in the page's own JS handler. + forms := strings.Count(html, `class="inline-add-form`) + scrolls := strings.Count(html, "scroll:#store-") + if forms == 0 { + t.Fatal("expected at least one inline add form in the grouped view") + } + if forms != scrolls { + t.Errorf("found %d inline add forms but %d scroll targets; every form needs one", + forms, scrolls) + } +} + +func TestSlug(t *testing.T) { + slug, ok := templateFuncMap()["slug"].(func(string) string) + if !ok { + t.Fatal("templateFuncMap has no slug function of type func(string) string") + } + + cases := []struct{ in, want string }{ + {"Costco", "costco"}, + {"Hardware Store", "hardware-store"}, + {"Trader Joe's", "trader-joe-s"}, + {" Spaced Out ", "spaced-out"}, + {"Multiple---Dashes", "multiple-dashes"}, + // Non-ASCII is treated as a separator rather than transliterated. That + // keeps the id valid without pulling in a transliteration table; the + // result only has to be stable and unique, not pretty. + {"Ünïcode Störe", "n-code-st-re"}, + {"", "unnamed"}, + {"!!!", "unnamed"}, + } + for _, c := range cases { + if got := slug(c.in); got != c.want { + t.Errorf("slug(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +// Two stores whose names differ only by punctuation must not collide into the +// same element id, or the scroll target becomes ambiguous. +func TestSlug_DistinguishesSimilarNames(t *testing.T) { + slug := templateFuncMap()["slug"].(func(string) string) + if slug("Store A") == slug("Store B") { + t.Error("distinct store names produced the same slug") + } +} |
