feat: render banners at 100% width when dimensions omitted#189
Open
anonvt wants to merge 1 commit into
Open
Conversation
When `<topsort-banner>` is rendered without `width`/`height` attributes, the component defaulted to `width=0` / `height=0`, producing an `<img>` with `style="width:0px; height:0px"` — invisible to users but still flagged with `data-ts-clickable` / `data-ts-resolved-bid`, so impressions were still billed against a zero-pixel banner. Treat 0 as a "not set" sentinel and substitute `width:100%` / `height:auto` on both the rendered `<img>`/`<hls-video>` and the `--ts-banner-width` / `--ts-banner-height` host CSS variables. The banner now fills its container at the asset's native aspect ratio when dimensions are omitted. The auction request itself is unchanged — width/height were never part of the `/v2/auctions` payload.
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.
Summary
<topsort-banner>is rendered withoutwidth/heightattributes, the component defaulted towidth=0/height=0, producing an<img>withstyle=\"width:0px; height:0px\"— invisible to users but still tagged withdata-ts-clickable/data-ts-resolved-bid, so analytics.js would record impressions on a zero-pixel banner.0as a "not set" sentinel and substitutewidth:100%/height:autoon the rendered<img>/<hls-video>and on the--ts-banner-width/--ts-banner-heighthost CSS variables. The banner now fills its container at the asset's native aspect ratio when dimensions are omitted./v2/auctionsrequest is unchanged — width/height were never part of the payload.Reproducer
Before: auction succeeds, but the rendered
<img>iswidth:0px; height:0px(invisible, still billed).After: rendered
<img>iswidth:100%; height:autoand visually fills its container.Behavior change
This is a minor behavior change for integrations that omit
width/height— they will now render a visible banner instead of an invisible one. Customers who set explicit dimensions are unaffected. Per-axis fallback works too: e.g.width=\"300\"with noheightrenderswidth:300px; height:auto.Test plan
pnpm lint— passes (only the pre-existingbiome.jsonwarning unrelated to this change)pnpm test:run— 106/106 passing, including 4 new regression tests<topsort-banner>withoutwidth/heightrenders at the container's widthNew regression tests
In
src/__tests__/topsort-banner.test.ts:<img>style iswidth:100%; height:autowhen attributes omitted--ts-banner-width:100%/--ts-banner-height:autowidthset +heightomitted ⇒height:autoonlyEach of these fails on
main(asserts100%/autowhere current code produces0px).🤖 Generated with Claude Code