WEBDEV-8968: Migrate transcript-view into elements - #84
Open
jbuckner wants to merge 2 commits into
Open
Conversation
Move @internetarchive/transcript-view out of the iaux monorepo as ia-transcript-view, with its child as ia-transcript-entry. Sixth of seven. - duration-formatter was a whole custom element that existed to turn a number into mm:ss. It's a formatDuration() function now, so every timestamp no longer costs an element registration and a shadow root. It also returns an empty string for NaN and Infinity rather than rendering NaN:NaN, and clamps negatives. - Only one scroll animation runs at a time. Each call started its own requestAnimationFrame loop with no way to stop it, and playback ticks arrive faster than a one-second animation finishes, so several loops ended up fighting over scrollTop. Progress is clamped to 1 so the last frame lands on the target instead of past it, and the animation is cancelled on disconnect along with the auto-scroll resume timer, which leaked. - An entry whose id is 0 can be found. elementClosestToTime guarded with a falsy check on the id, so a transcript starting at 0 never scrolled to its first entry. - Music entries have their own hover colour variable. The hover rule read the *active* variable, so the two could not be themed apart. - data-search-result-index is omitted rather than rendered as the string "undefined" on entries that aren't search results. TranscriptEntryConfig and TranscriptConfig stay classes, since displayText and searchResults are real behaviour rather than data. The music placeholder goes through msg() so it can be translated. scrollTimerDelay is a public property, which is how the tests set it instead of reaching for a private field. Claude-Session: https://claude.ai/code/session_01U6iNyGicKJywdrWJMTyFin Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
This was referenced Aug 27, 2026
…8968-migrate-transcript-view * WEBDEV-8967-migrate-expandable-search-bar: WEBDEV-8972: Run CI on every PR, not just ones based on main (#90)
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## WEBDEV-8967-migrate-expandable-search-bar #84 +/- ##
=============================================================================
+ Coverage 80.80% 81.58% +0.78%
=============================================================================
Files 41 46 +5
Lines 1224 1423 +199
Branches 276 319 +43
=============================================================================
+ Hits 989 1161 +172
- Misses 154 171 +17
- Partials 81 91 +10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
2 participants
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.
WEBDEV-8968. Stacked on #83, part of WEBDEV-8962.
Brings
transcript-viewover asia-transcript-view, and its child asia-transcript-entry.API change:
duration-formatterwas a whole custom element whose only job was turning a number intomm:ss. It's an exportedformatDuration()function now, so timestamps don't each cost an element registration and a shadow root. Same output for every input the old tests covered.Fixed on the way through:
requestAnimationFrameloop with no way to cancel it, and playback ticks arrive faster than the one-second animation finishes, so several loops ended up fighting overscrollTop. Progress is clamped to 1 so the final frame lands on target rather than past it, and the animation is cancelled on disconnect — along with the 15-second auto-scroll resume timer, which leaked.idis0can be found.elementClosestToTimeguarded with a falsy check on the id, so a transcript starting at 0 never scrolled to its first entry.data-search-result-indexis omitted rather than rendered as the literal string"undefined"on non-result entries.formatDurationreturns an empty string forNaN/Infinityinstead of renderingNaN:NaN.TranscriptEntryConfigandTranscriptConfigstay classes —displayTextandsearchResultsare real behaviour, not data, which is the line I've drawn against the interfaces in the earlier PRs. The music placeholder goes throughmsg().Worth knowing for whoever wires this up: assigning
configresetsselectedSearchResultIndexand stops the view following playback, and Lit re-commits object bindings on every render — so hold a stable reference rather than building the config insiderender(). There's a test pinning that, and a note in the story.CI shows only
deploy-previewhere because the PR isn't based onmain(WEBDEV-8972). Verified locally: build, full suite 340 passing, eslint and prettier clean, 95% coverage on this component.