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
Original file line number Diff line number Diff line change
Expand Up @@ -1175,6 +1175,7 @@ describe('elements/content-sidebar/activity-feed-v2/ActivityFeedV2', () => {
};
return {
commitDrag: (payload: unknown) => listeners.comment_range_draft_change?.(payload),
dismissDraft: () => listeners.comment_range_draft_dismiss?.(undefined),
getViewer: () => viewer,
rangeEmits: () =>
viewer.emit.mock.calls.filter(([event]) => String(event).startsWith('comment_range_draft')),
Expand Down Expand Up @@ -1310,6 +1311,41 @@ describe('elements/content-sidebar/activity-feed-v2/ActivityFeedV2', () => {
cleanup();
}
});

test('should uncheck the timestamp toggle when the viewer dismisses the draft', async () => {
const { cleanup, media } = mountAudio();
const { commitDrag, dismissDraft, getViewer, rangeEmits } = createRangeViewer();
try {
render(
<ActivityFeedV2
currentUser={mockCurrentUser}
feedItems={[] as ActivityFeedV2Props['feedItems']}
file={audioFile}
getViewer={getViewer}
isAudioPlayerV2Enabled
isTimestampedCommentsEnabled
/>,
);
Object.defineProperty(media, 'currentTime', { configurable: true, value: 8.055, writable: true });
await act(async () => {
lastEditorProps.videoTimestamp?.onPressedChange(true);
});
await act(async () => {
commitDrag({ endMs: 12000, startMs: 8055 });
});
expect(lastEditorProps.videoTimestamp?.isPressed).toBe(true);

await act(async () => {
dismissDraft();
});

expect(lastEditorProps.videoTimestamp?.isPressed).toBe(false);
expect(lastEditorProps.videoTimestamp?.formattedTimestamp).toBe('0:08');
expect(rangeEmits().pop()).toEqual(['comment_range_draft_clear', undefined]);
} finally {
cleanup();
}
});
});

describe('filter controls', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,52 @@ describe('useMediaTimestamp range selection', () => {
}
});

test('should uncheck the toggle and drop the range when the viewer dismisses the draft', () => {
const { cleanup, emitFromViewer, viewer } = renderWithRange();
try {
act(() => screen.getByText('press').click());
act(() => emitFromViewer('comment_range_draft_change', { endMs: 50000, startMs: 44000 }));
act(() => emitFromViewer('comment_range_draft_dismiss', undefined));

expect(screen.getByTestId('pressed').textContent).toBe('false');
expect(screen.getByTestId('end-ms').textContent).toBe('undefined');
expect(emittedEvents(viewer).pop()).toEqual(['comment_range_draft_clear', undefined]);
} finally {
cleanup();
}
});

test('should ignore a dismiss when the toggle is already off', () => {
const { cleanup, emitFromViewer, viewer } = renderWithRange();
try {
act(() => emitFromViewer('comment_range_draft_dismiss', undefined));

expect(screen.getByTestId('pressed').textContent).toBe('false');
expect(emittedEvents(viewer)).toHaveLength(0);
} finally {
cleanup();
}
});

test('should let the start follow pause and seek again after a dismiss', () => {
const { audio, cleanup, emitFromViewer } = renderWithRange();
try {
act(() => screen.getByText('press').click());
act(() => emitFromViewer('comment_range_draft_change', { endMs: 50000, startMs: 44000 }));
act(() => emitFromViewer('comment_range_draft_dismiss', undefined));
act(() => screen.getByText('press').click());

Object.defineProperty(audio, 'currentTime', { configurable: true, value: 61, writable: true });
act(() => audio.dispatchEvent(new Event('pause')));

expect(screen.getByTestId('pressed').textContent).toBe('true');
expect(screen.getByTestId('ms').textContent).toBe('61000');
expect(screen.getByTestId('end-ms').textContent).toBe('undefined');
} finally {
cleanup();
}
});

