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
89 changes: 89 additions & 0 deletions apps/app/src/components/ui/sidebar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,53 @@ function fireTouch(
fireEvent(target, event);
}

function firePointer(
target: Element | Document | Window,
type: "pointerdown" | "pointermove",
clientX: number,
clientY: number,
) {
const event = new Event(type, { bubbles: true, cancelable: true });
Object.defineProperties(event, {
pointerId: { value: 1 },
pointerType: { value: "touch" },
isPrimary: { value: true },
button: { value: 0 },
buttons: { value: 1 },
clientX: { value: clientX },
clientY: { value: clientY },
});
fireEvent(target, event);
}

function renderScrollerSwipeHarness() {
render(
<CompactViewportOverrideProvider isCompactViewport>
<SidebarProvider>
<Sidebar>Sidebar content</Sidebar>
<SidebarInset>
<div data-testid="scroller" style={{ overflowX: "auto" }}>
<div data-sidebar-swipe-selectable>Wide code block</div>
</div>
</SidebarInset>
</SidebarProvider>
</CompactViewportOverrideProvider>,
);
const scroller = screen.getByTestId("scroller");
let scrollWidthReads = 0;
Object.defineProperty(scroller, "scrollWidth", {
get: () => {
scrollWidthReads += 1;
return 500;
},
});
Object.defineProperty(scroller, "clientWidth", { get: () => 100 });
return {
prose: screen.getByText("Wide code block"),
getScrollWidthReads: () => scrollWidthReads,
};
}

function renderSelectableSwipeHarness() {
render(
<CompactViewportOverrideProvider isCompactViewport>
Expand Down Expand Up @@ -97,6 +144,48 @@ describe("mobile sidebar text-selection arbitration", () => {
expect(document.querySelector('[data-sidebar="panel"]')).not.toBeNull();
});

it("defers the horizontal-scroll-region probe until horizontal intent", () => {
const { prose, getScrollWidthReads } = renderScrollerSwipeHarness();

fireTouch(prose, "touchstart", createTouch(120, 160));

// The tap path must stay free of forced layout reads (#1269).
expect(getScrollWidthReads()).toBe(0);

fireTouch(window, "touchmove", createTouch(260, 164));
fireTouch(window, "touchmove", createTouch(280, 164));

// Exactly one probe per gesture, then the swipe cancels.
expect(getScrollWidthReads()).toBe(1);
expect(document.querySelector('[data-sidebar="panel"]')).toBeNull();
});

it("defers the probe on the pointer path as well", () => {
const { prose, getScrollWidthReads } = renderScrollerSwipeHarness();

firePointer(prose, "pointerdown", 120, 160);

expect(getScrollWidthReads()).toBe(0);

firePointer(window, "pointermove", 260, 164);
firePointer(window, "pointermove", 280, 164);

expect(getScrollWidthReads()).toBe(1);
expect(document.querySelector('[data-sidebar="panel"]')).toBeNull();
});

it("cancels a swipe whose start target detached before the probe", () => {
const { prose, getScrollWidthReads } = renderScrollerSwipeHarness();

fireTouch(prose, "touchstart", createTouch(120, 160));
prose.remove();
fireTouch(window, "touchmove", createTouch(260, 164));

// A detached target reports empty computed style; never probe or open.
expect(getScrollWidthReads()).toBe(0);
expect(document.querySelector('[data-sidebar="panel"]')).toBeNull();
});

it("cancels a pending prose swipe when native text selection begins", () => {
let hasSelection = false;
let selectionNode: Node | null = null;
Expand Down
44 changes: 36 additions & 8 deletions apps/app/src/components/ui/sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ type SidebarInsetSwipeSession = {
velocityX: number;
isDragging: boolean;
selectionRoot: Element | null;
startTarget: Element | null;
};

const sidebarMobileWidthStyle: SidebarMobileWidthStyle = {
Expand Down Expand Up @@ -154,12 +155,14 @@ function createSidebarInsetSwipeSession({
startX,
startY,
selectionRoot,
startTarget,
}: {
kind: "pointer" | "touch";
id: number;
startX: number;
startY: number;
selectionRoot: Element | null;
startTarget: Element | null;
}): SidebarInsetSwipeSession {
const nowMs = Date.now();
return {
Expand All @@ -174,6 +177,7 @@ function createSidebarInsetSwipeSession({
velocityX: 0,
isDragging: false,
selectionRoot,
startTarget,
};
}

Expand Down Expand Up @@ -205,6 +209,13 @@ function isHorizontallyScrollableElement(element: Element): boolean {
return element.scrollWidth > element.clientWidth + 1;
}

/**
* Each ancestor probe pairs `getComputedStyle` with a `scrollWidth` read, so a
* call forces a synchronous style + layout pass of the document. Never call
* this from a per-tap listener (`pointerdown`/`touchstart`): on a large
* timeline that flush can block a mobile main thread for seconds (#1269).
* Callers must defer it until a gesture shows real horizontal intent.
*/
function isInsideHorizontalScrollRegion(target: Element): boolean {
let element: Element | null = target;
while (element !== null) {
Expand Down Expand Up @@ -267,14 +278,9 @@ function shouldIgnoreSidebarSwipeTarget(target: EventTarget | null): boolean {
}

const selectionRoot = getSidebarSwipeSelectionRoot(target);
if (
selectionRoot !== null &&
hasExpandedTextSelectionWithin(selectionRoot)
) {
return true;
}

return isInsideHorizontalScrollRegion(target);
return (
selectionRoot !== null && hasExpandedTextSelectionWithin(selectionRoot)
);
}

function isSidebarInsetSwipeTarget(target: EventTarget | null): boolean {
Expand Down Expand Up @@ -880,6 +886,19 @@ const SidebarInset = React.forwardRef<
return;
}

// A live timeline update can detach the start target mid-gesture. A
// detached element reports empty computed style, so the probe below
// would wrongly pass; cancel the swipe instead of guessing.
if (
session.startTarget !== null &&
(!session.startTarget.isConnected ||
isInsideHorizontalScrollRegion(session.startTarget))
) {
clearSidebarMobileDragStyles();
clearSwipeSession();
return;
}

session.isDragging = true;
clearMobileDragSettleTimeout();
flushSync(() => {
Expand Down Expand Up @@ -1035,6 +1054,7 @@ const SidebarInset = React.forwardRef<
startX: touch.clientX,
startY: touch.clientY,
selectionRoot: getSidebarSwipeSelectionRoot(event.target),
startTarget: event.target instanceof Element ? event.target : null,
});

const removeListeners = () => {
Expand Down Expand Up @@ -1080,6 +1100,7 @@ const SidebarInset = React.forwardRef<
startX: event.clientX,
startY: event.clientY,
selectionRoot: getSidebarSwipeSelectionRoot(event.target),
startTarget: event.target instanceof Element ? event.target : null,
});

const removeListeners = () => {
Expand Down Expand Up @@ -1154,6 +1175,13 @@ const SidebarInset = React.forwardRef<
return;
}

if (
event.target instanceof Element &&
isInsideHorizontalScrollRegion(event.target)
) {
return;
}

if (event.cancelable) {
event.preventDefault();
}
Expand Down
Loading