Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,18 @@ export function CapVideoPlayer({
}
};

// Ensure all caption tracks remain hidden
const ensureTracksHidden = (): void => {
const tracks = Array.from(video.textTracks);
for (const track of tracks) {
if (track.kind === "captions" || track.kind === "subtitles") {
if (track.mode !== "hidden") {
track.mode = "hidden";
}
}
}
};

Comment on lines +300 to +311
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove inline comment to comply with repo guidelines

Inline comments are disallowed for TS/JS. The code is clear without it.

As per coding guidelines

-		// Ensure all caption tracks remain hidden
 		const ensureTracksHidden = (): void => {
 			const tracks = Array.from(video.textTracks);
 			for (const track of tracks) {
 				if (track.kind === "captions" || track.kind === "subtitles") {
 					if (track.mode !== "hidden") {
 						track.mode = "hidden";
 					}
 				}
 			}
 		};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Ensure all caption tracks remain hidden
const ensureTracksHidden = (): void => {
const tracks = Array.from(video.textTracks);
for (const track of tracks) {
if (track.kind === "captions" || track.kind === "subtitles") {
if (track.mode !== "hidden") {
track.mode = "hidden";
}
}
}
};
const ensureTracksHidden = (): void => {
const tracks = Array.from(video.textTracks);
for (const track of tracks) {
if (track.kind === "captions" || track.kind === "subtitles") {
if (track.mode !== "hidden") {
track.mode = "hidden";
}
}
}
};
🤖 Prompt for AI Agents
In apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx around lines 300 to
311, remove the inline comment "// Ensure all caption tracks remain hidden" to
comply with the repo guideline disallowing inline comments in TS/JS; leave the
ensureTracksHidden function and its implementation unchanged and run a quick
lint/format to confirm no other inline comments remain in this block.

