From f8e24f732c9ec8b43ac136a9a6030707a554dbed Mon Sep 17 00:00:00 2001 From: Bryan Date: Tue, 20 Aug 2024 17:11:14 -0700 Subject: [PATCH] Explicitly track offsetX in timeline context menu (#1435) * fix: explicitly keep track of the timeline context menu position for firefox compatibility --- src/components/timeline/Timeline.svelte | 2 +- .../timeline/TimelineContextMenu.svelte | 14 ++++---- .../timeline/TimelineViewControls.svelte | 36 +++++++++++-------- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/components/timeline/Timeline.svelte b/src/components/timeline/Timeline.svelte index 1ac644e730..6b8a219fe1 100644 --- a/src/components/timeline/Timeline.svelte +++ b/src/components/timeline/Timeline.svelte @@ -263,7 +263,7 @@ export function viewTimeRangeChanged(viewTimeRange: TimeRange, zoomTransform?: ZoomTransform) { dispatch('viewTimeRangeChanged', viewTimeRange); - // Assign zoom transform if provided to syncronize all d3 zoom handlers + // Assign zoom transform if provided to synchronize all d3 zoom handlers if (zoomTransform) { timelineZoomTransform = zoomTransform; } else { diff --git a/src/components/timeline/TimelineContextMenu.svelte b/src/components/timeline/TimelineContextMenu.svelte index dd56ff2582..3f8ea83b2b 100644 --- a/src/components/timeline/TimelineContextMenu.svelte +++ b/src/components/timeline/TimelineContextMenu.svelte @@ -69,6 +69,7 @@ let hasActivityLayer: boolean = false; let mouseOverOrigin: MouseOverOrigin | undefined = undefined; let row: Row | undefined = undefined; + let offsetX: number | undefined; // TODO imports here could be better, should we handle the vertical guide creation in Timeline? $: timelines = $view?.definition.plan.timelines ?? []; @@ -106,6 +107,8 @@ $: activityDirectiveStartDate = activityDirective ? new Date(getUnixEpochTimeFromInterval(planStartTimeYmd, activityDirective.start_offset)) : null; + // Explicitly keep track of offsetX because Firefox ends up zeroing it out on the original `contextmenu` MouseEvent + $: offsetX = contextMenu?.e.offsetX; function jumpToActivityDirective() { if (span !== null) { @@ -163,8 +166,8 @@ } function onZoom(duration: number) { - if (xScaleView && contextMenu) { - const time = activityDirectiveStartDate ? activityDirectiveStartDate : xScaleView.invert(contextMenu.e.offsetX); + if (xScaleView && offsetX !== undefined) { + const time = activityDirectiveStartDate ? activityDirectiveStartDate : xScaleView.invert(offsetX); const newViewTimeRange: TimeRange = { end: Math.min(time.getTime() + duration / 2, maxTimeRange.end), start: Math.max(time.getTime() - duration / 2, maxTimeRange.start), @@ -353,7 +356,7 @@ {:else} xScaleView && contextMenu && addVerticalGuide(xScaleView.invert(contextMenu.e.offsetX))} + on:click={() => xScaleView && offsetX !== undefined && addVerticalGuide(xScaleView.invert(offsetX))} > Place Guide @@ -370,8 +373,7 @@ }, ], ]} - on:click={() => - xScaleView && contextMenu && updateSimulationStartTime(xScaleView.invert(contextMenu.e.offsetX))} + on:click={() => xScaleView && offsetX !== undefined && updateSimulationStartTime(xScaleView.invert(offsetX))} > Set Simulation Start @@ -387,7 +389,7 @@ }, ], ]} - on:click={() => xScaleView && contextMenu && updateSimulationEndTime(xScaleView.invert(contextMenu.e.offsetX))} + on:click={() => xScaleView && offsetX !== undefined && updateSimulationEndTime(xScaleView.invert(offsetX))} > Set Simulation End diff --git a/src/components/timeline/TimelineViewControls.svelte b/src/components/timeline/TimelineViewControls.svelte index 6209394376..903ff1fb15 100644 --- a/src/components/timeline/TimelineViewControls.svelte +++ b/src/components/timeline/TimelineViewControls.svelte @@ -69,22 +69,28 @@ scrollIfOffscreen(); } - $: if ( - typeof window !== 'undefined' && - ($selectedActivityDirective || $selectedSpan || $simulationDatasetId !== null) - ) { - viewURL = new URL(window.location.href); - viewURL.searchParams.set(SearchParameters.START_TIME, new Date(viewTimeRange.start).toISOString()); - viewURL.searchParams.set(SearchParameters.END_TIME, new Date(viewTimeRange.end).toISOString()); - if ($selectedActivityDirective) { - viewURL.searchParams.set(SearchParameters.ACTIVITY_ID, $selectedActivityDirective.id.toFixed()); - } - if ($selectedSpan) { - viewURL.searchParams.set(SearchParameters.SPAN_ID, $selectedSpan.span_id.toFixed()); - } - if ($simulationDatasetId) { - viewURL.searchParams.set(SearchParameters.SIMULATION_DATASET_ID, $simulationDatasetId.toFixed()); + // The time range can potentially become invalid by zooming in too much which will cause the following to fail + // A try/catch is used here to prevent the UI from becoming unresponsive if an invalid time range is provided + $: try { + if ( + typeof window !== 'undefined' && + ($selectedActivityDirective || $selectedSpan || $simulationDatasetId !== null) + ) { + viewURL = new URL(window.location.href); + viewURL.searchParams.set(SearchParameters.START_TIME, new Date(viewTimeRange.start).toISOString()); + viewURL.searchParams.set(SearchParameters.END_TIME, new Date(viewTimeRange.end).toISOString()); + if ($selectedActivityDirective) { + viewURL.searchParams.set(SearchParameters.ACTIVITY_ID, $selectedActivityDirective.id.toFixed()); + } + if ($selectedSpan) { + viewURL.searchParams.set(SearchParameters.SPAN_ID, $selectedSpan.span_id.toFixed()); + } + if ($simulationDatasetId) { + viewURL.searchParams.set(SearchParameters.SIMULATION_DATASET_ID, $simulationDatasetId.toFixed()); + } } + } catch (e) { + console.log(e); } function onKeydown(e: KeyboardEvent) {