Skip to content

Chore/resolve todos - #1732

Open
Arnei wants to merge 9 commits into
opencast:developfrom
Arnei:chore/resolve-todos
Open

Arnei wants to merge 9 commits into
opencast:developfrom
Arnei:chore/resolve-todos

Conversation

@Arnei

@Arnei Arnei commented Sep 8, 2026

Copy link
Copy Markdown
Member

Addresses various "TODO" comments in the code that are low impact, meaning they required no or only small changes that don't change a whole lot.

How to test this

Should be no functional changes. Could take a look at the track selection or thumbnail generation pages to see if they still hold up.

AI Usage

Claude Sonnet 5 was used for finding these and giving resolve suggestions.

@Arnei Arnei added the type:code-quality Internal improvements to the codebase label Sep 8, 2026
@github-actions github-actions Bot added the status:conflicts Conflicts with another pull request or issue label Sep 14, 2026
@github-actions

Copy link
Copy Markdown

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

workflowPostAndProcessSlice, which this referenced, was already
deleted in an earlier commit that merged the duplicate save logic
into this file.

This commit was largely AI generated.
width: 100vw ignored the parent container's padding, overflowing it
at narrow viewport widths. width: 100% resolves against the actual
container and behaves identically otherwise.

This commit was largely AI generated.
The Opencast backend's EditingData no longer has this field, so
fetching it resolved to undefined and posting it back was a no-op.
Removed it from the type contract and videoSlice; TrackSelection.tsx,
where it drove real in-session UI behavior, now uses local useState
instead.

This commit was largely AI generated.
canBeAddedSubtitles already tracks which languages are still
available; the button now only renders when that list is non-empty.

This commit was largely AI generated.
Returning false from the catch block already is the correct handling
for this boolean type-guard; there was nothing left to do.

This commit was largely AI generated.
The scrubber's extra +20px height was undocumented and disconnected
from its actual source, timelineStampsHeight, which lives in the
parent component. height: "100%" (as the TODO suggested) would have
been wrong: the scrubber deliberately spans the timestamp ruler above
the waveform/segments area too, not just that area's own height.

This commit was largely AI generated.
width is not just an incidental pass-through to SegmentsList - it
flows into CutMark, which genuinely needs it for positioning, drag
bounds, and drag-to-time conversion. It can't be dropped without
breaking cut-mark dragging, so the question of what happens if it
stops being passed was never actually testable. The ResizeObserver
library that originally prompted the question was also since replaced
(usehooks-ts, for React 19 compatibility).

This commit was largely AI generated.
package.json/package-lock.json confirm react-player now resolves from
the npm registry, not the fork. Commit 6d385ca (updating to
react-player v3) already dropped it, noting the rewrite resolved the
hotloading bug the fork worked around.

This commit was largely AI generated.
This commit was largely AI generated.
@Arnei
Arnei force-pushed the chore/resolve-todos branch from 0bbc0fa to 645e759 Compare September 15, 2026 06:50
@github-actions github-actions Bot removed the status:conflicts Conflicts with another pull request or issue label Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:code-quality Internal improvements to the codebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant