diff --git a/packages/element/src/axis.ts b/packages/element/src/axis.ts index ce856f9..5f99be8 100644 --- a/packages/element/src/axis.ts +++ b/packages/element/src/axis.ts @@ -49,9 +49,75 @@ const LABEL_BUDGETS = [24, 16, 10] as const; /** Top margin kept clear, so the tallest row is not flush with the edge. */ const HEAD_ROOM = 18; -/** Rough width of the label font, for measuring without laying anything out. */ -const CHAR_PX = 7.2; -const LABEL_PADDING = 20; +/** The label's own horizontal padding, from the stylesheet, plus a little slack. */ +const LABEL_PADDING = 18; + +/** Used only until a real font can be read, and if canvas is unavailable. */ +const FALLBACK_CHAR_PX = 7.2; + +/** + * How wide a headline will actually be. + * + * Rows are assigned from label widths, and the widths were previously guessed + * at a flat 7.2 pixels per character. That number was measured against one + * sans-serif at one size, so any theme that changes `--bt-font` or the label + * size got a layout computed from a lie: too wide and the axis wastes rows, + * too narrow and labels overlap the neighbours the packing was supposed to + * keep them away from. A serif theme ships in this repo, so the mismatch was + * not hypothetical. + * + * Canvas measures the real string in the real font without laying anything out, + * which matters because this runs inside a per-frame loop. Results are cached + * per string, and the cache is dropped whenever the font changes. + */ +class TextMeasure { + #context: CanvasRenderingContext2D | null = null; + #cache = new Map(); + #font = ""; + + constructor(private readonly doc: Document) { + try { + this.#context = doc.createElement("canvas").getContext("2d"); + } catch { + // No canvas is not a reason to fail; the estimate below still draws. + this.#context = null; + } + } + + /** + * Points the measurer at whatever the labels are actually rendering in. + * + * Composed from the longhand properties rather than read from the `font` + * shorthand: the shorthand comes back empty in some browsers when the pieces + * were set individually, which is exactly how a stylesheet sets them. + */ + useFontOf(element: Element): void { + const style = getComputedStyle(element); + const font = `${style.fontStyle} ${style.fontWeight} ${style.fontSize} ${style.fontFamily}`; + + if (font === this.#font) return; + this.#font = font; + this.#cache.clear(); + if (this.#context) this.#context.font = font; + } + + /** Throws away cached widths — for when a web font finishes loading. */ + invalidate(): void { + this.#cache.clear(); + } + + width(text: string): number { + const cached = this.#cache.get(text); + if (cached !== undefined) return cached; + + const measured = this.#context + ? this.#context.measureText(text).width + : text.length * FALLBACK_CHAR_PX; + + this.#cache.set(text, measured); + return measured; + } +} /** How wide a label may grow when the pointer is on it. */ const HOVER_MAX_PX = 460; @@ -86,11 +152,13 @@ export class Axis { #events: TimelineEvent[] = []; #scale: Scale = { segments: [], total: 1, range: 1, maxPxPerUnit: 1400 }; + #measure: TextMeasure; constructor( private readonly doc: Document, private readonly onSelect: (id: string) => void, ) { + this.#measure = new TextMeasure(doc); this.el = doc.createElement("div"); this.el.className = "bt-axis"; this.el.tabIndex = 0; @@ -239,9 +307,25 @@ export class Axis { } } + /** + * Drops cached text widths, so the next frame measures again. + * + * Called when a web font finishes loading: everything measured before that + * was measured in the fallback face, and the rows were assigned from it. + */ + invalidateMetrics(): void { + this.#measure.invalidate(); + } + #renderMarkers(state: AxisState): void { const { pxPerUnit, offset, width, height, selectedId } = state; + // Measure in whatever the labels are really rendering in. Read from a live + // label rather than from a detached probe, so a page that themes the + // element through `--bt-font` is measured in its font and not the default. + const sample = this.#markersLayer.querySelector(".bt-label"); + if (sample?.isConnected) this.#measure.useFontOf(sample); + const rows = Math.max( 1, Math.floor((height - RULER - HEAD_ROOM) / LANE_HEIGHT), @@ -256,9 +340,12 @@ export class Axis { // Two widths per event: what it takes collapsed, and what it takes with // the whole headline showing. Rows are decided on the collapsed one so // the stack does not reshuffle every time the selection moves. - const widthOf = (chars: number) => chars * CHAR_PX + LABEL_PADDING; - const shut = widthOf(Math.min(event.headline.length, budget)); - const open = widthOf(event.headline.length); + // The truncated string is measured, not the character count guessed at: + // "Illinois" and "WWW" are the same length and nothing like the same + // width, and a proportional font is the normal case. + const shut = + this.#measure.width(event.headline.slice(0, budget)) + LABEL_PADDING; + const open = this.#measure.width(event.headline) + LABEL_PADDING; // A label is centred on a point and left-aligned on a span, so its box // is measured from wherever it actually starts. diff --git a/packages/element/src/element.ts b/packages/element/src/element.ts index 093f0d6..fb9c767 100644 --- a/packages/element/src/element.ts +++ b/packages/element/src/element.ts @@ -131,6 +131,22 @@ export class BestTimeElement extends HTMLElement { document.addEventListener("keydown", this.#onKey); + // Rows are assigned from measured label widths, and a web font that arrives + // after the first paint changes every one of them. Without this the layout + // stays committed to widths measured in the fallback face, which on a page + // using a display font means labels that overlap or rows that are half + // empty. Optional chaining because `document.fonts` is absent in some + // embedded webviews. + document.fonts?.ready + .then(() => { + if (!this.isConnected) return; + this.#axis.invalidateMetrics(); + this.#schedule(); + }) + .catch(() => { + // A font that never resolves is not a reason to break the timeline. + }); + if (!this.#events.length) void this.load(); } diff --git a/scripts/seed-issues.mjs b/scripts/seed-issues.mjs new file mode 100644 index 0000000..5f0c816 --- /dev/null +++ b/scripts/seed-issues.mjs @@ -0,0 +1,355 @@ +/** + * Files the starter issues. + * + * An empty issue tracker tells a visitor there is nothing to do here. A dozen + * real, scoped issues tells them where to start — so this exists to make the + * boring part of launching a project take seconds instead of half an hour of + * copy-paste into a browser form. + * + * Run it once: + * + * node scripts/seed-issues.mjs # create everything + * node scripts/seed-issues.mjs --dry-run # show what would be created + * node scripts/seed-issues.mjs --print # dump markdown to paste by hand + * + * Needs the GitHub CLI (`gh auth login`) for anything but --print. + */ +import { spawnSync } from "node:child_process"; + +const REPO = "odiwr/BestTime"; +const BLOB = `https://github.com/${REPO}/blob/main`; + +const args = new Set(process.argv.slice(2)); +const DRY = args.has("--dry-run"); +const PRINT = args.has("--print"); + +/** + * Labels that must exist before an issue can carry them. + * + * GitHub ships `good first issue`, `help wanted`, `enhancement`, + * `documentation` and `bug` already, so only the project's own are here. + * Creating one that exists is not an error worth stopping for. + */ +const LABELS = [ + ["media-adapter", "0e8a16", "Support for embedding a new media host"], + ["theme", "d4c5f9", "CSS only — no JavaScript needed"], + ["parser", "fbca04", "Reading dates, sheets, and CSV"], + ["discussion", "c2e0c6", "Needs agreement on approach before code"], +]; + +const ISSUES = [ + { + title: "Media: support Mastodon posts", + labels: ["good first issue", "media-adapter"], + body: `A Mastodon post URL should embed in the media pane. + +Mastodon is federated, so there is no domain list to match against — the test +has to be shape-based, something like \`/@user/123456789\` at the end of the +path. Mastodon instances serve oEmbed at \`/api/oembed?url=…\` with permissive +CORS, which is what makes this possible from the browser. + +**Where to start** + +- [docs/media-adapters.md](${BLOB}/docs/media-adapters.md) — the walkthrough +- [packages/core/src/media.ts](${BLOB}/packages/core/src/media.ts) — the existing adapters +- [packages/core/test/parse.test.ts](${BLOB}/packages/core/test/parse.test.ts) — where the test goes + +Roughly fifteen lines plus a test. Say so here and it is yours.`, + }, + { + title: "Media: support Spotify tracks and albums", + labels: ["good first issue", "media-adapter"], + body: `\`open.spotify.com/track/ID\` should embed as \`open.spotify.com/embed/track/ID\`. +Same for \`/album/\`, \`/playlist/\` and \`/episode/\`. + +Probably the simplest adapter in the list — a regex and a string. Good first +issue if you have never touched this codebase. + +See [docs/media-adapters.md](${BLOB}/docs/media-adapters.md).`, + }, + { + title: "Media: work out whether Bandcamp can be embedded", + labels: ["media-adapter", "help wanted"], + body: `Bandcamp is currently *claimed* by an adapter but returns \`null\`, so it falls +through to a plain link. + +The reason: their player embed needs a numeric album id that only appears in the +page's own markup, and a bare album URL cannot be turned into a player from +outside without fetching and scraping the page. + +**The actual task is research, not code.** Does their oEmbed endpoint give up +the id, and does it send CORS headers permissive enough to be called from a +reader's browser? + +"No, it is not possible" is a completely acceptable outcome — write down what +you found and we will note it in the docs. + +See the \`bandcamp\` entry in [packages/core/src/media.ts](${BLOB}/packages/core/src/media.ts).`, + }, + { + title: "Themes: a dark one, a newsroom one, and something fun", + labels: ["good first issue", "theme"], + body: `[themes/](${BLOB}/themes) has three: default, parchment, and high-contrast. + +Wanted: +- A proper dark theme (not just the automatic dark variant) +- Something sober, for newsrooms and institutions +- Something loud and fun + +**No JavaScript required.** A theme is one CSS file of custom properties. Copy +[themes/default.css](${BLOB}/themes/default.css), change the values, save it +under a new name, open a PR with a screenshot. + +One theme per PR, please — easier to review and to credit.`, + }, + { + title: "A usable axis on phones", + labels: ["enhancement", "help wanted"], + body: `Below 640px the axis is hidden entirely and readers get only the previous/next +arrows. That was a deliberate call for the first version — a draggable, +zoomable ruler eight rows deep is not usable at 375px, and faking one costs +more than it gives. + +It is also the biggest gap in the project, and probably affects half the +traffic of any public timeline. + +**Needs** +- Pinch-to-zoom and drag-to-pan via pointer events +- A layout that survives 375px — likely fewer rows and shorter labels +- Not breaking the desktop behaviour + +**Where** +- \`@container (min-width: 640px)\` guard on \`.bt-axis\` in [packages/element/src/styles.ts](${BLOB}/packages/element/src/styles.ts) +- \`#bindAxis\` in [packages/element/src/element.ts](${BLOB}/packages/element/src/element.ts) + +Worth commenting with your approach before writing much.`, + }, + { + title: "Dates in languages other than English", + labels: ["help wanted", "parser", "discussion"], + body: `[packages/core/src/prose.ts](${BLOB}/packages/core/src/prose.ts) understands +English month names and English words for periods — "1970s", "21st century", +"present", "late", "mid", "early". + +A sheet written in French, Spanish, German or anything else falls back to a +bare year at best, and to nothing at all at worst. + +**This wants a design discussion before code.** The likely shape is a pluggable +locale table rather than a growing pile of regexes, plus a \`locale\` attribute +on the element. Non-Gregorian calendars are a separate and much larger question. + +Comment with your thinking before starting — this is the kind of change that is +painful to redo.`, + }, + { + title: "Honour the Group column to build parallel rows", + labels: ["enhancement", "discussion"], + body: `TimelineJS sheets have a \`Group\` column. BestTime currently ignores it. + +Honouring it would mean lanes are assigned by group rather than purely by +packing — one row per country, per team, per person — which turns a single +timeline into a comparison of several. + +**Design questions to settle first** +- Does a group get a fixed row, or a band of rows it packs within? +- What happens when a group needs more rows than fit? +- Are groups labelled down the left edge, and does that eat horizontal space? + +Affects \`packLanes\` in [packages/core/src/scale.ts](${BLOB}/packages/core/src/scale.ts) +and the layout in [packages/element/src/axis.ts](${BLOB}/packages/element/src/axis.ts). + +Discussion first, please.`, + }, + { + title: "Read inline data from a + +\`\`\` + +The parsing already exists — \`eventsFromObjects\` in +[packages/core/src/parse.ts](${BLOB}/packages/core/src/parse.ts). This is about +reading the child script in \`connectedCallback\` and using it in preference to +\`src\` when both are present. + +**Watch out for:** the child may not be parsed yet when \`connectedCallback\` +runs for an element in the initial HTML. Worth checking what the timing +actually is rather than assuming. + +See \`connectedCallback\` and \`load()\` in [packages/element/src/element.ts](${BLOB}/packages/element/src/element.ts).`, + }, + { + title: "Export the current view as an image", + labels: ["enhancement", "help wanted"], + body: `"Can I download this timeline as a PNG" is the most predictable feature request +a project like this gets, and it is worth having an answer ready. + +Two plausible approaches: +- Redraw the axis to a \`\` — full control, duplicates the layout code +- Serialise to SVG with \`\` — reuses the DOM, patchy support + +Both have to deal with cross-origin images in the media pane, which will taint +a canvas and block export. + +Worth a comment describing the approach before building it.`, + }, + { + title: "Audit keyboard and screen-reader support", + labels: ["accessibility", "help wanted"], + body: `The axis has had reasonable-but-unverified accessibility work: markers are real +\`