fix(studio): dedupe repeated selection telemetry (#3498)

This commit is contained in:
James Russo
2026-08-25 22:58:37 -07:00
committed by GitHub
parent dae5b7b90b
commit 9aaa7552fc
5 changed files with 214 additions and 87 deletions
@@ -208,6 +208,54 @@ describe("useDomSelection additive", () => {
});
describe("useDomSelection", () => {
it("ignores a repeated non-additive selection of the same target", () => {
const element = document.createElement("div");
element.id = "headline";
const first = makeSelection("Headline", element);
const repeated = makeSelection("Headline", element);
const harness = renderHarness({
activeCompPath: "intro.html",
projectId: "project-1",
refreshKey: 0,
});
act(() => harness.current().applyDomSelection(first));
harness.timeline.setSelectedTimelineElementId.mockClear();
harness.timeline.setTimelineSelectionSet.mockClear();
act(() => harness.current().applyDomSelection(repeated));
expect(harness.current().domEditSelection).toBe(first);
expect(harness.current().domEditGroupSelections).toEqual([first]);
expect(harness.timeline.setSelectedTimelineElementId).not.toHaveBeenCalled();
expect(harness.timeline.setTimelineSelectionSet).not.toHaveBeenCalled();
harness.cleanup();
});
it("still refreshes selection data for the same target when preserving the group", () => {
const element = document.createElement("div");
element.id = "headline";
const first = makeSelection("Headline", element);
const refreshed = makeSelection("Updated headline", element);
const harness = renderHarness({
activeCompPath: "intro.html",
projectId: "project-1",
refreshKey: 0,
});
act(() => harness.current().applyDomSelection(first));
act(() =>
harness.current().applyDomSelection(refreshed, {
preserveGroup: true,
revealPanel: false,
}),
);
expect(harness.current().domEditSelection).toBe(refreshed);
expect(harness.current().domEditGroupSelections).toEqual([refreshed]);
harness.cleanup();
});
it("clears a committed selection when the active composition path changes", () => {
const { selection, harness } = setupSelectedHarness();
expect(harness.current().domEditSelection).toBe(selection);
+25 -81
View File
@@ -1,10 +1,9 @@
import { useState, useCallback, useRef, useEffect } from "react";
import type { SelectElementOptions, TimelineElement } from "../player";
import type { TimelineElement } from "../player";
import {
getAllPreviewTargetsFromPointer,
getPreviewTargetFromPointer,
} from "../utils/studioPreviewHelpers";
import { type RightPanelTab } from "../utils/studioHelpers";
import {
domEditSelectionsTargetSame,
domEditSelectionInGroup,
@@ -22,86 +21,18 @@ import { reapplyPositionEditsAfterSeek } from "../components/editor/manualEdits"
import { useStudioTestHooks } from "./useStudioTestHooks";
import { logSelect } from "../utils/selectDebug";
import { announceTimelineSelection as announceSelectionToTimeline } from "./domSelectionTimelineMirror";
import type {
ApplyDomSelectionOptions,
UseDomSelectionParams,
UseDomSelectionReturn,
} from "./useDomSelectionTypes";
// ── Types ──
export interface ApplyDomSelectionOptions {
revealPanel?: boolean;
additive?: boolean;
preserveGroup?: boolean;
// A clear that came FROM the timeline must not be echoed back, or picking a
// clip with no canvas node would deselect the clip you just picked.
announce?: boolean;
}
export interface ResolveDomSelectionOptions {
preferClipAncestor?: boolean;
skipSourceProbe?: boolean;
activeGroupElement?: HTMLElement | null;
}
export interface UseDomSelectionParams {
projectId: string | null;
activeCompPath: string | null;
isMasterView: boolean;
compIdToSrc: Map<string, string>;
captionEditMode: boolean;
previewIframeRef: React.MutableRefObject<HTMLIFrameElement | null>;
timelineElements: TimelineElement[];
getTimelineSelectionSet: () => ReadonlySet<string>;
setSelectedTimelineElementId: (id: string | null, options?: SelectElementOptions) => void;
/** Publishes a whole multi-selection to the timeline; the anchor is set separately. */
setTimelineSelectionSet: (ids: Set<string>) => void;
setRightCollapsed: (collapsed: boolean) => void;
setRightPanelTab: (tab: RightPanelTab) => void;
previewIframe: HTMLIFrameElement | null;
refreshKey: number;
rightPanelTab: RightPanelTab;
}
export interface UseDomSelectionReturn {
// State
domEditSelection: DomEditSelection | null;
domEditGroupSelections: DomEditSelection[];
domEditHoverSelection: DomEditSelection | null;
activeGroupElement: HTMLElement | null;
// Refs
domEditSelectionRef: React.MutableRefObject<DomEditSelection | null>;
domEditGroupSelectionsRef: React.MutableRefObject<DomEditSelection[]>;
domEditHoverSelectionRef: React.MutableRefObject<DomEditSelection | null>;
activeGroupElementRef: React.MutableRefObject<HTMLElement | null>;
// State setters (needed by useDomEditSession for agent-prompt reset flows)
setDomEditSelection: React.Dispatch<React.SetStateAction<DomEditSelection | null>>;
setDomEditGroupSelections: React.Dispatch<React.SetStateAction<DomEditSelection[]>>;
setActiveGroupElement: (el: HTMLElement | null) => void;
// Callbacks
applyDomSelection: (
selection: DomEditSelection | null,
options?: ApplyDomSelectionOptions,
) => void;
clearDomSelection: () => void;
buildDomSelectionFromTarget: (
target: HTMLElement,
options?: ResolveDomSelectionOptions,
) => Promise<DomEditSelection | null>;
resolveDomSelectionFromPreviewPoint: (
clientX: number,
clientY: number,
options?: ResolveDomSelectionOptions,
) => Promise<DomEditSelection | null>;
resolveAllDomSelectionsFromPreviewPoint: (
clientX: number,
clientY: number,
) => Promise<DomEditSelection[]>;
updateDomEditHoverSelection: (selection: DomEditSelection | null) => void;
buildDomSelectionForTimelineElement: (
element: TimelineElement,
) => Promise<DomEditSelection | null>;
handleTimelineElementSelect: (element: TimelineElement | null) => Promise<void>;
refreshDomEditSelectionFromPreview: (selection: DomEditSelection) => Promise<void>;
refreshDomEditGroupSelectionsFromPreview: (selections: DomEditSelection[]) => Promise<void>;
applyMarqueeSelection: (selections: DomEditSelection[], additive: boolean) => void;
}
export type {
ApplyDomSelectionOptions,
ResolveDomSelectionOptions,
UseDomSelectionParams,
UseDomSelectionReturn,
} from "./useDomSelectionTypes";
// ── Hook ──
@@ -187,6 +118,19 @@ export function useDomSelection({
const isAdditiveSelection = Boolean(options?.additive);
const currentSelection = domEditSelectionRef.current;
const previousGroup = domEditGroupSelectionsRef.current;
const isRepeatedSingleSelection =
!isAdditiveSelection &&
!options?.preserveGroup &&
previousGroup.length === 1 &&
domEditSelectionsTargetSame(currentSelection, selection) &&
domEditSelectionsTargetSame(previousGroup[0], selection);
if (isRepeatedSingleSelection) {
if (options?.revealPanel !== false) {
setRightCollapsed(false);
if (rightPanelTabRef.current !== "variables") setRightPanelTab("design");
}
return;
}
const currentGroup = isAdditiveSelection
? seedDomEditGroupWithSelection(previousGroup, currentSelection)
: previousGroup;
@@ -0,0 +1,82 @@
import type { Dispatch, MutableRefObject, SetStateAction } from "react";
import type { DomEditSelection } from "../components/editor/domEditing";
import type { SelectElementOptions, TimelineElement } from "../player";
import type { RightPanelTab } from "../utils/studioHelpers";
export interface ApplyDomSelectionOptions {
revealPanel?: boolean;
additive?: boolean;
preserveGroup?: boolean;
// A clear that came FROM the timeline must not be echoed back, or picking a
// clip with no canvas node would deselect the clip you just picked.
announce?: boolean;
}
export interface ResolveDomSelectionOptions {
preferClipAncestor?: boolean;
skipSourceProbe?: boolean;
activeGroupElement?: HTMLElement | null;
}
export interface UseDomSelectionParams {
projectId: string | null;
activeCompPath: string | null;
isMasterView: boolean;
compIdToSrc: Map<string, string>;
captionEditMode: boolean;
previewIframeRef: MutableRefObject<HTMLIFrameElement | null>;
timelineElements: TimelineElement[];
getTimelineSelectionSet: () => ReadonlySet<string>;
setSelectedTimelineElementId: (id: string | null, options?: SelectElementOptions) => void;
/** Publishes a whole multi-selection to the timeline; the anchor is set separately. */
setTimelineSelectionSet: (ids: Set<string>) => void;
setRightCollapsed: (collapsed: boolean) => void;
setRightPanelTab: (tab: RightPanelTab) => void;
previewIframe: HTMLIFrameElement | null;
refreshKey: number;
rightPanelTab: RightPanelTab;
}
export interface UseDomSelectionReturn {
// State
domEditSelection: DomEditSelection | null;
domEditGroupSelections: DomEditSelection[];
domEditHoverSelection: DomEditSelection | null;
activeGroupElement: HTMLElement | null;
// Refs
domEditSelectionRef: MutableRefObject<DomEditSelection | null>;
domEditGroupSelectionsRef: MutableRefObject<DomEditSelection[]>;
domEditHoverSelectionRef: MutableRefObject<DomEditSelection | null>;
activeGroupElementRef: MutableRefObject<HTMLElement | null>;
// State setters (needed by useDomEditSession for agent-prompt reset flows)
setDomEditSelection: Dispatch<SetStateAction<DomEditSelection | null>>;
setDomEditGroupSelections: Dispatch<SetStateAction<DomEditSelection[]>>;
setActiveGroupElement: (el: HTMLElement | null) => void;
// Callbacks
applyDomSelection: (
selection: DomEditSelection | null,
options?: ApplyDomSelectionOptions,
) => void;
clearDomSelection: () => void;
buildDomSelectionFromTarget: (
target: HTMLElement,
options?: ResolveDomSelectionOptions,
) => Promise<DomEditSelection | null>;
resolveDomSelectionFromPreviewPoint: (
clientX: number,
clientY: number,
options?: ResolveDomSelectionOptions,
) => Promise<DomEditSelection | null>;
resolveAllDomSelectionsFromPreviewPoint: (
clientX: number,
clientY: number,
) => Promise<DomEditSelection[]>;
updateDomEditHoverSelection: (selection: DomEditSelection | null) => void;
buildDomSelectionForTimelineElement: (
element: TimelineElement,
) => Promise<DomEditSelection | null>;
handleTimelineElementSelect: (element: TimelineElement | null) => Promise<void>;
refreshDomEditSelectionFromPreview: (selection: DomEditSelection) => Promise<void>;
refreshDomEditGroupSelectionsFromPreview: (selections: DomEditSelection[]) => Promise<void>;
applyMarqueeSelection: (selections: DomEditSelection[], additive: boolean) => void;
}
@@ -4,11 +4,15 @@ import React, { act } from "react";
import { createRoot } from "react-dom/client";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { readStudioUiPreferences } from "../utils/studioUiPreferences";
import { trackStudioEvent } from "../utils/studioTelemetry";
import { usePanelLayout } from "./usePanelLayout";
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
vi.mock("../utils/studioTelemetry", () => ({ trackStudioEvent: vi.fn() }));
beforeEach(() => {
vi.mocked(trackStudioEvent).mockClear();
const entries = new Map<string, string>();
Object.defineProperty(window, "localStorage", {
configurable: true,
@@ -165,6 +169,29 @@ describe("usePanelLayout — right inspector panes", () => {
harness.unmount();
});
it("tracks only actual right-panel tab changes, including rapid repeated calls", () => {
const harness = renderPanelLayout();
act(() => {
harness.getState().setRightPanelTab("design");
harness.getState().setRightPanelTab("design");
});
expect(trackStudioEvent).not.toHaveBeenCalled();
act(() => {
harness.getState().setRightPanelTab("layers");
harness.getState().setRightPanelTab("layers");
});
expect(trackStudioEvent).toHaveBeenCalledOnce();
expect(trackStudioEvent).toHaveBeenCalledWith("tab_switch", {
panel: "right_panel",
tab: "layers",
});
expect(harness.getState().rightPanelTab).toBe("layers");
harness.unmount();
});
it("caps a panel relative to the window instead of at a flat 600px", () => {
resizeWindowTo(700);
const harness = renderPanelLayout();
@@ -289,6 +316,17 @@ describe("usePanelLayout — right inspector panes", () => {
const harness = renderPanelLayoutWith(usePanelLayoutFlatOn);
expect(harness.getState().rightInspectorPanes).toEqual({ layers: false, design: true });
// The flat inspector can change its visible pane without changing the
// umbrella rightPanelTab value. A later element selection must still bring
// Design back even though rightPanelTab already says "design".
act(() => harness.getState().setExclusiveRightInspectorPane("layers"));
expect(harness.getState()).toMatchObject({
rightPanelTab: "design",
rightInspectorPanes: { layers: true, design: false },
});
act(() => harness.getState().setRightPanelTab("design"));
expect(harness.getState().rightInspectorPanes).toEqual({ layers: false, design: true });
// Element-select / block-params-close / header Inspector-button callers
// all reach setRightPanelTab directly, not through the in-panel tab
// click's own setExclusiveRightInspectorPane call — this must still
+21 -6
View File
@@ -56,9 +56,13 @@ export function usePanelLayout(initialState?: InitialPanelLayoutState) {
const [rightPanelTab, setRightPanelTab] = useState<RightPanelTab>(
initialState?.rightPanelTab ?? "design",
);
const rightPanelTabRef = useRef(rightPanelTab);
rightPanelTabRef.current = rightPanelTab;
const [rightInspectorPanes, setRightInspectorPanes] = useState<RightInspectorPanes>(() =>
getInitialRightInspectorPanes(initialState?.rightPanelTab),
);
const rightInspectorPanesRef = useRef(rightInspectorPanes);
rightInspectorPanesRef.current = rightInspectorPanes;
// Set when the user explicitly reopens a panel the window had auto-collapsed,
// so the rail cannot immediately swallow it again. Cleared once the window is
// wide enough that auto-collapse is no longer in play.
@@ -161,7 +165,9 @@ export function usePanelLayout(initialState?: InitialPanelLayoutState) {
const setRightCollapsedWithOverride = useCallback((collapsed: boolean) => {
setRightCollapsed(collapsed);
if (!collapsed) setAutoCollapseOverride((prev) => ({ ...prev, right: true }));
if (!collapsed) {
setAutoCollapseOverride((prev) => (prev.right ? prev : { ...prev, right: true }));
}
}, []);
const handlePanelResizeStart = useCallback((side: PanelSide, e: React.PointerEvent) => {
@@ -192,6 +198,15 @@ export function usePanelLayout(initialState?: InitialPanelLayoutState) {
const trackedSetRightPanelTab = useCallback(
(tab: RightPanelTab) => {
const paneAlreadySelected =
tab !== "design" && tab !== "layers"
? true
: STUDIO_FLAT_INSPECTOR_ENABLED
? rightInspectorPanesRef.current[tab] &&
!rightInspectorPanesRef.current[tab === "design" ? "layers" : "design"]
: rightInspectorPanesRef.current[tab];
if (rightPanelTabRef.current === tab && paneAlreadySelected) return;
rightPanelTabRef.current = tab;
if (tab === "design" || tab === "layers") {
// Flat inspector: Layers always renders full-height by itself (see
// StudioRightPanel's render gate), so this MUST land on the same
@@ -202,11 +217,11 @@ export function usePanelLayout(initialState?: InitialPanelLayoutState) {
// inspector tab) would otherwise additively leave both panes `true`
// and reproduce the "both tabs highlight, only one renders" bug this
// still-additive branch used to cause under the flat flag.
setRightInspectorPanes(
STUDIO_FLAT_INSPECTOR_ENABLED
? { design: tab === "design", layers: tab === "layers" }
: (panes) => ({ ...panes, [tab]: true }),
);
const nextPanes = STUDIO_FLAT_INSPECTOR_ENABLED
? { design: tab === "design", layers: tab === "layers" }
: { ...rightInspectorPanesRef.current, [tab]: true };
rightInspectorPanesRef.current = nextPanes;
setRightInspectorPanes(nextPanes);
}
setRightPanelTab(tab);
trackStudioEvent("tab_switch", { panel: "right_panel", tab });