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
12 changes: 12 additions & 0 deletions .changeset/shiny-webs-write.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"@knocklabs/client": patch
"@knocklabs/react": patch
---

[Guides] Add additional debug settings to guide client, and improvements to the guide toolbar V2

- Add `focusedGuideKeys` debug setting to pin a target guide during preview
- Add `ignoreDisplayInterval` debug setting to ignore throttling during preview
- Add `skipEngagementTracking` debug setting to skip sending engagement updates to the API during preview
- Add control UIs to the guide toolbar V2 for the newly added debug settings
- Add a drag handle to the guide toolbar for drag and drop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,19 @@ import "./styles.css";

type Props = {
onClick: () => void;
positioned?: boolean;
};

export const KnockButton = ({ onClick }: Props) => {
export const KnockButton = ({ onClick, positioned = true }: Props) => {
return (
<Button
onClick={onClick}
position="fixed"
top="4"
right="4"
{...(positioned && {
position: "fixed" as const,
top: "4" as const,
right: "4" as const,
style: { zIndex: TOOLBAR_Z_INDEX },
})}
bg="surface-2"
shadow="3"
rounded="3"
Expand All @@ -22,7 +26,6 @@ export const KnockButton = ({ onClick }: Props) => {
variant="soft"
data-tgph-appearance="dark"
aria-label="Expand guide toolbar"
style={{ zIndex: TOOLBAR_Z_INDEX }}
>
<svg
width="40"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { Icon } from "@telegraph/icon";
import { Box } from "@telegraph/layout";
import { GripVertical } from "lucide-react";
import React from "react";

// How far the drag handle protrudes beyond the toolbar's right edge (px)
export const DRAG_HANDLE_OVERHANG = 16;

type DragHandleProps = {
onPointerDown: (e: React.PointerEvent) => void;
isDragging: boolean;
};

export const DragHandle = ({ onPointerDown, isDragging }: DragHandleProps) => {
return (
<Box
data-tgph-appearance="dark"
onPointerDown={onPointerDown}
borderRadius="2"
position="absolute"
style={{
top: "9px",
right: `-${DRAG_HANDLE_OVERHANG}px`,
height: "24px",
cursor: isDragging ? "grabbing" : "grab",
touchAction: "none",
userSelect: "none",
}}
>
<Icon color="gray" size="1" icon={GripVertical} aria-hidden />
</Box>
);
};
23 changes: 21 additions & 2 deletions packages/react/src/modules/guide/components/Toolbar/V2/V2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ import { KnockButton } from "../KnockButton";
import { TOOLBAR_Z_INDEX } from "../shared";
import "../styles.css";

import { DRAG_HANDLE_OVERHANG, DragHandle } from "./DragHandle";
import { GuideContextDetails } from "./GuideContextDetails";
import { GuideRow } from "./GuideRow";
import {
DisplayOption,
GuidesListDisplaySelect,
} from "./GuidesListDisplaySelect";
import { detectToolbarParam } from "./helpers";
import { useDraggable } from "./useDraggable";
import {
InspectionResult,
useInspectGuideClientStore,
Expand Down Expand Up @@ -68,15 +70,32 @@ export const V2 = () => {
};
}, [isVisible, client]);

const containerRef = React.useRef<HTMLDivElement>(null);
const { position, isDragging, handlePointerDown } = useDraggable({
elementRef: containerRef,
reclampDeps: [isCollapsed],
rightPadding: DRAG_HANDLE_OVERHANG,
initialPosition: { top: 16, right: 16 },
});

const result = useInspectGuideClientStore();
if (!result) {
return null;
}

return (
<Box position="fixed" top="4" right="4" style={{ zIndex: TOOLBAR_Z_INDEX }}>
<Box
tgphRef={containerRef}
position="fixed"
style={{
top: position.top + "px",
right: position.right + "px",
zIndex: TOOLBAR_Z_INDEX,
}}
>
<DragHandle onPointerDown={handlePointerDown} isDragging={isDragging} />
{isCollapsed ? (
<KnockButton onClick={() => setIsCollapsed(false)} />
<KnockButton onClick={() => setIsCollapsed(false)} positioned={false} />
) : (
<Stack
direction="column"
Expand Down
159 changes: 159 additions & 0 deletions packages/react/src/modules/guide/components/Toolbar/V2/useDraggable.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
import React from "react";

// NOTE: This hook was generated entirely with Claude then lightly touched up,
// and the behavior works correctly from testing it manually in the browser.

type Position = { top: number; right: number };

type UseDraggableOptions = {
elementRef: React.RefObject<HTMLElement | null>;
initialPosition?: Position;
reclampDeps?: React.DependencyList;
/** Extra space to reserve beyond the element's right edge (px). */
rightPadding?: number;
};

type UseDraggableReturn = {
position: Position;
isDragging: boolean;
handlePointerDown: (e: React.PointerEvent) => void;
};

const DEFAULT_POSITION: Position = { top: 16, right: 16 };

/**
* @param rightPadding Extra space to reserve on the right edge (e.g. for a
* drag handle that protrudes beyond the element). The element's left edge
* is clamped so that `elementWidth + rightPadding` stays within the viewport.
*/
export function clampPosition(
pos: Position,
elementWidth: number,
elementHeight: number,
rightPadding = 0,
): Position {
const viewportWidth = window.innerWidth;
const viewportHeight = window.innerHeight;

const totalWidth = elementWidth + rightPadding;
const left = viewportWidth - pos.right - elementWidth;
const clampedLeft = Math.max(0, Math.min(left, viewportWidth - totalWidth));
const clampedTop = Math.max(
0,
Math.min(pos.top, viewportHeight - elementHeight),
);
const clampedRight = viewportWidth - clampedLeft - elementWidth;

return { top: clampedTop, right: clampedRight };
}

export function useDraggable({
elementRef,
initialPosition = DEFAULT_POSITION,
reclampDeps = [],
rightPadding = 0,
}: UseDraggableOptions): UseDraggableReturn {
const [position, setPosition] = React.useState<Position>(initialPosition);
const [isDragging, setIsDragging] = React.useState(false);

const positionRef = React.useRef(position);
positionRef.current = position;

const startPointerRef = React.useRef({ x: 0, y: 0 });
const startPositionRef = React.useRef<Position>({ top: 0, right: 0 });
const rafIdRef = React.useRef<number | null>(null);
const isDraggingRef = React.useRef(false);
const cleanupListenersRef = React.useRef<(() => void) | null>(null);

const reclamp = React.useCallback(() => {
const el = elementRef.current;
if (!el) return;
const rect = el.getBoundingClientRect();
setPosition((prev) =>
clampPosition(prev, rect.width, rect.height, rightPadding),
);
}, [elementRef, rightPadding]);

// Stable pointerdown handler
const handlePointerDown = React.useCallback(
(e: React.PointerEvent) => {
e.preventDefault();
startPointerRef.current = { x: e.clientX, y: e.clientY };
startPositionRef.current = { ...positionRef.current };
isDraggingRef.current = true;
setIsDragging(true);

const onPointerMove = (moveEvent: PointerEvent) => {
if (!isDraggingRef.current) return;

if (rafIdRef.current !== null) return;

rafIdRef.current = requestAnimationFrame(() => {
rafIdRef.current = null;
if (!isDraggingRef.current) return;

const dx = moveEvent.clientX - startPointerRef.current.x;
const dy = moveEvent.clientY - startPointerRef.current.y;

const newPos: Position = {
top: startPositionRef.current.top + dy,
right: startPositionRef.current.right - dx,
};

const el = elementRef.current;
if (!el) return;
const rect = el.getBoundingClientRect();
const clamped = clampPosition(
newPos,
rect.width,
rect.height,
rightPadding,
);
setPosition(clamped);
});
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RAF throttle uses stale event, losing final drag position

Low Severity

The RAF throttle in onPointerMove drops intermediate events entirely and captures the stale moveEvent from the closure of the event that triggered the schedule, rather than the most recent event. When a new pointermove arrives while a RAF is pending, it's discarded (return), so the RAF callback processes an outdated position. Combined with cleanup cancelling the pending RAF on pointerup without applying a final position update, this can cause the toolbar to visibly snap back by several pixels on release during fast drags. The standard pattern stores the latest event coordinates in a mutable ref and reads them inside the RAF callback.

Additional Locations (1)

Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds benign enough, can come back if a real issue.


const cleanup = () => {
isDraggingRef.current = false;
setIsDragging(false);
if (rafIdRef.current !== null) {
cancelAnimationFrame(rafIdRef.current);
rafIdRef.current = null;
}
document.removeEventListener("pointermove", onPointerMove);
document.removeEventListener("pointerup", onPointerUp);
cleanupListenersRef.current = null;
};

const onPointerUp = () => cleanup();

document.addEventListener("pointermove", onPointerMove);
document.addEventListener("pointerup", onPointerUp);
cleanupListenersRef.current = cleanup;
},
[elementRef, rightPadding],
);

// Cleanup on unmount
React.useEffect(() => {
return () => {
cleanupListenersRef.current?.();
};
}, []);

// Re-clamp on window resize
React.useEffect(() => {
const onResize = () => reclamp();
window.addEventListener("resize", onResize);
return () => window.removeEventListener("resize", onResize);
}, [reclamp]);

// Re-clamp when deps change (e.g. collapse toggle)
React.useEffect(() => {
const id = requestAnimationFrame(() => reclamp());
return () => cancelAnimationFrame(id);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, reclampDeps);

return { position, isDragging, handlePointerDown };
}
Loading
Loading