fix(shotkit): follow-up review polish (caption options, lint, thumbnail-only)#8
Merged
Merged
Conversation
…il-only)
Second-tier findings deferred from the v1.3.0 correctness pass:
- step() honors flat caption display options (e.g. { position }) like caption(),
instead of silently dropping anything outside options.captionOptions
- lintDemoStoryboard() no longer warns 'missing mp4' when demo.mp4/crop/zoom is
set (public callers need not pass mp4Requested)
- thumbnail-only demo no longer re-muxes/overwrites the source .webm
(Deliberately not changed: scheduled-caption option handling — scheduled
captions use controller-level captionOptions by design; no wrong output.)
lint clean, 13 suites / 83 tests pass.
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
1 participant
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.
Second-tier findings deferred from the v1.3.0 correctness pass (#7). None break the handoff contract; these are ergonomics/waste fixes.
step(text, fn, options)now honors flat caption display options (e.g.{ position }, by analogy withcaption()) instead of silently dropping anything outsideoptions.captionOptions.lintDemoStoryboard()no longer emits a spurious "missing mp4" warning when the demo config setsmp4/crop/zoom— the public caller need not passmp4Requested..webm; the thumbnail is taken from the original clip.Deliberately NOT changed — [6] (scheduled-caption option handling): on close reading it is working-as-designed. Scheduled captions (from
captions: [{at,text}], which carries no per-caption options) merge with controller-levelcaptionOptions, so they render at the intended position; per-callcaption(text, opts)options are per-call by design. No wrong on-screen output, so no behavioral change.Verification
npm run lintcleannpm test→ 13 suites / 83 tests (added lint-mp4 + reused validation/readiness coverage)Note
Lands under
## [Unreleased]in CHANGELOG;package.jsonstays1.3.0(still unpublished on npm —latestis1.1.1). Owner decides whether to fold into the first1.3.0publish or bump.