const handleLoadedMetadataWithTracks = () => {
setVideoLoaded(true);
if (!hasPlayedOnce) {
Expand All @@ -305,6 +317,11 @@ export function CapVideoPlayer({
setupTracks();
};

// Monitor for track changes and ensure they stay hidden
const handleTrackChange = () => {
ensureTracksHidden();
};

Comment on lines +320 to +324
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove inline comment to comply with repo guidelines

Keep code self-explanatory; comments are disallowed.

As per coding guidelines

-		// Monitor for track changes and ensure they stay hidden
 		const handleTrackChange = () => {
 			ensureTracksHidden();
 		};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Monitor for track changes and ensure they stay hidden
const handleTrackChange = () => {
ensureTracksHidden();
};
const handleTrackChange = () => {
ensureTracksHidden();
};
🤖 Prompt for AI Agents
In apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx around lines 320 to
324, remove the inline comment "// Monitor for track changes and ensure they
stay hidden" so the code complies with the repo guideline of no comments; leave
the handleTrackChange function as-is (const handleTrackChange = () => {
ensureTracksHidden(); };) and ensure the function name is sufficiently
descriptive so no comment is needed.

video.addEventListener("loadeddata", handleLoadedData);
video.addEventListener("canplay", handleCanPlay);
video.addEventListener("loadedmetadata", handleLoadedMetadataWithTracks);
Expand All @@ -313,6 +330,11 @@ export function CapVideoPlayer({
video.addEventListener("error", handleError as EventListener);
video.addEventListener("loadedmetadata", handleLoadedMetadataWithTracks);

// Add event listeners to monitor track changes
video.textTracks.addEventListener("change", handleTrackChange);
video.textTracks.addEventListener("addtrack", handleTrackChange);
video.textTracks.addEventListener("removetrack", handleTrackChange);

Comment on lines +333 to +337
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove inline comment and optionally guard for addEventListener availability

Comments are disallowed. If you need cross-browser resilience, wrap in a feature check.

-		// Add event listeners to monitor track changes
-		video.textTracks.addEventListener("change", handleTrackChange);
-		video.textTracks.addEventListener("addtrack", handleTrackChange);
-		video.textTracks.addEventListener("removetrack", handleTrackChange);
+		{
+			const tt = video.textTracks as unknown as { addEventListener?: EventTarget["addEventListener"] };
+			if (typeof tt.addEventListener === "function") {
+				video.textTracks.addEventListener("change", handleTrackChange);
+				video.textTracks.addEventListener("addtrack", handleTrackChange);
+				video.textTracks.addEventListener("removetrack", handleTrackChange);
+			}
+		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Add event listeners to monitor track changes
video.textTracks.addEventListener("change", handleTrackChange);
video.textTracks.addEventListener("addtrack", handleTrackChange);
video.textTracks.addEventListener("removetrack", handleTrackChange);
{
const tt = video.textTracks as unknown as { addEventListener?: EventTarget["addEventListener"] };
if (typeof tt.addEventListener === "function") {
video.textTracks.addEventListener("change", handleTrackChange);
video.textTracks.addEventListener("addtrack", handleTrackChange);
video.textTracks.addEventListener("removetrack", handleTrackChange);
}
}
🤖 Prompt for AI Agents
In apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx around lines 333 to
337, remove the inline comment and replace the direct calls to
video.textTracks.addEventListener with a feature-guarded approach: check that
video.textTracks and video.textTracks.addEventListener exist before calling, and
only attach "change", "addtrack", and "removetrack" listeners when available; if
addEventListener is missing, optionally fall back to assigning handlers on
supported events or skip attaching to maintain cross-browser resilience.

if (video.readyState === 4) {
handleLoadedData();
}
Expand Down Expand Up @@ -341,6 +363,9 @@ export function CapVideoPlayer({
"loadedmetadata",
handleLoadedMetadataWithTracks,
);
video.textTracks.removeEventListener("change", handleTrackChange);
video.textTracks.removeEventListener("addtrack", handleTrackChange);
video.textTracks.removeEventListener("removetrack", handleTrackChange);
if (retryTimeout.current) clearTimeout(retryTimeout.current);
};
}
Expand All @@ -355,6 +380,9 @@ export function CapVideoPlayer({
"loadedmetadata",
handleLoadedMetadataWithTracks,
);
video.textTracks.removeEventListener("change", handleTrackChange);
video.textTracks.removeEventListener("addtrack", handleTrackChange);
video.textTracks.removeEventListener("removetrack", handleTrackChange);
if (retryTimeout.current) {
clearTimeout(retryTimeout.current);
}
Expand Down Expand Up @@ -462,7 +490,6 @@ export function CapVideoPlayer({
kind="captions"
srcLang="en"
src={captionsSrc}
default
/>
</MediaPlayerVideo>
)}
Expand Down
37 changes: 30 additions & 7 deletions apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -233,18 +233,47 @@ export function HLSVideoPlayer({
}
};

// Ensure all caption tracks remain hidden
const ensureTracksHidden = (): void => {
const tracks = video.textTracks;
for (let i = 0; i < tracks.length; i++) {
const track = tracks[i];
if (
track &&
(track.kind === "captions" || track.kind === "subtitles")
) {
if (track.mode !== "hidden") {
track.mode = "hidden";
}
}
}
};

Comment on lines +236 to +251
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove inline comment to comply with repo guidelines

Keep code self-descriptive; comments are disallowed.

As per coding guidelines

-		// Ensure all caption tracks remain hidden
 		const ensureTracksHidden = (): void => {
 			const tracks = video.textTracks;
 			for (let i = 0; i < tracks.length; i++) {
 				const track = tracks[i];
 				if (
 					track &&
 					(track.kind === "captions" || track.kind === "subtitles")
 				) {
 					if (track.mode !== "hidden") {
 						track.mode = "hidden";
 					}
 				}
 			}
 		};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Ensure all caption tracks remain hidden
const ensureTracksHidden = (): void => {
const tracks = video.textTracks;
for (let i = 0; i < tracks.length; i++) {
const track = tracks[i];
if (
track &&
(track.kind === "captions" || track.kind === "subtitles")
) {
if (track.mode !== "hidden") {
track.mode = "hidden";
}
}
}
};
const ensureTracksHidden = (): void => {
const tracks = video.textTracks;
for (let i = 0; i < tracks.length; i++) {
const track = tracks[i];
if (
track &&
(track.kind === "captions" || track.kind === "subtitles")
) {
if (track.mode !== "hidden") {
track.mode = "hidden";
}
}
}
};
🤖 Prompt for AI Agents
apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsx around lines 236-251,
remove the inline comment "// Ensure all caption tracks remain hidden" to comply
with repo guidelines; keep the clear function name ensureTracksHidden and body
as-is (or if needed, make the function name/variable names more descriptive) so
the code remains self-explanatory without comments.

const handleLoadedMetadata = (): void => {
setupTracks();
};

// Monitor for track changes and ensure they stay hidden
const handleTrackChange = () => {
ensureTracksHidden();
};

Comment on lines +256 to +260
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove inline comment to comply with repo guidelines

Comments are disallowed per repo rules.

As per coding guidelines

-		// Monitor for track changes and ensure they stay hidden
 		const handleTrackChange = () => {
 			ensureTracksHidden();
 		};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Monitor for track changes and ensure they stay hidden
const handleTrackChange = () => {
ensureTracksHidden();
};
const handleTrackChange = () => {
ensureTracksHidden();
};
🤖 Prompt for AI Agents
In apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsx around lines 256 to
260, remove the inline comment "// Monitor for track changes and ensure they
stay hidden" so the repo guideline against comments is satisfied; keep the
function declaration and body (const handleTrackChange = () => {
ensureTracksHidden(); };) unchanged, simply delete the commented line and ensure
formatting/indentation remains correct.

video.addEventListener("loadedmetadata", handleLoadedMetadata);

// Add event listeners to monitor track changes
video.textTracks.addEventListener("change", handleTrackChange);
video.textTracks.addEventListener("addtrack", handleTrackChange);
video.textTracks.addEventListener("removetrack", handleTrackChange);

Comment on lines +263 to +267
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove inline comment to comply with repo guidelines

Also consider a feature check wrapper for resilience.

As per coding guidelines

-		// Add event listeners to monitor track changes
-		video.textTracks.addEventListener("change", handleTrackChange);
-		video.textTracks.addEventListener("addtrack", handleTrackChange);
-		video.textTracks.addEventListener("removetrack", handleTrackChange);
+		{
+			const tt = video.textTracks as unknown as { addEventListener?: EventTarget["addEventListener"] };
+			if (typeof tt.addEventListener === "function") {
+				video.textTracks.addEventListener("change", handleTrackChange);
+				video.textTracks.addEventListener("addtrack", handleTrackChange);
+				video.textTracks.addEventListener("removetrack", handleTrackChange);
+			}
+		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Add event listeners to monitor track changes
video.textTracks.addEventListener("change", handleTrackChange);
video.textTracks.addEventListener("addtrack", handleTrackChange);
video.textTracks.addEventListener("removetrack", handleTrackChange);
{
const tt = video.textTracks as unknown as { addEventListener?: EventTarget["addEventListener"] };
if (typeof tt.addEventListener === "function") {
video.textTracks.addEventListener("change", handleTrackChange);
video.textTracks.addEventListener("addtrack", handleTrackChange);
video.textTracks.addEventListener("removetrack", handleTrackChange);
}
}
🤖 Prompt for AI Agents
In apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsx around lines 263 to
267, remove the inline comment and replace the raw event listener calls with a
guarded implementation: first ensure video?.textTracks exists, then check that
textTracks.addEventListener is a function before calling it (or fall back to
attaching to each track via track.addEventListener), and similarly guard
removals; do not leave inline comments — add a concise explanatory TODO or move
explanation to a nearby doc comment if needed.

if (video.readyState >= 1) {
setupTracks();
}

return () => {
video.removeEventListener("loadedmetadata", handleLoadedMetadata);
video.textTracks.removeEventListener("change", handleTrackChange);
video.textTracks.removeEventListener("addtrack", handleTrackChange);
video.textTracks.removeEventListener("removetrack", handleTrackChange);
if (captionTrack) {
captionTrack.removeEventListener("cuechange", handleCueChange);
}
Expand Down Expand Up @@ -335,13 +364,7 @@ export function HLSVideoPlayer({
autoPlay={autoplay}
>
<track default kind="chapters" src={chaptersSrc} />
<track
label="English"
kind="captions"
srcLang="en"
src={captionsSrc}
default
/>
<track label="English" kind="captions" srcLang="en" src={captionsSrc} />
</MediaPlayerVideo>
{currentCue && toggleCaptions && (
<div
Expand Down
Loading