Conversation
The alt_text CSV column was collected but never written to generated object frontmatter, so image alt attributes and viewer aria-labels fell back to object titles. generate_collections.py now registers the column and emits it: user-written alt text reaches the objects gallery, object pages, and story steps. Originally fixed by Ana María Cárdenas on her 3D_refactor branch.
There was a problem hiding this comment.
Pull request overview
This PR updates the generate_collections.py build script to ensure object alt_text from JSON makes it into generated Jekyll object pages (front matter), aligning the site build output with existing accessibility support across templates and JS.
Changes:
- Bump the script docstring version marker to
v1.6.0. - Treat
alt_textas a first-class object field (exclude fromextra_metadata) and emit it in object page front matter when present.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…uist-generated Opening move of the v1.6.0 code-health release: the repository should be honest with a newcomer about what is generated and what is source. assets/js/telar-story.js is an esbuild bundle that reads like source. The build:js script now stamps a GENERATED banner with a rebuild pointer, .gitattributes marks bundle and sourcemap linguist-generated (collapsed diffs, honest language stats), and assets/js/README.md maps the directory: generated vs source vs vendored, what loads each file, how to rebuild.
Part of the v1.6.0 code-health pass: dead code is removed only after verification, not on a tool's word alone. No template includes it - the info overlay and credits badge it carried moved into _layouts/story.html during the card-stack rewrite, and the file kept receiving maintenance (v1.5.3 updated its loading text) without ever rendering. Verified: no static or dynamic include anywhere, no script, workflow, or docs reference. The v1.6.0 upgrade must delete it from user sites.
scripts/migrations/base.py and the v0.2.0-v0.4.2 migrations were the only scripts in the tree without version footers; each is stamped with the release that ships it. tests/unit/test_process_audio.py stamped v1.6.0.
has_latex scanned only *_text layer columns, so a story with LaTeX only in its step text never set the flag and KaTeX never loaded - raw $...$ shown, contradicting the documented behaviour (story step text: automatic). The scan now covers question and answer alongside the layer columns. Four regression tests.
…panel-state sync Three user-facing fixes: - Embed mode at widths >= 800px: .mobile-nav inherited the vertical-layout right pin, so the fixed box stretched across the viewport and its translateX(-50%) centring pushed the buttons off-screen. right: auto restores the intended geometry. - Button navigation now boots in the intro state when an intro card exists: the first next tap lands on step 1 (previously it jumped to step 2 and step 1 was unreachable going forward) and prev is disabled on the intro. Deep-link restoration still runs after init and overrides. - Panel state reconciles on hidden.bs.offcanvas: closing a panel with the offcanvas X bypassed closePanel(), leaving stale open-state that blocked keyboard reopening. Bundle rebuilt.
v1.6.0 code-health pass. Each removal was verified against cross-layer references (templates, workflows, generated markup, docs) before deletion, then gated on the unit suite and a full pipeline run. - discover_sheet_gids.py: the --output-env flag had no consumer in any workflow, script, or docs; removed along with the env-var name mapping that only its branch read. - fetch_google_sheets.py: instructions/instrucciones entries in system_tabs were unreachable - the skip check runs before the set is consulted. - generate_iiif.py: pre-v0.9.0 telar-content/images fallback (zero references anywhere) and edge-colour sampling whose info.json field nothing reads. - process_pdf.py: unused imports; the backend parameter was dead - PDF tiling always uses vips dzsave. - stories.py: unused import. search.py: the search index's demo field was always false (demo objects are tagged _demo) and nothing reads it.
v1.6.0 code-health pass: one declaration site per fact, so future edits can't silently miss a copy. Same-file deduplication, behaviour-preserving - pipeline output verified byte-identical before and after: - build_local_site.py: run_command/run_command_list share one private helper; both public signatures and the shell-safety rationale kept. - generate_collections.py: the frontmatter regex uses the compiled module constant instead of a second inline copy. - core.py: the christmas-tree if/else no longer duplicates whole loop bodies; process_func is chosen once. - csv_utils.py: is_header_row's supplemental name list trimmed to the three names not already in COLUMN_NAME_MAPPING. - demo.py: loop-invariant glossary dict hoisted out of the per-step loop. - markdown.py: the widgets/images/LaTeX/markdown pipeline (three copies) and the frontmatter+title parsing (two copies) factored into helpers. - objects.py: media-type lists imported from telar.media_type instead of re-typed literals. - objects.py: the seven HTTP-status warning branches collapse to a lookup. - stories.py: shared warn helper; coordinate defaults as one mapping.
…e in media_type The audio extension list was declared in four places, and the lowercase-only copies in process_audio.py and core.py fed exact-case existence probes. An uppercase-extension file (interview.MP3) classified as Audio by detect_media_type was missed by the audio manifest (object absent from window.audioObjects, so the story page fell back to IIIF rendering) and by peaks/clip generation - on case-sensitive filesystems, which is what CI builds on, while case-insensitive macOS masked it locally. All sites now import the both-case list from telar.media_type; lowercase precedence preserved; regression test added.
…lready states The docstring narrated the module's extraction history and enumerated two consumers when six modules now import it; it now states the actual constraint - single source of truth for media-type detection, kept dependency-free so any pipeline module can import it without a cycle. The pointer comment in generate_collections.py said only what its import line already says.
v1.6.0 code-health pass. CSS has no test coverage, so the gate here was mechanical: compiled-CSS diff with every hunk attributed, selector greps over the built site, and before/after screenshots. Removals, each verified producer-less by checkout-wide grep and post-build _site grep: - an orphaned filter-category implementation in _layout.scss (the live filters use section toggles + aria-expanded); its equal-specificity rules were silently overriding the live filter styling, so removal also restores the intended padding, text colour, and count size on the gallery filter sidebar - the comparison-widget block in _widgets.scss (~175 lines: no template, not registered in widgets.py, no JS) - mobile rules targeting bare .carousel-caption where the template emits carousel-caption-below - .panel-content (panels use ids), .share-success-message (copy feedback swaps an SVG), bare .objects-filter-toggle Consolidations, compiled output verified identical: - shared focus-ring mixin for the share panel's four inputs - light-glass-button mixin shared by .share-button and .btn-nav-back - carousel-caption and accordion panel-colour mixins alongside the existing tabs mixin, used by both #panel-layerN and .user-page - card layout values read the _responsive.scss variables; split declarations merged
The lockfile has pinned lenis 1.3.23 since the v1.5.2 dependency bump, but every shipped bundle was still built from a stale node_modules carrying 1.3.19 - npm ci && npm run build:js could not reproduce the committed artifact. Rebuilt from lockfile dependencies; the diff is confined to the bundled lenis.mjs/lenis-snap.mjs sections. Retested: 296 JS unit tests and the full scroll-driven end-to-end suite (navigation, panels, embed - 35 passed, 1 skipped) against the served build.
v1.6.0 code-health pass over the pre-module satellites. Every removal was verified against cross-layer references (layouts, includes, data attributes, generated markup) before deletion, then checked functionally in the browser (gallery filtering, layer panels, share panel). - telar-icons.js deleted: no layout or include loads it; the icons it served are inlined directly in the templates that use them. NOTICE re-pointed at the surviving inline Lucide copies (embed.js, share-panel.js, story-unlock.js, six templates); its row dropped from assets/js/README.md. The v1.6.0 migration must delete the file from user sites. - telar.js: PanelStack class and window.Telar.PanelStack removed - zero callers; live panel stacking is state.panelStack inside the bundled telar-story modules. - widgets.js: removed a panelLoaded listener for an event nothing dispatches; the header comment now states the actual init behaviour. - objects-filter.js: facetMap was built on every init but never read. - share-panel.js: three init()-scope constants never read in that scope; same-named locals in other functions untouched. Verified: 291 JS tests and a clean Jekyll build with no references to the deleted file in the built site.
v1.6.0 code-health pass over the bundled telar-story modules. Every
removal re-verified against all call sites before deletion; the bundle is
rebuilt from the edited sources.
- deep-link.js: removed a hash-update guard flag and its exported getter,
built for a hashchange listener that was never written; removed the
unimplemented ps{n} primary-source fragment token (nothing ever wrote
one, and no reader branch existed), simplifying the fragment writer to
the glossary-only form that was ever wired.
- iiif-card.js: removed _compensateForCardOverlay - production positioning
runs through _applyFocalTarget, which performs the same geometry
resolution; its test suite went with it.
- text-card.js: removed the superseded card-construction functions
(createTextCard, createFullObjectCard, activate/deactivate) - card
construction lives in card-pool.js, which never called these; the
full-object variant's CSS classes have no definitions anywhere.
isFullObjectMode remains the module's live export.
- utils.js: removed calculateViewportPosition, superseded by the
two-circle focal model in iiif-card.js; its tests went with it.
- panels.js: removed the unreachable 'glossary' branch of getPanelContent
(no call site passes 'glossary'; the glossary panel is driven directly
by telar.js) and documented the layer1/layer2-only contract.
- audio-card.js, video-card.js: play-overlay aria-labels now use the
state.objectsIndex O(1) lookup the rest of the engine uses, replacing
per-call array scans of window.objectsData.
Verified: 283 JS tests (the two deleted suites accounted for), clean
Jekyll build, and the full end-to-end suite against a served build -
story locking 17/17, navigation/panels/embed 35 passed, 1 skipped.
v1.6.0 code-health pass over the story engine's largest module. - Import list: four specifiers removed (destroyVideoPlayer, computeVideoLayout, destroyAudioPlayer, removeAudioClipEndDim) that were imported but never referenced; all four stay live in their home modules. - The three z-index functions marked "Legacy exports kept for existing tests" are gone, coverage first: their banding assertions now run against computeZIndexPlan, the production API, and additionally cover the reappearing-object case (A -> B -> A gets a fresh higher band) that the legacy per-object functions could not express. - An always-true conditional removed: the title-card branch directly above it guarantees a truthy objectId, so the wrapper never filtered anything; its 52-line body is unchanged, just dedented. - Unreachable defensive guards removed, each against a verified invariant: optional chaining on state fields that are declared with their shape and never reassigned, a fallback on a config field that is always set, and two silent `if (!step) continue` skips in preloadAhead that would have masked scene-map corruption as a no-op - a corrupt state now fails loudly at the natural access point. The boundary guard on user-supplied config (config?.peekHeight ?? 1) stays: external input is a legitimate place to be defensive. Verified: 281 JS tests (six legacy tests replaced by four banding-invariant tests), clean Jekyll build, and the full end-to-end suite against a served build - 52 passed, 1 skipped, including every navigation path through the de-guarded code.
…-agree copies
v1.6.0 code-health pass over the Liquid layer. Liquid has no unit tests,
so the gate was a rendered-output diff with every changed hunk attributed,
plus the end-to-end suites and a live gallery-filter check.
- objects-index.html: the two ~50-line object grids (browse-and-search vs
plain) now render through a single parameterized include,
_includes/object-grid-item.html - object, lang, and show_filters are
passed explicitly because include scope does not inherit the layout's
assigns (same note as story-step.html). The triple sort-assign block is
hoisted above the conditional. Rendered gallery output is unchanged; all
eight data-* filter attributes verified identical, and live filtering
(facet counts, media-type narrowing) checked in the browser.
- story.html: site.data[page.data_file] is now derived once (story_data)
and reused at the five sites that each re-derived it; the {{ content }}
fallback and the page.steps/page.first_object else-branch are removed -
no story anywhere is frontmatter-driven, every story flows through
data_file.
- index.html: a write-only gs_error_details assign removed.
- object.html: a dead patternFill/patternSvg pair removed - the weave
pattern they fed was removed earlier and the variable was never read.
- Cross-link comments on copies that must stay in sync: the @layer order
string (default.html, story.html, _sass/_responsive.scss - the HTML
declaration must win the layer-ordering race, so it cannot be
consolidated), the KaTeX loader pair (_includes/katex.html and
story.html's lazy inline loader - a version bump must hit both), and the
skip_collections alias (header.html, index.html, objects-index.html -
one line each, not worth an include).
- upgrade.py: comments at the three hand-synced migration registration
sites (import block, LATEST_VERSION, MIGRATIONS list) stating that all
three must agree and that there is no auto-discovery. Values untouched.
Verified: 281 JS tests, clean Jekyll build, story and gallery pages
rendering correctly in both English and Spanish builds, and on the
development instance the full end-to-end suite (52 passed, 1 skipped,
story locking 17/17) plus a live filter probe.
…ng; remove dead vignette
…ve dead clip pipeline
…localize embed banner fallback
…tart only after looping
…; add fragment layout
anuzk13
added a commit
to anuzk13/telar
that referenced
this pull request
Jul 10, 2026
Looking at the code in this branch: UCSB-AMPLab#40
The storyData?.audioObjects branches could never be taken: story.html injects the manifest only as window.audioObjects, and window.storyData carries only steps and firstObject (or the encrypted stub). Re-derived from Ana María Cárdenas Gasca's fix on her 3D_refactor branch (her commit 1b9c8f5).
…eader Flagged in Ana María Cárdenas Gasca's code review: the header said "rendered inside the default shell", a metaphor defined only in default.html's own header, which a reader of this file has no reason to have seen.
…nd notice
The pure-Python iiif fallback (used when libvips is absent) wrote v3
info.json but width-only {w}, tile directories — every deep-zoom tile
404s on a static server. Tile dirs are now canonicalized from on-disk
truth after generation; full/max/0/default.jpg is generated for the
fallback like every other backend; the backend choice is printed with
an install hint instead of falling back silently. Contract test derives
OpenSeadragon's expected URLs from the emitted info.json and asserts
they exist. Reported by Ana María Cárdenas Gasca.
Atomic framework-file replacement (93 files from the release diff), two dead-file deletions, additive-only .gitattributes with a conditional manual step, wholesale language-pack replacement, and bilingual manual steps: the required build.yml update for private stories and the language-pack re-apply note. Tested end-to-end with upgrade.py against scratch clones in both languages.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release branch for v1.6.0 — Code Health. A dedicated housekeeping release: remove verified dead code, consolidate duplicated logic, make the repository honest to a newcomer (what is generated, what is source, what loads what), and improve in-code documentation. Nothing is deleted on a tool's word alone — every removal is verified against cross-layer references (templates, data attributes, generated markup) and gated on tests, build output comparison, and browser checks before it lands here.
Changes accumulate on this branch and merge only when the release is complete and migration-tested.
Changes so far:
Signposting: GENERATED banner on the bundle,
assets/js/README.mddirectory map, linguist-generated attributesFixes: alt_text reaching object pages, LaTeX detection in step text, embed nav position, intro-state navigation, offcanvas panel-state sync, uppercase audio-extension detection
Dead code removed:
_includes/viewer.html, pipeline script flags/fallbacks, ~300 lines of orphaned SCSS, satellite dead code incl. the unusedtelar-icons.js, ~440 lines of superseded story-engine code (unwired deep-link tokens, replaced card-construction and positioning paths)Consolidations: pipeline helpers, SCSS mixins, one home for the media-type lists, object lookups on the shared index, one parameterized include for the two object grids, story data derived once per story page
Housekeeping: version footers on early migrations, bundle rebuilt on the lockfile's Lenis 1.3.23, docstrings that state constraints rather than history
Localization: hardcoded English UI strings routed through the language packs (media-type labels, audio player controls, story chrome, glossary headings, panel error copy, carousel controls, audio pipeline warnings), language strings exposed site-wide to JS, and the upgrade summary generated in the site's language
Fixes: broken accordion focus ring, headings/blockquotes now follow the active theme, Offcanvas instance leak on panel reopen, audio manifest written by a single owner, palette-transparency images composite correctly, text-card layout matches the documented "no coordinates = full object" rule, embed nav positioning aligned with the site-wide responsive boundary
Localization: degraded-mode navigation fallback, upgrade-success alert, embed banner site-name fallback; title-card content now escapes consistently with the documented plain-text contract
Dead code removed: unused clip-extraction pipeline, unused manifest-metadata extractor, no-op glossary shim, never-visible intro vignette scaffolding
Housekeeping: design-token layer completed (accent + UI constants, JS inline styles read tokens), shared share-panel blocks extracted to parameterized includes, comment sweep replacing history narration with present-tense constraints
Consolidations: inline template algorithms extracted to shared modules — IIIF thumbnail resolution (three copies →
iiif-thumbnails.js), object-page theme contrast and video-embed parsing (object-theme.js,video-embed.js), the IIIF URL-mismatch diagnostic (iiif-url-warning.js), and carousel image-src resolution moved from the template into the widget parserNaming: story-engine identifiers renamed to match their actual lifecycles (card registry vs. viewer pool, interpolation vs. scrubbing)
Protected stories rebuilt: encryption moved from generation time to a post-build step that encrypts the fully rendered steps (restoring glossary links, LaTeX, and localized errors in locked stories), with leak gates over the rendered output and a fail-closed workflow interlock; templates re-keyed to frontmatter; one shared steps loop for open and protected rendering
Fixes: audio clips now start at their configured start time on first play; panel close button icon restored in vertical/embed mode; upgrade summary output localized
Debt sweep: thumbnail resolution hardened (sorted size selection, live direct-image fallback, robust level-0 detection across IIIF profile forms), video/audio objects no longer trigger doomed IIIF fetches on the objects index, KaTeX configuration single-sourced in
_data/katex.yml, protected-story content gate covers dense scripts with a fair length bar, vestigial ffmpeg requirement removed, widget template comments no longer ship in rendered pages, stale generated data files self-clean, WaveSurfer loader extracted from the object layout, dead config fallbacks removedLocal development: the no-libvips tile fallback now produces working deep-zoom tiles (canonical IIIF v3 paths, full-image request, and a visible backend notice); locally generated tile trees self-heal on the next build