diff --git a/CHANGELOG.md b/CHANGELOG.md index 51cf6c9..73700f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Link cards show what distinguishes the link, not just its host. The display text was + the hostname alone, so every link to a platform rendered identically — two posts + pointing at different repositories both read `github.com`. The path is now kept, with + whole trailing segments dropped until it fits the pill, so a long path shortens rather + than truncating mid-word. Bare-domain links are unaffected. Applies to the feed card + and the article detail page, which share the helper. +- The link pill can no longer wrap to a second line or widen the page. It was + `width: fit-content` with no bound, so a long enough target grew past its card; + it now caps at its container with a single-line ellipsis, verified down to a 60px + container on both surfaces. - An unset `TEMPLATES_PATH` no longer globs the working directory. `filepath.Join("", "*.html")` is `"*.html"`, so any unrelated `.html` file in the directory markgo happened to start in was parsed as the template set, and boot failed with a set missing every diff --git a/internal/services/template.go b/internal/services/template.go index 7601113..8f114f0 100644 --- a/internal/services/template.go +++ b/internal/services/template.go @@ -967,17 +967,7 @@ var templateFuncs = template.FuncMap{ return t.Format("Jan 2") } }, - "extractDomain": func(urlStr string) string { - u, err := url.Parse(urlStr) - if err != nil { - return urlStr - } - host := u.Hostname() - if host == "" { - return urlStr - } - return strings.TrimPrefix(host, "www.") - }, + "extractDomain": extractLinkDisplay, "displayTitle": func(a *models.Article) string { return a.DisplayTitle() }, @@ -1007,6 +997,54 @@ var templateFuncs = template.FuncMap{ }, } +// linkDisplayCap bounds the link pill's text. The 320px card leaves the pill about +// 218px of text room once container padding, card padding, the pill's own padding, +// the gap and the arrow are subtracted — roughly 32 characters at --font-size-sm. +// CSS truncation is the structural guarantee that the pill never wraps; this cap is +// what keeps the text readable, because it drops whole path segments instead of +// cutting a word in half. +const linkDisplayCap = 32 + +// extractLinkDisplay renders a link target as display text for the link pill. +// +// The host alone is not enough when the host is a platform: two github.com links +// render identically and the path carries the entire identity. So the path is kept, +// and trailing segments are dropped whole until the result fits — a truncated +// segment reads worse than a shorter path. Scheme, "www.", fragment and trailing +// slash are always dropped. +// +// Query strings are deliberately not shown. A query wide enough to distinguish +// (news.ycombinator.com/item?id=12345) overflows the mobile card, and the card's +// title already carries primary identity — the pill is secondary context. +// +// Registered as "extractDomain" because operator templates loaded via +// TEMPLATES_PATH call it by that name; renaming would break them at render time. +func extractLinkDisplay(urlStr string) string { + u, err := url.Parse(urlStr) + if err != nil { + return urlStr + } + host := strings.TrimPrefix(u.Hostname(), "www.") + if host == "" { + return urlStr + } + + segments := strings.FieldsFunc( + strings.TrimSuffix(u.EscapedPath(), "/"), + func(r rune) bool { return r == '/' }, + ) + for len(segments) > 0 { + candidate := host + "/" + strings.Join(segments, "/") + if utf8.RuneCountInString(candidate) <= linkDisplayCap { + return candidate + } + segments = segments[:len(segments)-1] + } + // Host alone may still exceed the cap; CSS truncates it rather than this + // returning something that isn't a real hostname. + return host +} + // Helper function func toFloat(v any) (float64, bool) { switch v := v.(type) { diff --git a/internal/services/template_test.go b/internal/services/template_test.go index 41af46e..a2902c9 100644 --- a/internal/services/template_test.go +++ b/internal/services/template_test.go @@ -9,6 +9,7 @@ import ( "testing" "testing/fstest" "time" + "unicode/utf8" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -1207,3 +1208,63 @@ func TestLoadTemplates_EmptyPathIgnoresStrayHTMLInWorkingDir(t *testing.T) { assert.Nil(t, service.templates.Lookup("unrelated.html"), "a stray CWD file must never enter the template set") } + +func TestExtractLinkDisplay(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + // The bare-domain posts must render byte-identical to the host-only + // behaviour they had before, or this fix regresses seven cards to fix two. + {"bare host", "https://aipolicy.1mb.dev", "aipolicy.1mb.dev"}, + {"bare host trailing slash", "https://geetanjaliapp.com/", "geetanjaliapp.com"}, + {"www stripped", "https://www.example.com/", "example.com"}, + + // The defect: two platform links that both rendered "github.com". + {"platform path kept", "https://github.com/rupa/z", "github.com/rupa/z"}, + {"platform path kept, longer", "https://github.com/mattn/go-sqlite3", "github.com/mattn/go-sqlite3"}, + + {"fragment dropped", "https://go.dev/ref/mod#go-mod-file-retract", "go.dev/ref/mod"}, + + // Query deliberately not shown — it cannot fit the mobile pill, and the + // card title carries primary identity. + {"query dropped", "https://news.ycombinator.com/item?id=12345", "news.ycombinator.com/item"}, + + // Over the cap: whole segments are dropped, never a mid-word cut. + { + "deep path drops whole segments", + "https://example.com/a/very/deep/path/that/keeps/going/and/going/forever", + "example.com/a/very/deep/path", + }, + + // Degenerate inputs keep the previous passthrough behaviour. + {"not a url", "not a url", "not a url"}, + {"no host", "/relative/path", "/relative/path"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, extractLinkDisplay(tc.in)) + }) + } +} + +// Whatever the helper emits must fit the pill's character budget on its own. The +// CSS ellipsis is the safety net for pathological input; it should not be what +// keeps ordinary links readable. +func TestExtractLinkDisplay_StaysWithinPillBudget(t *testing.T) { + longs := []string{ + "https://example.com/a/very/deep/path/that/keeps/going/and/going/forever", + "https://github.com/some-org/some-really-long-repository-name-here", + "https://news.ycombinator.com/item?id=12345", + "https://go.dev/ref/mod#go-mod-file-retract", + } + for _, u := range longs { + got := extractLinkDisplay(u) + assert.LessOrEqual(t, utf8.RuneCountInString(got), linkDisplayCap, + "%q rendered %q, over the %d-char pill budget", u, got, linkDisplayCap) + assert.NotContains(t, got, "…", + "segments are dropped whole, so no ellipsis should reach the template") + } +} diff --git a/web/static/css/article.css b/web/static/css/article.css index 0db7f10..5b39cea 100644 --- a/web/static/css/article.css +++ b/web/static/css/article.css @@ -90,6 +90,9 @@ padding: var(--spacing-1) var(--spacing-3); background-color: var(--color-bg-tertiary); border-radius: var(--radius-md); + /* Bounds the pill to its container so a long target cannot widen the page. + See .feed-card-link-url — same contract, same reason. */ + max-width: 100%; transition: background-color var(--transition-fast); } @@ -100,11 +103,18 @@ .article-link-domain { font-weight: 500; + /* min-width:0 defeats the flex default of min-width:auto, without which the + item refuses to shrink below its content and the ellipsis never applies. */ + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; } .article-link-arrow { font-size: var(--font-size-xs); opacity: 0.7; + flex-shrink: 0; } /* AMA type — question is the header, answer is the body */ diff --git a/web/static/css/cards.css b/web/static/css/cards.css index c52fc65..28e1755 100644 --- a/web/static/css/cards.css +++ b/web/static/css/cards.css @@ -159,6 +159,10 @@ background-color: var(--color-bg-tertiary); border-radius: var(--radius-md); width: fit-content; + /* fit-content grows to the text, so without this a long target widens the pill + past the card and scrolls the page. The link helper caps its output, but CSS + is the guarantee — no input can wrap the pill to a second line. */ + max-width: 100%; transition: background-color var(--transition-fast); } @@ -169,11 +173,19 @@ .feed-card-domain { font-weight: 500; + /* min-width:0 is load-bearing — a flex item defaults to min-width:auto, which + refuses to shrink below its content and defeats the ellipsis entirely. */ + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; } .feed-card-external-icon { font-size: var(--font-size-xs); opacity: 0.7; + /* The arrow is the affordance; the text yields to it, never the reverse. */ + flex-shrink: 0; } /* AMA Card */