Skip to content

improvement: better time formatting for clip track #1005

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ameer2468 merged 2 commits into from
Sep 12, 2025
Merged

improvement: better time formatting for clip track #1005

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2a8c5c
Update ClipTrack.tsx
ameer2468 Sep 12, 2025
aa7a8db
Update ClipTrack.tsx
ameer2468 Sep 12, 2025
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
23 changes: 17 additions & 6 deletions apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,20 @@ import {
useSegmentWidth,
} from "./Track";

function formatTime(totalSeconds: number): string {
const hours = Math.floor(totalSeconds / 3600);
const minutes = Math.floor((totalSeconds % 3600) / 60);
const seconds = Math.floor(totalSeconds % 60);

if (hours > 0) {
return `${hours}h ${minutes}m ${seconds}s`;
} else if (minutes > 0) {
return `${minutes}m ${seconds}s`;
} else {
return `${seconds}s`;
}
}
Comment on lines +32 to +44
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Fix sub-second rounding and avoid name collision with existing utils.formatTime.

Current logic floors everything (e.g., 0.4s -> "0s"), which is misleading in CutOffsetButton and a UX regression from the prior 0.1s precision. Also, naming this helper formatTime collides conceptually with apps/desktop/src/routes/editor/utils.ts:formatTime (mm:ss[.ff]) and can confuse future readers.

Suggested: rename and improve formatting to preserve tenths under 10s, handle negatives, and keep integer seconds when minutes/hours are present.

-function formatTime(totalSeconds: number): string {
-	const hours = Math.floor(totalSeconds / 3600);
-	const minutes = Math.floor((totalSeconds % 3600) / 60);
-	const seconds = Math.floor(totalSeconds % 60);
-
-	if (hours > 0) {
-		return `${hours}h ${minutes}m ${seconds}s`;
-	} else if (minutes > 0) {
-		return `${minutes}m ${seconds}s`;
-	} else {
-		return `${seconds}s`;
-	}
-}
+function formatDuration(totalSeconds: number): string {
+	const sign = totalSeconds < 0 ? "-" : "";
+	const s = Math.abs(totalSeconds);
+	const hours = Math.floor(s / 3600);
+	const minutes = Math.floor((s % 3600) / 60);
+	const secFloat = s % 60;
+	// Mixed units: keep integer seconds to avoid 60s rollovers
+	if (hours > 0) return `${sign}${hours}h ${minutes}m ${Math.floor(secFloat)}s`;
+	if (minutes > 0) return `${sign}${minutes}m ${Math.floor(secFloat)}s`;
+	// Sub-minute: show tenths for <10s, else whole seconds
+	const secs = secFloat < 10 ? Math.round(secFloat * 10) / 10 : Math.round(secFloat);
+	return `${sign}${secs.toFixed(secFloat < 10 ? 1 : 0)}s`;
+}

Follow-up: consider centralizing this in editor/utils as a separate humanizer (e.g., humanizeDuration) to avoid two different “formatTime” semantics.

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "PWD: $(pwd)"
echo "Git branch:"
git rev-parse --abbrev-ref HEAD || true

echo
echo "Searching for 'formatTime' occurrences (definitions & calls)..."
rg -n --hidden -S -C2 '\bfunction\s+formatTime\b|\bformatTime\s*\(' || true

echo
echo "Show ClipTrack.tsx if present:"
if [ -f "apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx" ]; then
  sed -n '1,240p' apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx
else
  rg -n --hidden -S 'ClipTrack.tsx' || true
fi

echo
echo "Show editor utils if present (possible other formatTime):"
if [ -f "apps/desktop/src/routes/editor/utils.ts" ]; then
  sed -n '1,240p' apps/desktop/src/routes/editor/utils.ts
else
  rg -n --hidden -S --glob '!node_modules' 'src/routes/editor/utils.ts|routes/editor/utils.ts|/routes/editor/utils.ts' || true
  rg -n --hidden -S -C2 '\bformatTime\b' || true
fi

echo
echo "Search for CutOffsetButton usages:"
rg -n --hidden -S -C3 'CutOffsetButton' || true

echo
echo "Search for existing humanize/formatDuration helpers:"
rg -n --hidden -S -C2 'formatDuration|humanizeDuration|humanizeTime|formatTimeHuman' || true

Length of output: 22669

Fix sub-second rounding and rename local formatTime to avoid collision

  • Problem: apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx defines a local formatTime that floors seconds (0.4s -> "0s"), causing a UX regression in CutOffsetButton; it also conceptually collides with apps/desktop/src/routes/editor/utils.ts:formatTime (mm:ss).
  • Change: replace the helper in ClipTrack.tsx with the implementation below (renamed to formatDuration and preserving tenths for <10s, handling negatives, keeping integer seconds when minutes/hours present):
