Skip to content

Commit d9b1d6c

Browse files
authored
feat: Improve Visual Distinction Between Trim and Chapters Editors (#1445)
* Update .gitignore * feat: Improve Visual Distinction Between Trim and Chapters Editors * fix: Convert timeline header styles to CSS classes Moved inline styles for timeline headers in chapters and video editors to dedicated CSS classes for better maintainability and consistency. * Bump version to 7.3.0 Update the VERSION in cms/version.py to 7.3.0 for the new release. * build assets * Update segment color schemes in video and chapters editor. * build assets * build assets * fix: Prevent Safari from resetting segments after drag operations Prevent Safari from resetting segments when loadedmetadata fires multiple times and fix stale state issues in click handlers by using refs instead of closure variables. * build assets * Bump version to 7.3.0-beta.3 Update the VERSION string in cms/version.py to reflect the new pre-release version 7.3.0-beta.3.
1 parent aeef828 commit d9b1d6c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+398
-254
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,4 @@ frontend-tools/video-editor/client/public/videos/sample-video.mp3
3535
frontend-tools/chapters-editor/client/public/videos/sample-video.mp3
3636
static/chapters_editor/videos/sample-video.mp3
3737
static/video_editor/videos/sample-video.mp3
38+
templates/todo-MS4.md

cms/version.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
VERSION = "7.2.2"
1+
VERSION = "7.3.0-beta.3"

frontend-tools/chapters-editor/client/src/App.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,11 @@ const App = () => {
150150
canRedo={historyPosition < history.length - 1}
151151
/>
152152

153+
{/* Timeline Header */}
154+
<div className="timeline-header-container">
155+
<h2 className="timeline-header-title">Add Chapters</h2>
156+
</div>
157+
153158
{/* Timeline Controls */}
154159
<TimelineControls
155160
currentTime={currentTime}

frontend-tools/chapters-editor/client/src/components/ClipSegments.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ const ClipSegments = ({ segments, selectedSegmentId }: ClipSegmentsProps) => {
2828

2929
// Generate the same color background for a segment as shown in the timeline
3030
const getSegmentColorClass = (index: number) => {
31-
// Return CSS class based on index modulo 8
32-
// This matches the CSS nth-child selectors in the timeline
33-
return `segment-default-color segment-color-${(index % 8) + 1}`;
31+
// Return CSS class based on index modulo 20
32+
// This matches the CSS classes for up to 20 segments
33+
return `segment-default-color segment-color-${(index % 20) + 1}`;
3434
};
3535

3636
// Get selected segment
@@ -65,8 +65,8 @@ const ClipSegments = ({ segments, selectedSegmentId }: ClipSegmentsProps) => {
6565
<div className="segment-actions">
6666
<button
6767
className="delete-button"
68-
aria-label="Delete Segment"
69-
data-tooltip="Delete this segment"
68+
aria-label="Delete Chapter"
69+
data-tooltip="Delete this chapter"
7070
onClick={() => handleDeleteSegment(segment.id)}
7171
>
7272
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20" fill="currentColor">

frontend-tools/chapters-editor/client/src/components/TimelineControls.tsx

Lines changed: 76 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,16 @@ const TimelineControls = ({
177177
const [isAutoSaving, setIsAutoSaving] = useState(false);
178178
const autoSaveTimerRef = useRef<NodeJS.Timeout | null>(null);
179179
const clipSegmentsRef = useRef(clipSegments);
180-
180+
// Track when a drag just ended to prevent Safari from triggering clicks after drag
181+
const dragJustEndedRef = useRef<boolean>(false);
182+
const dragEndTimeoutRef = useRef<NodeJS.Timeout | null>(null);
183+
184+
// Helper function to detect Safari browser
185+
const isSafari = () => {
186+
if (typeof window === 'undefined') return false;
187+
const userAgent = navigator.userAgent || navigator.vendor || (window as any).opera;
188+
return /Safari/.test(userAgent) && !/Chrome/.test(userAgent) && !/Chromium/.test(userAgent);
189+
};
181190

182191
// Keep clipSegmentsRef updated
183192
useEffect(() => {
@@ -867,6 +876,12 @@ const TimelineControls = ({
867876
logger.debug('Clearing auto-save timer in cleanup:', autoSaveTimerRef.current);
868877
clearTimeout(autoSaveTimerRef.current);
869878
}
879+
880+
// Clear any pending drag end timeout
881+
if (dragEndTimeoutRef.current) {
882+
clearTimeout(dragEndTimeoutRef.current);
883+
dragEndTimeoutRef.current = null;
884+
}
870885
};
871886
}, [scheduleAutoSave]);
872887

@@ -1084,16 +1099,20 @@ const TimelineControls = ({
10841099
};
10851100

10861101
// Helper function to calculate available space for a new segment
1087-
const calculateAvailableSpace = (startTime: number): number => {
1102+
const calculateAvailableSpace = (startTime: number, segmentsOverride?: Segment[]): number => {
10881103
// Always return at least 0.1 seconds to ensure tooltip shows
10891104
const MIN_SPACE = 0.1;
10901105

1106+
// Use override segments if provided, otherwise use ref to get latest segments
1107+
// This ensures we always have the most up-to-date segments, especially important for Safari
1108+
const segmentsToUse = segmentsOverride || clipSegmentsRef.current;
1109+
10911110
// Determine the amount of available space:
10921111
// 1. Check remaining space until the end of video
10931112
const remainingDuration = Math.max(0, duration - startTime);
10941113

10951114
// 2. Find the next segment (if any)
1096-
const sortedSegments = [...clipSegments].sort((a, b) => a.startTime - b.startTime);
1115+
const sortedSegments = [...segmentsToUse].sort((a, b) => a.startTime - b.startTime);
10971116

10981117
// Find the next and previous segments
10991118
const nextSegment = sortedSegments.find((seg) => seg.startTime > startTime);
@@ -1109,14 +1128,6 @@ const TimelineControls = ({
11091128
availableSpace = duration - startTime;
11101129
}
11111130

1112-
// Log the space calculation for debugging
1113-
logger.debug('Space calculation:', {
1114-
position: formatDetailedTime(startTime),
1115-
nextSegment: nextSegment ? formatDetailedTime(nextSegment.startTime) : 'none',
1116-
prevSegment: prevSegment ? formatDetailedTime(prevSegment.endTime) : 'none',
1117-
availableSpace: formatDetailedTime(Math.max(MIN_SPACE, availableSpace)),
1118-
});
1119-
11201131
// Always return at least MIN_SPACE to ensure tooltip shows
11211132
return Math.max(MIN_SPACE, availableSpace);
11221133
};
@@ -1125,16 +1136,19 @@ const TimelineControls = ({
11251136
const updateTooltipForPosition = (currentPosition: number) => {
11261137
if (!timelineRef.current) return;
11271138

1139+
// Use ref to get latest segments to avoid stale state issues
1140+
const currentSegments = clipSegmentsRef.current;
1141+
11281142
// Find if we're in a segment at the current position with a small tolerance
1129-
const segmentAtPosition = clipSegments.find((seg) => {
1143+
const segmentAtPosition = currentSegments.find((seg) => {
11301144
const isWithinSegment = currentPosition >= seg.startTime && currentPosition <= seg.endTime;
11311145
const isVeryCloseToStart = Math.abs(currentPosition - seg.startTime) < 0.001;
11321146
const isVeryCloseToEnd = Math.abs(currentPosition - seg.endTime) < 0.001;
11331147
return isWithinSegment || isVeryCloseToStart || isVeryCloseToEnd;
11341148
});
11351149

11361150
// Find the next and previous segments
1137-
const sortedSegments = [...clipSegments].sort((a, b) => a.startTime - b.startTime);
1151+
const sortedSegments = [...currentSegments].sort((a, b) => a.startTime - b.startTime);
11381152
const nextSegment = sortedSegments.find((seg) => seg.startTime > currentPosition);
11391153
const prevSegment = [...sortedSegments].reverse().find((seg) => seg.endTime < currentPosition);
11401154

@@ -1144,21 +1158,13 @@ const TimelineControls = ({
11441158
setShowEmptySpaceTooltip(false);
11451159
} else {
11461160
// We're in a cutaway area
1147-
// Calculate available space for new segment
1148-
const availableSpace = calculateAvailableSpace(currentPosition);
1161+
// Calculate available space for new segment using current segments
1162+
const availableSpace = calculateAvailableSpace(currentPosition, currentSegments);
11491163
setAvailableSegmentDuration(availableSpace);
11501164

11511165
// Always show empty space tooltip
11521166
setSelectedSegmentId(null);
11531167
setShowEmptySpaceTooltip(true);
1154-
1155-
// Log position info for debugging
1156-
logger.debug('Cutaway position:', {
1157-
current: formatDetailedTime(currentPosition),
1158-
prevSegmentEnd: prevSegment ? formatDetailedTime(prevSegment.endTime) : 'none',
1159-
nextSegmentStart: nextSegment ? formatDetailedTime(nextSegment.startTime) : 'none',
1160-
availableSpace: formatDetailedTime(availableSpace),
1161-
});
11621168
}
11631169

11641170
// Update tooltip position
@@ -1188,14 +1194,19 @@ const TimelineControls = ({
11881194

11891195
if (!timelineRef.current || !scrollContainerRef.current) return;
11901196

1197+
// Safari-specific fix: Ignore clicks that happen immediately after a drag operation
1198+
// Safari fires click events after drag ends, which can cause issues with stale state
1199+
if (isSafari() && dragJustEndedRef.current) {
1200+
return;
1201+
}
1202+
11911203
// If on mobile device and video hasn't been initialized, don't handle timeline clicks
11921204
if (isIOSUninitialized) {
11931205
return;
11941206
}
11951207

11961208
// Check if video is globally playing before the click
11971209
const wasPlaying = videoRef.current && !videoRef.current.paused;
1198-
logger.debug('Video was playing before timeline click:', wasPlaying);
11991210

12001211
// Reset continuation flag when clicking on timeline - ensures proper boundary detection
12011212
setContinuePastBoundary(false);
@@ -1216,14 +1227,6 @@ const TimelineControls = ({
12161227

12171228
const newTime = position * duration;
12181229

1219-
// Log the position for debugging
1220-
logger.debug(
1221-
'Timeline clicked at:',
1222-
formatDetailedTime(newTime),
1223-
'distance from end:',
1224-
formatDetailedTime(duration - newTime)
1225-
);
1226-
12271230
// Store position globally for iOS Safari (this is critical for first-time visits)
12281231
if (typeof window !== 'undefined') {
12291232
window.lastSeekedPosition = newTime;
@@ -1236,8 +1239,12 @@ const TimelineControls = ({
12361239
setClickedTime(newTime);
12371240
setDisplayTime(newTime);
12381241

1242+
// Use ref to get latest segments to avoid stale state issues, especially in Safari
1243+
// Safari can fire click events immediately after drag before React re-renders
1244+
const currentSegments = clipSegmentsRef.current;
1245+
12391246
// Find if we clicked in a segment with a small tolerance for boundaries
1240-
const segmentAtClickedTime = clipSegments.find((seg) => {
1247+
const segmentAtClickedTime = currentSegments.find((seg) => {
12411248
// Standard check for being inside a segment
12421249
const isInside = newTime >= seg.startTime && newTime <= seg.endTime;
12431250
// Additional checks for being exactly at the start or end boundary (with small tolerance)
@@ -1258,7 +1265,7 @@ const TimelineControls = ({
12581265
if (isPlayingSegments && wasPlaying) {
12591266
// Update the current segment index if we clicked into a segment
12601267
if (segmentAtClickedTime) {
1261-
const orderedSegments = [...clipSegments].sort((a, b) => a.startTime - b.startTime);
1268+
const orderedSegments = [...currentSegments].sort((a, b) => a.startTime - b.startTime);
12621269
const targetSegmentIndex = orderedSegments.findIndex((seg) => seg.id === segmentAtClickedTime.id);
12631270

12641271
if (targetSegmentIndex !== -1) {
@@ -1311,8 +1318,9 @@ const TimelineControls = ({
13111318
// We're in a cutaway area - always show tooltip
13121319
setSelectedSegmentId(null);
13131320

1314-
// Calculate the available space for a new segment
1315-
const availableSpace = calculateAvailableSpace(newTime);
1321+
// Calculate the available space for a new segment using current segments from ref
1322+
// This ensures we use the latest segments even if React hasn't re-rendered yet
1323+
const availableSpace = calculateAvailableSpace(newTime, currentSegments);
13161324
setAvailableSegmentDuration(availableSpace);
13171325

13181326
// Calculate and set tooltip position correctly for zoomed timeline
@@ -1334,18 +1342,6 @@ const TimelineControls = ({
13341342

13351343
// Always show the empty space tooltip in cutaway areas
13361344
setShowEmptySpaceTooltip(true);
1337-
1338-
// Log the cutaway area details
1339-
const sortedSegments = [...clipSegments].sort((a, b) => a.startTime - b.startTime);
1340-
const prevSegment = [...sortedSegments].reverse().find((seg) => seg.endTime < newTime);
1341-
const nextSegment = sortedSegments.find((seg) => seg.startTime > newTime);
1342-
1343-
logger.debug('Clicked in cutaway area:', {
1344-
position: formatDetailedTime(newTime),
1345-
availableSpace: formatDetailedTime(availableSpace),
1346-
prevSegmentEnd: prevSegment ? formatDetailedTime(prevSegment.endTime) : 'none',
1347-
nextSegmentStart: nextSegment ? formatDetailedTime(nextSegment.startTime) : 'none',
1348-
});
13491345
}
13501346
}
13511347
};
@@ -1498,6 +1494,10 @@ const TimelineControls = ({
14981494
return seg;
14991495
});
15001496

1497+
// Update the ref immediately during drag to ensure we always have latest segments
1498+
// This is critical for Safari which may fire events before React re-renders
1499+
clipSegmentsRef.current = updatedSegments;
1500+
15011501
// Create a custom event to update the segments WITHOUT recording in history during drag
15021502
const updateEvent = new CustomEvent('update-segments', {
15031503
detail: {
@@ -1582,6 +1582,26 @@ const TimelineControls = ({
15821582
return seg;
15831583
});
15841584

1585+
// CRITICAL: Update the ref immediately with the new segments
1586+
// This ensures that if Safari fires a click event before React re-renders,
1587+
// the click handler will use the updated segments instead of stale ones
1588+
clipSegmentsRef.current = finalSegments;
1589+
1590+
// Safari-specific fix: Set flag to ignore clicks immediately after drag
1591+
// Safari fires click events after drag ends, which can interfere with state updates
1592+
if (isSafari()) {
1593+
dragJustEndedRef.current = true;
1594+
// Clear the flag after a delay to allow React to re-render with updated segments
1595+
// Increased timeout to ensure state has propagated
1596+
if (dragEndTimeoutRef.current) {
1597+
clearTimeout(dragEndTimeoutRef.current);
1598+
}
1599+
dragEndTimeoutRef.current = setTimeout(() => {
1600+
dragJustEndedRef.current = false;
1601+
dragEndTimeoutRef.current = null;
1602+
}, 200); // 200ms to ensure React has processed the state update and re-rendered
1603+
}
1604+
15851605
// Now we can create a history record for the complete drag operation
15861606
const actionType = isLeft ? 'adjust_segment_start' : 'adjust_segment_end';
15871607
document.dispatchEvent(
@@ -1594,6 +1614,13 @@ const TimelineControls = ({
15941614
})
15951615
);
15961616

1617+
// Dispatch segment-drag-end event for other listeners
1618+
document.dispatchEvent(
1619+
new CustomEvent('segment-drag-end', {
1620+
detail: { segmentId },
1621+
})
1622+
);
1623+
15971624
// After drag is complete, do a final check to see if playhead is inside the segment
15981625
if (selectedSegmentId === segmentId && videoRef.current) {
15991626
const currentTime = videoRef.current.currentTime;
@@ -3943,9 +3970,7 @@ const TimelineControls = ({
39433970
<button
39443971
onClick={() => setShowSaveChaptersModal(true)}
39453972
className="save-chapters-button"
3946-
data-tooltip={clipSegments.length === 0
3947-
? "Clear all chapters"
3948-
: "Save chapters"}
3973+
{...(clipSegments.length === 0 && { 'data-tooltip': 'Clear all chapters' })}
39493974
>
39503975
{clipSegments.length === 0
39513976
? 'Clear Chapters'

0 commit comments

Comments
 (0)