test.each([
['a malformed payload', undefined],
['a non-numeric start', { endMs: 50000, startMs: 'nope' }],
Expand Down Expand Up @@ -773,6 +819,7 @@ describe('useMediaTimestamp range selection', () => {
try {
render(<TestHarness enabled getViewer={() => (isPreviewLoaded ? harness.viewer : null)} isAudioPlayerV2 />);
expect(harness.hasListener('comment_range_draft_change')).toBe(false);
expect(harness.hasListener('comment_range_draft_dismiss')).toBe(false);

isPreviewLoaded = true;
act(() => jest.advanceTimersByTime(500));
Expand Down Expand Up @@ -816,6 +863,7 @@ describe('useMediaTimestamp range selection', () => {

expect(emittedEvents(viewer)).toHaveLength(0);
expect(hasListener('comment_range_draft_change')).toBe(false);
expect(hasListener('comment_range_draft_dismiss')).toBe(false);
expect(screen.getByTestId('ms').textContent).toBe('43500');
} finally {
cleanup();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const VIEWER_POLL_MS = 500;
export const EVENT_RANGE_DRAFT = 'comment_range_draft';
export const EVENT_RANGE_DRAFT_CHANGE = 'comment_range_draft_change';
export const EVENT_RANGE_DRAFT_CLEAR = 'comment_range_draft_clear';
export const EVENT_RANGE_DRAFT_DISMISS = 'comment_range_draft_dismiss';

const findMediaElement = (): HTMLMediaElement | null => {
if (typeof document === 'undefined') {
Expand Down Expand Up @@ -72,6 +73,7 @@ const readRangeChange = (payload: unknown): { endMs?: number; startMs: number }
* - Pressed on while media is playing: captured value frozen until pause/seek.
* - Pressed on while media is paused: captured value updates on pause/seek.
* - Toggle off->on: captures current time and pauses the media if it was playing.
* - Viewer dismisses the draft: same as the user toggling off.
* - New media src: captured value resets to 0; pressed state persists. A dragged range survives
* untouched, since a src change on the same element is a token refresh, not different content.
* - New media element: any selected range is dropped.
Expand Down Expand Up @@ -131,17 +133,21 @@ export const useMediaTimestamp = (
}
}, [enabled]);

const uncheckTimestamp = React.useCallback(() => {
isPressedRef.current = false;
setIsPressed(false);
isRangePinnedRef.current = false;
setTimestampEndMs(undefined);
emitClear();
}, [emitClear]);

const onPressedChange = React.useCallback(
(pressed: boolean) => {
if (!enabled) {
return;
}
if (!pressed) {
isPressedRef.current = false;
setIsPressed(false);
isRangePinnedRef.current = false;
setTimestampEndMs(undefined);
emitClear();
uncheckTimestamp();
return;
}
const media = findMediaElement();
Expand All @@ -159,7 +165,7 @@ export const useMediaTimestamp = (
setTimestampEndMs(undefined);
emitDraft(capturedMs);
},
[emitClear, emitDraft, enabled],
[emitDraft, enabled, uncheckTimestamp],
);

React.useEffect(() => {
Expand Down Expand Up @@ -259,6 +265,15 @@ export const useMediaTimestamp = (
setTimestampEndMs(change.endMs);
};

// Click-outside on the waveform. The viewer has already taken its handles down, so the
// clear this echoes back is a no-op there, and there is nothing to echo with no draft up.
const handleRangeDismiss = () => {
if (!isPressedRef.current) {
return;
}
uncheckTimestamp();
};

// Poll for the viewer until we find one.
let attachedViewer: ViewerHandle | null = null;
let pollId: ReturnType<typeof setInterval> | undefined;
Expand All @@ -269,6 +284,7 @@ export const useMediaTimestamp = (
return;
}
viewer.addListener(EVENT_RANGE_DRAFT_CHANGE, handleRangeChange);
viewer.addListener(EVENT_RANGE_DRAFT_DISMISS, handleRangeDismiss);
attachedViewer = viewer;
clearInterval(pollId);
};
Expand All @@ -281,8 +297,9 @@ export const useMediaTimestamp = (
return () => {
clearInterval(pollId);
attachedViewer?.removeListener(EVENT_RANGE_DRAFT_CHANGE, handleRangeChange);
attachedViewer?.removeListener(EVENT_RANGE_DRAFT_DISMISS, handleRangeDismiss);
};
}, [getViewer, isRangeEnabled]);
}, [getViewer, isRangeEnabled, uncheckTimestamp]);

// Take down any handles still up for a composer that is going away.
React.useEffect(
Expand Down
Loading