-function formatTime(totalSeconds: number): string {
-	const hours = Math.floor(totalSeconds / 3600);
-	const minutes = Math.floor((totalSeconds % 3600) / 60);
-	const seconds = Math.floor(totalSeconds % 60);
-
-	if (hours > 0) {
-		return `${hours}h ${minutes}m ${seconds}s`;
-	} else if (minutes > 0) {
-		return `${minutes}m ${seconds}s`;
-	} else {
-		return `${seconds}s`;
-	}
-}
+function formatDuration(totalSeconds: number): string {
+	const sign = totalSeconds < 0 ? "-" : "";
+	const s = Math.abs(totalSeconds);
+	const hours = Math.floor(s / 3600);
+	const minutes = Math.floor((s % 3600) / 60);
+	const secFloat = s % 60;
+	// Mixed units: keep integer seconds to avoid 60s rollovers
+	if (hours > 0) return `${sign}${hours}h ${minutes}m ${Math.floor(secFloat)}s`;
+	if (minutes > 0) return `${sign}${minutes}m ${Math.floor(secFloat)}s`;
+	// Sub-minute: show tenths for <10s, else whole seconds
+	const secs = secFloat < 10 ? Math.round(secFloat * 10) / 10 : Math.round(secFloat);
+	return `${sign}${secs.toFixed(secFloat < 10 ? 1 : 0)}s`;
+}
  • Follow-up: centralize as a humanizeDuration function in apps/desktop/src/routes/editor/utils.ts to avoid two different “formatTime” semantics.
📝 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
function formatTime(totalSeconds: number): string {
const hours = Math.floor(totalSeconds / 3600);
const minutes = Math.floor((totalSeconds % 3600) / 60);
const seconds = Math.floor(totalSeconds % 60);
if (hours > 0) {
return `${hours}h ${minutes}m ${seconds}s`;
} else if (minutes > 0) {
return `${minutes}m ${seconds}s`;
} else {
return `${seconds}s`;
}
}
function formatDuration(totalSeconds: number): string {
const sign = totalSeconds < 0 ? "-" : "";
const s = Math.abs(totalSeconds);
const hours = Math.floor(s / 3600);
const minutes = Math.floor((s % 3600) / 60);
const secFloat = s % 60;
// Mixed units: keep integer seconds to avoid 60s rollovers
if (hours > 0) return `${sign}${hours}h ${minutes}m ${Math.floor(secFloat)}s`;
if (minutes > 0) return `${sign}${minutes}m ${Math.floor(secFloat)}s`;
// Sub-minute: show tenths for <10s, else whole seconds
const secs = secFloat < 10 ? Math.round(secFloat * 10) / 10 : Math.round(secFloat);
return `${sign}${secs.toFixed(secFloat < 10 ? 1 : 0)}s`;
}
🤖 Prompt for AI Agents 
In apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx around lines 22–34, the
local formatTime helper floors seconds (producing "0s" for 0.4s) and collides
conceptually with utils.formatTime; rename it to formatDuration, handle negative
values by prefixing '-' and formatting the absolute value, preserve tenths of a
second for durations under 10s (e.g., 0.4s -> "0.4s", 9.8s -> "9.8s"), and keep
integer seconds when minutes or hours are present (hours -> "1h 2m 3s", minutes
-> "2m 3s"); replace Math.floor on seconds with logic that uses one decimal for
<10s and integer seconds otherwise, and then consider centralizing this as
humanizeDuration in apps/desktop/src/routes/editor/utils.ts to avoid duplicate
semantics.


function WaveformCanvas(props: {
systemWaveform?: number[];
micWaveform?: number[];
Expand Down Expand Up @@ -468,7 +482,7 @@ export function ClipTrack(
</span>
<div class="flex gap-1 items-center text-md dark:text-gray-12 text-gray-1">
<IconLucideClock class="size-3.5" />{" "}
{(segment.end - segment.start).toFixed(1)}s
{formatTime(segment.end - segment.start)}
</div>
</div>
</Show>
Expand Down Expand Up @@ -627,21 +641,18 @@ function CutOffsetButton(props: {
class?: string;
onClick?(): void;
}) {
const formatTime = (t: number) =>
t < 1 ? Math.round(t * 10) / 10 : Math.round(t);

return (
<button
class={cx(
"h-7 bg-red-300 hover:bg-red-400 text-xs tabular-nums text-white p-2 flex flex-row items-center transition-colors",
"h-7 bg-red-300 text-nowrap hover:bg-red-400 text-xs tabular-nums text-white p-2 flex flex-row items-center transition-colors",
props.class,
)}
onClick={() => props.onClick?.()}
>
{props.value === 0 ? (
<IconCapScissors class="size-3.5" />
) : (
<>{formatTime(props.value)}s</>
<>{formatTime(props.value)}</>
)}
Comment on lines +655 to 656
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 12, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Update call site to the renamed formatter.

Aligns with the improved duration formatting and sub-second handling.

-				<>{formatTime(props.value)}</>
+				<>{formatDuration(props.value)}</>
📝 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
<>{formatTime(props.value)}</>
)}
<>{formatDuration(props.value)}</>
)}
🤖 Prompt for AI Agents 
In apps/desktop/src/routes/editor/Timeline/ClipTrack.tsx around lines 655-656,
the call to the old formatter formatTime(props.value) must be updated to the
renamed formatter (formatDuration) to get the improved duration and sub-second
handling; replace the call with formatDuration(props.value) and update or add
the import from the formatter module if necessary so the new function is
available in this file.

</button>
);
Expand Down
Loading