fix(skills): define .video-wrapper.pip-pill in talking-head-recut template - #3117
Open
imprimisxo wants to merge 1 commit into
Open
fix(skills): define .video-wrapper.pip-pill in talking-head-recut template#3117imprimisxo wants to merge 1 commit into
imprimisxo wants to merge 1 commit into
Conversation
…plate pip-pill is referenced three times (GSAP lookup table rows for pip bottom-right and top-left, and the className toggle snippet) and defined zero times, so every pip following the table renders with no chrome. Not a misspelling of .framed: that rule is radius + shadow, while pip-pill is documented as radius + white ring + shadow. Renaming would silently drop the ring, so the missing rule is defined instead. The ring is a box-shadow spread rather than a border so it adds no layout box and the pixel-exact pip targets stay correct.
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.
What
Defines
.video-wrapper.pip-pillin thetalking-head-recutcomposition template. The class is referenced three times and defined zero times, so every pip that follows the GSAP lookup table currently renders with no chrome at all.Three larger findings from the same read are written up under "Further findings" — no changes made for those, they need a maintainer call first.
Why
pip-pillis used in three places:pip(bottom-right) →pip-pill(border-radius + ring + shadow)pip(top-left) →pip-pilltl.set("#video-wrap", { className: "video-wrapper pip-pill" }, T)and described at line 1120 as "border-radius + white ring + drop shadow". There is no CSS rule for it anywhere in the file. Because line 1125 sets
classNamewholesale, applying it removes whatever chrome was there and replaces it with a class that does nothing — a pip ends up with square corners and no shadow.This is not a misspelling of
.framed..video-wrapper.framed(line 912) isborder-radius: 16px+ drop shadow;pip-pillis documented as that plus a white ring. So it is a distinct intended style that was never implemented, and renaming the references to.framedwould silently drop the ring. The fix defines what the docs already promise.The ring is a
box-shadowspread rather than aborderon purpose: the pip targets in both tables are pixel-exact ({ left: 1480, top: 760, width: 400, height: 300 }), and a border would add to the layout box and shift them.How
Eleven lines added to the
<style>block in the composition template, directly after the.framedrule it builds on. Values mirror.framedso the two stay visually consistent. Nothing else touched.Further findings — no changes made, these need a decision
1. The
#video-wrapGSAP target table exists twice, and the copies disagreeLines 577–582 ("4 composition layouts") and lines 1111–1118 ("GSAP target lookup table") both map composition layout →
#video-wraptarget.They are not the same table. The second adds a
piptop-left row, a "hide video" row, and an "extra css class" column; the first adds portrait values and a "when to use" column. So each is incomplete on its own, and an agent reading only one gets a partial picture.They also contradict each other on pip chrome: line 581 says
pipgets.framed, line 1115 sayspipgetspip-pill. This PR makes both classes valid, so either instruction now produces working output — but the disagreement about which chrome a pip should have is still there, and only you can say which is intended.Suggested: one merged table (all three ratios, both extra columns), defined once near the layout section and referenced from Step 9.
2. The
zonebounds table also exists twiceLines 182–188 (Step 6) and 591–597 (Step 7), near-identical, with wording that has already drifted —
whiteboard-areareads "inset 40px margin (or 45% of portrait height)" in the first and "inset 40px margin (landscape) or bottom 45% (portrait)" in the second. The second phrasing is the clearer one.Suggested: keep the Step 6 copy (where
zoneis introduced), have Step 7 point at it.3. The composition template could ship as an asset instead of prose
Lines 799–1065 — 267 lines, ~19KB — are a complete, self-contained
<!doctype html>…</html>document: font-face block,:roottheme vars, stage/video-wrapper/card-host CSS, the#stageelement with itsdata-*timing attributes, an example card-host pair, and the full GSAP scaffold. Today the agent retypes all of it from markdown.The skill already ships and stages an
assets/tree, so the mechanism exists:Step 9 would then reduce to filling in
data-duration/data-fps/data-width/data-height, swapping the:rootpalette for the chosenthemeId, emitting onecard-host clipdiv per card, and appending the compiled GSAP statements.The argument for it: a copied file cannot drift from the documented contract, and retyping is exactly where the lint violations the doc warns about (
timed_element_missing_clip_class,font_family_without_font_face) get introduced — both are hard-coded correctly in the template. It would also take SKILL.md from ~65KB to ~46KB on its own.If that lands and you want to go further, these blocks are each needed at exactly one step and nowhere else, and the skill already has a healthy
references/tree to follow:data-animkindsHappy to send any of these as follow-up PRs — I held off because (1) and (3) in particular are your call on intent, not mechanical fixes.
Test plan
pip-pillhas 3 usages and 0 definitions, that.video-wrapper.framedis defined at line 912 with different properties, and that the diff is confined to the 11 added linesFound while auditing skill files for context-budget cost.