Conversation
cjpillsbury
reviewed
Sep 15, 2025
| ], | ||
| "scripts": { | ||
| "build": "rollup -c", | ||
| "dev": "rollup -c -w", |
cjpillsbury
reviewed
Sep 15, 2025
| /** external props spread last to allow for overriding */ | ||
| style: { | ||
| ...props.style, | ||
| '--slider-fill': `${Math.round(sliderFill)}%`, |
Collaborator
There was a problem hiding this comment.
thought: If we go the route of custom CSS properties for these things, a nitpick improvement here - put the spread second to make it easy to override default behaviors.
cjpillsbury
reviewed
Sep 15, 2025
| type TimeRangeRootState = ReturnType<useTimeRangeRootState>; | ||
| type TimeRangeRootProps = ReturnType<useTimeRangeRootProps>; | ||
|
|
||
| export const renderTimeRangeRoot = (props: TimeRangeRootProps, state: TimeRangeRootState) => { |
Collaborator
There was a problem hiding this comment.
per prior out of band convo, we'll likely want to do some architecture expansion so that less of this lives in the render function (would require updates to toConnectedComponent or similar). Ideally (imo), we want to land on something more like the simplicity of the non-compound components, roughly like:
export const renderTimeRangeRoot = (props: TimeRangeRootProps, state: TimeRangeRootState) => {
return <div {...props}>{props.children}</div>
};
cjpillsbury
reviewed
Sep 15, 2025
| return { | ||
| ...props, | ||
| style: { | ||
| ...props.style, |
Collaborator
There was a problem hiding this comment.
same spread order callout/comment as above, re: overridable styles if this is where the dust settles
cjpillsbury
reviewed
Sep 15, 2025
| // EXPORTS | ||
| // ============================================================================ | ||
|
|
||
| export const TimeRange = Object.assign( |
Collaborator
There was a problem hiding this comment.
nitpick: Is Object.assign needed at all? Can we just:
export const TimeRange = {
Root: TimeRangeRoot,
Track: TimeRangeTrack,
Thumb: TimeRangeThumb,
Pointer: TimeRangePointer,
Progress: TimeRangeProgress,
} as const;?
sampotts
reviewed
Sep 15, 2025
Comment on lines
+7
to
+11
| const formatTime = (time: number): string => { | ||
| const minutes = Math.floor(time / 60); | ||
| const seconds = Math.floor(time % 60); | ||
| return `${minutes}:${seconds.toString().padStart(2, '0')}`; | ||
| }; |
Collaborator
There was a problem hiding this comment.
a definite nit: but I'd imagine this could be shared with other time display components and even non-React stuff eventually. Perhaps the same for below.
Merged
Merged
cjpillsbury
added a commit
that referenced
this pull request
May 21, 2026
Adds the first cluster B Borderline content-compensation feature doc. Heuristically detect "pseudo-ended" state — playback stalls near duration boundary on sources with mismatched manifest-duration vs actual segment data (canonically Safari) — and trigger termination cleanly so ended fires correctly. Two phases (detection + action), Naive vs Full depth per row matching Notion epic #10's framing: - Detection: Naive = don't (status quo). Full = heuristic monitor of playhead approaching duration + non-progressing + buffer doesn't reach duration. - Action: Naive = passive (browser stalls). Full = trigger clean termination via MediaSource.endOfStream() OR mediaSource.duration adjustment (mechanism choice open). Composition: VOD-engine variant only. Live engine variants don't carry this behavior (live has Infinity duration, no boundary to approach). Per the failure-mode catalog's composition-variant entry — variant-specific behavior, not runtime branch. Cross-cutting impact captures: action-mechanism choice trade-offs (endOfStream caller coordination with mse-mms-pipeline's existing gate vs duration multi-writer), distinction from buffer-stall-recovery (both detect "not progressing"; discriminator is "near duration boundary"), composition-variant placement, threshold tuning, false-positive avoidance, browser-specificity. Open questions: action-mechanism choice, detection thresholds, coordination with buffer-stall-recovery, live→VOD transition shape, browser detection gate, composition with edit-list-compensation, duration-undefined defensive case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cjpillsbury
added a commit
that referenced
this pull request
May 21, 2026
Adds the first cluster B Borderline content-compensation feature doc. Heuristically detect "pseudo-ended" state — playback stalls near duration boundary on sources with mismatched manifest-duration vs actual segment data (canonically Safari) — and trigger termination cleanly so ended fires correctly. Two phases (detection + action), Naive vs Full depth per row matching Notion epic #10's framing: - Detection: Naive = don't (status quo). Full = heuristic monitor of playhead approaching duration + non-progressing + buffer doesn't reach duration. - Action: Naive = passive (browser stalls). Full = trigger clean termination via MediaSource.endOfStream() OR mediaSource.duration adjustment (mechanism choice open). Composition: VOD-engine variant only. Live engine variants don't carry this behavior (live has Infinity duration, no boundary to approach). Per the failure-mode catalog's composition-variant entry — variant-specific behavior, not runtime branch. Cross-cutting impact captures: action-mechanism choice trade-offs (endOfStream caller coordination with mse-mms-pipeline's existing gate vs duration multi-writer), distinction from buffer-stall-recovery (both detect "not progressing"; discriminator is "near duration boundary"), composition-variant placement, threshold tuning, false-positive avoidance, browser-specificity. Open questions: action-mechanism choice, detection thresholds, coordination with buffer-stall-recovery, live→VOD transition shape, browser detection gate, composition with edit-list-compensation, duration-undefined defensive case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
3 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.
fix #13