Skip to content

Commit a7536fd

Browse files
committed
fix(captions): pick the closest position preset, not the first one checked
activeHorizontalPositionPreset checked "is this centered?" before "is this flush left/right?" — fine normally, but as width approaches 100 the whole reachable range shrinks toward 0 right along with it, so a band sitting exactly at the true left/right edge could fall inside the center check's epsilon too and get reported as centered. Comparing all three candidates and keeping the nearest one is correct regardless of how narrow the range gets. Found via code review (CodeRabbit hit its OSS rate limit on the PR, so this ran as a subagent review instead). Reachable today only through a hand-edited or externally-generated project file — legacyEditor.captions has no schema validation — not through the shipped integer-stepped width slider.
1 parent 619057a commit a7536fd

2 files changed

Lines changed: 34 additions & 6 deletions

File tree

‎src/lib/ai-edition/captions/captions.test.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
activeHorizontalPositionPreset,
77
activeVerticalPositionPreset,
88
CAPTION_BAND_HEIGHT_PCT,
9+
CAPTION_POSITION_PRESET_EPSILON,
910
captionBackgroundCss,
1011
captionBandRect,
1112
captionHorizontalPositionOffset,
@@ -277,6 +278,18 @@ describe("caption position presets", () => {
277278
expect(activeHorizontalPositionPreset({ ...ON, width: 100, offsetX: 0 })).toBe("center");
278279
});
279280

281+
it("still picks left/right over center when a near-full-width band squeezes them inside the epsilon", () => {
282+
// At width this close to 100, range.x.min/max themselves fall inside
283+
// CAPTION_POSITION_PRESET_EPSILON of 0 — checking "is this near center?"
284+
// first would wrongly claim an offset that is exactly at the true edge.
285+
const squeezed = { ...ON, width: 100 - 1e-7 };
286+
const range = captionOffsetRange(squeezed);
287+
expect(Math.abs(range.x.min)).toBeLessThan(CAPTION_POSITION_PRESET_EPSILON);
288+
expect(activeHorizontalPositionPreset({ ...squeezed, offsetX: range.x.min })).toBe("left");
289+
expect(activeHorizontalPositionPreset({ ...squeezed, offsetX: range.x.max })).toBe("right");
290+
expect(activeHorizontalPositionPreset({ ...squeezed, offsetX: 0 })).toBe("center");
291+
});
292+
280293
it("sets offsetX to the true frame edge for left/right, matching the reachable range", () => {
281294
const range = captionOffsetRange(ON);
282295
expect(captionHorizontalPositionOffset(ON, "left")).toBeCloseTo(range.x.min, 6);

‎src/lib/ai-edition/captions/settings.ts‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -211,18 +211,33 @@ export function activeVerticalPositionPreset(
211211
* There is no stored `horizontalPosition` field — `offsetX` is already an
212212
* absolute-feeling value centred on 0 with a range that reaches both frame
213213
* edges (see `captionOffsetRange`), so "left"/"center"/"right" are just names
214-
* for three points on that existing range, checked in this order because all
215-
* three coincide at `offsetX === 0` when the band is full-width (no travel).
214+
* for three points on that existing range. All three coincide at `offsetX===0`
215+
* when the band is full-width (no travel) — and can also *nearly* coincide
216+
* for a band merely close to full-width, where `range.x.min`/`max` shrink
217+
* toward 0 as well. Picking the CLOSEST candidate (not the first one within
218+
* epsilon) is what keeps that near-degenerate case from reporting "center"
219+
* for an offset that is actually sitting exactly on `range.x.min`/`max`.
216220
*/
217221
export function activeHorizontalPositionPreset(
218222
settings: CaptionSettings,
219223
): CaptionHorizontalPosition | null {
220224
const range = captionOffsetRange(settings);
221225
const { offsetX } = settings;
222-
if (Math.abs(offsetX) < CAPTION_POSITION_PRESET_EPSILON) return "center";
223-
if (Math.abs(offsetX - range.x.min) < CAPTION_POSITION_PRESET_EPSILON) return "left";
224-
if (Math.abs(offsetX - range.x.max) < CAPTION_POSITION_PRESET_EPSILON) return "right";
225-
return null;
226+
const candidates: ReadonlyArray<[CaptionHorizontalPosition, number]> = [
227+
["center", 0],
228+
["left", range.x.min],
229+
["right", range.x.max],
230+
];
231+
let closest: CaptionHorizontalPosition | null = null;
232+
let closestDistance = CAPTION_POSITION_PRESET_EPSILON;
233+
for (const [preset, target] of candidates) {
234+
const distance = Math.abs(offsetX - target);
235+
if (distance < closestDistance) {
236+
closest = preset;
237+
closestDistance = distance;
238+
}
239+
}
240+
return closest;
226241
}
227242

228243
/** The `offsetX` that puts the band at a given horizontal preset, for a preset

0 commit comments

Comments
 (0)