From 0a71e7fb88e83a25bdb4eb629bd8e6827542c2c4 Mon Sep 17 00:00:00 2001 From: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com> Date: Sun, 31 Mar 2024 20:52:01 -0500 Subject: [PATCH] fix wrong segments when changing filters in motion only mode --- .../timeline/MotionReviewTimeline.tsx | 38 +++- .../components/timeline/ReviewTimeline.tsx | 209 +++++++++--------- web/src/hooks/use-draggable-element.ts | 36 ++- 3 files changed, 170 insertions(+), 113 deletions(-) diff --git a/web/src/components/timeline/MotionReviewTimeline.tsx b/web/src/components/timeline/MotionReviewTimeline.tsx index 43d5f324d..deac2ea44 100644 --- a/web/src/components/timeline/MotionReviewTimeline.tsx +++ b/web/src/components/timeline/MotionReviewTimeline.tsx @@ -4,6 +4,7 @@ import { useTimelineUtils } from "@/hooks/use-timeline-utils"; import { MotionData, ReviewSegment, ReviewSeverity } from "@/types/review"; import ReviewTimeline from "./ReviewTimeline"; import { isDesktop } from "react-device-detect"; +import { useMotionSegmentUtils } from "@/hooks/use-motion-segment-utils"; export type MotionReviewTimelineProps = { segmentDuration: number; @@ -75,14 +76,37 @@ export function MotionReviewTimeline({ [timelineStart, alignStartDateToTimeline, segmentDuration], ); + const { getMotionSegmentValue } = useMotionSegmentUtils( + segmentDuration, + motion_events, + ); + // Generate segments for the timeline const generateSegments = useCallback(() => { - const segmentCount = Math.ceil(timelineDuration / segmentDuration); + const segments = []; + let segmentTime = timelineStartAligned; - return Array.from({ length: segmentCount }, (_, index) => { - const segmentTime = timelineStartAligned - index * segmentDuration; + while (segmentTime >= timelineStartAligned - timelineDuration) { + const motionStart = segmentTime; + const motionEnd = motionStart + segmentDuration; - return ( + const segmentMotion = + getMotionSegmentValue(motionStart) > 0 || + getMotionSegmentValue(motionStart + segmentDuration / 2) > 0; + const overlappingReviewItems = events.some( + (item) => + (item.start_time >= motionStart && item.start_time < motionEnd) || + (item.end_time > motionStart && item.end_time <= motionEnd) || + (item.start_time <= motionStart && item.end_time >= motionEnd), + ); + + if ((!segmentMotion || overlappingReviewItems) && motionOnly) { + // exclude segment if necessary when in motion only mode + segmentTime -= segmentDuration; + continue; + } + + segments.push( + />, ); - }); + segmentTime -= segmentDuration; + } + return segments; // we know that these deps are correct // eslint-disable-next-line react-hooks/exhaustive-deps }, [ diff --git a/web/src/components/timeline/ReviewTimeline.tsx b/web/src/components/timeline/ReviewTimeline.tsx index 6a686c8e5..6d5ba883f 100644 --- a/web/src/components/timeline/ReviewTimeline.tsx +++ b/web/src/components/timeline/ReviewTimeline.tsx @@ -30,7 +30,7 @@ export type ReviewTimelineProps = { setExportEndTime?: React.Dispatch>; timelineCollapsed?: boolean; dense: boolean; - children: ReactNode; + children: ReactNode[]; }; export function ReviewTimeline({ @@ -113,6 +113,7 @@ export function ReviewTimeline({ setIsDragging: setIsDraggingHandlebar, draggableElementTimeRef: handlebarTimeRef, dense, + timelineSegments: children, }); const { @@ -136,6 +137,7 @@ export function ReviewTimeline({ draggableElementTimeRef: exportStartTimeRef, setDraggableElementPosition: setExportStartPosition, dense, + timelineSegments: children, }); const { @@ -159,6 +161,7 @@ export function ReviewTimeline({ draggableElementTimeRef: exportEndTimeRef, setDraggableElementPosition: setExportEndPosition, dense, + timelineSegments: children, }); const handleHandlebar = useCallback( @@ -321,119 +324,123 @@ export function ReviewTimeline({
{children} - {showHandlebar && ( -
-
-
-
-
-
-
-
-
-
- )} - {showExportHandles && ( + {children.length > 0 && ( <> -
+ {showHandlebar && (
-
-
-
-
-
-
-
-
-
-
-
+ className={`bg-destructive rounded-full mx-auto ${ + dense + ? "w-12 md:w-20" + : segmentDuration < 60 + ? "w-24" + : "w-20" + } h-5 ${isDraggingHandlebar && isMobile ? "fixed top-[18px] left-1/2 transform -translate-x-1/2 z-20 w-32 h-[30px] bg-destructive/80" : "static"} flex items-center justify-center`} + > +
+
- + )} + {showExportHandles && ( + <> +
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ + )} )} diff --git a/web/src/hooks/use-draggable-element.ts b/web/src/hooks/use-draggable-element.ts index 15b8773b2..1f957e39d 100644 --- a/web/src/hooks/use-draggable-element.ts +++ b/web/src/hooks/use-draggable-element.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useMemo, useState } from "react"; +import { ReactNode, useCallback, useEffect, useMemo, useState } from "react"; import { isMobile } from "react-device-detect"; import scrollIntoView from "scroll-into-view-if-needed"; import { useTimelineUtils } from "./use-timeline-utils"; @@ -23,6 +23,7 @@ type DraggableElementProps = { setIsDragging: React.Dispatch>; setDraggableElementPosition?: React.Dispatch>; dense: boolean; + timelineSegments: ReactNode[]; }; function useDraggableElement({ @@ -45,6 +46,7 @@ function useDraggableElement({ setIsDragging, setDraggableElementPosition, dense, + timelineSegments, }: DraggableElementProps) { const [clientYPosition, setClientYPosition] = useState(null); const [initialClickAdjustment, setInitialClickAdjustment] = useState(0); @@ -213,10 +215,10 @@ function useDraggableElement({ ); useEffect(() => { - if (timelineRef.current) { + if (timelineRef.current && timelineSegments.length) { setSegments(Array.from(timelineRef.current.querySelectorAll(".segment"))); } - }, [timelineRef, segmentDuration, timelineDuration, timelineCollapsed]); + }, [timelineRef, timelineCollapsed, timelineSegments]); useEffect(() => { let animationFrameId: number | null = null; @@ -426,7 +428,13 @@ function useDraggableElement({ ]); useEffect(() => { - if (timelineRef.current && draggableElementTime && timelineCollapsed) { + if ( + timelineRef.current && + draggableElementTime && + timelineCollapsed && + timelineSegments && + segments + ) { setFullTimelineHeight(timelineRef.current.scrollHeight); const alignedSegmentTime = alignStartDateToTimeline(draggableElementTime); @@ -452,14 +460,30 @@ function useDraggableElement({ if (setDraggableElementTime) { setDraggableElementTime(searchTime); } - break; + return; + } + } + } + if (!segmentElement) { + // segment still not found, just start at the beginning of the timeline or at now() + if (segments?.length) { + const searchTime = parseInt( + segments[0].getAttribute("data-segment-id") || "0", + 10, + ); + if (setDraggableElementTime) { + setDraggableElementTime(searchTime); + } + } else { + if (setDraggableElementTime) { + setDraggableElementTime(timelineStartAligned); } } } } // we know that these deps are correct // eslint-disable-next-line react-hooks/exhaustive-deps - }, [timelineCollapsed]); + }, [timelineCollapsed, segments]); useEffect(() => { if (timelineRef.current && segments) {