Skip to content

Commit

Permalink
Activity Layer Context Menu Fix (#1146)
Browse files Browse the repository at this point in the history
Immediately compute selected directive or span on right click instead of waiting a frame in order to avoid triggering unexpected context menu appearance
  • Loading branch information
AaronPlave authored Mar 11, 2024
1 parent f4dc1b6 commit 2900086
Showing 1 changed file with 45 additions and 44 deletions.
89 changes: 45 additions & 44 deletions src/components/timeline/LayerActivity.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -259,26 +259,31 @@
}
}
function getDirectivesAndSpansForOffset(offsetX: number, offsetY: number) {
const activityDirectives = searchQuadtreeRect<ActivityDirective>(
quadtreeActivityDirectives,
offsetX,
offsetY,
activityHeight,
maxActivityWidth,
visibleActivityDirectivesById,
);
const spans = searchQuadtreeRect<Span>(
quadtreeSpans,
offsetX,
offsetY,
activityHeight,
maxActivityWidth,
visibleSpansById,
);
return { activityDirectives, spans };
}
function onMousedown(e: MouseEvent | undefined): void {
// Do not process events if meta/ctrl is pressed to avoid interaction conflicts with zoom/pan
if (e && timelineInteractionMode === TimelineInteractionMode.Interact) {
const { offsetX, offsetY } = e;
const activityDirectives = searchQuadtreeRect<ActivityDirective>(
quadtreeActivityDirectives,
offsetX,
offsetY,
activityHeight,
maxActivityWidth,
visibleActivityDirectivesById,
);
const spans = searchQuadtreeRect<Span>(
quadtreeSpans,
offsetX,
offsetY,
activityHeight,
maxActivityWidth,
visibleSpansById,
);
const { activityDirectives, spans } = getDirectivesAndSpansForOffset(offsetX, offsetY);
/**
* The setTimeout is needed to prevent a race condition with mousedown events and change events.
Expand All @@ -302,22 +307,9 @@
let activityDirectives: ActivityDirective[] = [];
let spans: Span[] = [];
if (mode === 'packed') {
activityDirectives = searchQuadtreeRect<ActivityDirective>(
quadtreeActivityDirectives,
offsetX,
offsetY,
activityHeight,
maxActivityWidth,
visibleActivityDirectivesById,
);
spans = searchQuadtreeRect<Span>(
quadtreeSpans,
offsetX,
offsetY,
activityHeight,
maxActivityWidth,
visibleSpansById,
);
const hits = getDirectivesAndSpansForOffset(offsetX, offsetY);
activityDirectives = hits.activityDirectives;
spans = hits.spans;
} else {
activityDirectives = searchQuadtreeRect<ActivityDirective>(
quadtreeHeatmap,
Expand Down Expand Up @@ -364,18 +356,27 @@
}
const showContextMenu = !!e && isRightClick(e);
if (showContextMenu) {
// TODO if we don't move this to the end of the call stack we risk not having selectedActivityDirectiveId or selectedSpanId
// properly selected since the right click has to trigger selected entity store update first..
// Could potentially do a quadtree search based off right click position instead of relying on these stores?
setTimeout(() => {
dispatch('contextMenu', {
e,
layerId: id,
origin: 'layer-activity',
selectedActivityDirectiveId,
selectedSpanId,
});
}, 0);
// Get new selected directive or span in order to ensure that no race condition exists between
// the selectedActivityDirectiveId and selectedSpanId stores and the dispatching of this event
// since there is no guarantee that the mousedown event triggering updates to those stores will complete
// before the context menu event dispatch fires
const { offsetX, offsetY } = e;
const { activityDirectives, spans } = getDirectivesAndSpansForOffset(offsetX, offsetY);
let newSelectedActivityDirectiveId = null;
let newSelectedSpanId = null;
if (activityDirectives.length > 0) {
newSelectedActivityDirectiveId = activityDirectives[0].id;
} else if (spans.length > 0) {
newSelectedSpanId = spans[0].id;
}
dispatch('contextMenu', {
e,
layerId: id,
origin: 'layer-activity',
selectedActivityDirectiveId: newSelectedActivityDirectiveId,
selectedSpanId: newSelectedSpanId,
});
}
}
Expand Down

0 comments on commit 2900086

Please sign in to comment.