Skip to content

Commit 721ccd0

Browse files
authored
fix(pdf-server): translucent highlights, pinch to/from fullscreen, toolbar layout (#587)
* fix(pdf-server): translucent highlights, pinch to/from fullscreen, toolbar layout Highlights rendered opaque: inline `background: <hex>` overrode the CSS rgba(...,0.35). Now convert def.color via cssColorToRgb and force alpha 0.35. Added a toHaveCSS regression check in pdf-annotations.spec.ts. Pinch-in while inline (wheel ctrlKey deltaY<0 or two-finger spread >1.15x) now enters fullscreen. Pinch-out in fullscreen past 0.9x of fit-scale, when already at/below fit, exits to inline and clears userHasZoomed so refit sizes the inline view. previewScaleRaw tracks the unclamped intent so the exit fires even when fit ~= ZOOM_MIN. A modeTransitionInFlight latch (held 250ms post-toggle) keeps the gesture tail from re-toggling or immediately zooming the new view. Fullscreen toolbar: flex-wrap: nowrap and tighter 0.25rem vertical padding (min-height 40px + safe-top instead of 48px + wrap). Search bar top/right now follow --safe-top/--safe-right so it sits flush below the toolbar instead of overlapping it. Base .canvas-container gets touch-action: pan-x pan-y so the inline pinch is capturable on iOS. * fix(pdf-server): drop --safe-top from fullscreen toolbar padding Hosts wrap fullscreen in their own header (title + close), which already clears the top safe-area. Adding --safe-top to our toolbar padding-top double-dipped, leaving a visible gap between the host header and our toolbar. Keep --safe-left/right (host header doesn't cover the sides). * feat(pdf-server): horizontal swipe changes pages whenever page fits width Previously gated on scale <= 1.0, which blocked page-nav in fullscreen where fit-scale is often >100%. Now gate on actual horizontal overflow (scrollWidth > clientWidth) so swipe works at any fit-to-width scale and still defers to native panning once you zoom past it. * fix(pdf-server): refit to inline width after pinch-out exits fullscreen handleHostContextChanged calls refitScale() before the iframe has actually shrunk, and the ResizeObserver's inline branch only refits on width *growth* (to avoid a requestFitToContent shrink-loop). So the fullscreen->inline shrink never triggered a refit and the page stayed at the fullscreen scale. Add a one-shot forceNextResizeRefit flag, set on fullscreen->inline (both the pinch-out path and handleHostContextChanged), consumed by the ResizeObserver on the next size change. One-shot keeps the shrink-loop guard intact for ordinary inline resizes. * feat(pdf-server): floor fullscreen zoom at fit-to-page In fullscreen, pinch-out and the zoom-out button now floor at the fit-to-page scale instead of ZOOM_MIN, so the page never shrinks below fully-visible (no dead margin around it). previewScaleRaw stays unclamped so a continued pinch-out past fit still triggers exit-to-inline. * fix(pdf-server): wheel pinch-out can exit fullscreen again The fit-floor pinned previewScale at fit, but the wheel handler multiplied the *clamped* previewScale, so it could never accumulate below fit*0.9 to trigger exit. Drive the wheel accumulator off previewScaleRaw instead, and bound previewScaleRaw to [floor*0.7, ZOOM_MAX] so it can cross the 0.9 exit threshold without drifting unboundedly (which would make direction reversal feel sticky). Touch path was unaffected (absolute ratio). * fix(pdf-server): always refit to inline width on fullscreen exit userHasZoomed stayed true after Escape/button/host-× exits, so refitScale() bailed even though forceNextResizeRefit let the ResizeObserver call it. Clear userHasZoomed in handleHostContextChanged whenever we land inline — fullscreen zoom level is meaningless there. Dropped the now-dead requestFitToContent fallback in the same block. * refactor(pdf-server): rubber-band pinch-out instead of dual-tracked clamp The previewScaleRaw side-channel made the gesture feel dead (page pinned at fit, no feedback) and the wheel accumulator interaction was fragile. New model: previewScale is the only tracked value. In fullscreen it may overshoot down to 0.75*fit so the user *sees* the page pull away as they pinch out. On commit: - started near fit (<=1.05*fit) AND preview <0.9*fit -> exit to inline - otherwise clamp committed scale to >=fit (overshoot snaps back) beginPinch seeds fitScaleAtPinchStart synchronously from when !userHasZoomed (the common enter-fullscreen-at-fit case) so the first frame already has the right floor; the async computeFitScale refines it. * test(pdf-server): update inline-pinch e2e for new fullscreen behavior 'trackpad pinch is ignored outside fullscreen' is no longer the contract. Replace with two tests: pinch-in inline -> .main.fullscreen appears; pinch-out inline -> zoom unchanged, no fullscreen. The two pdf-annotations.spec.ts failures in the previous run were flaky (passed on retry).
1 parent 9da4a75 commit 721ccd0

File tree

4 files changed

+186
-30
lines changed

4 files changed

+186
-30
lines changed

examples/pdf-server/src/mcp-app.css

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,10 @@ body {
227227
align-items: flex-start;
228228
padding: 1rem;
229229
background: var(--bg200);
230+
/* JS owns pinch (inline pinch-in → fullscreen). pan-x/pan-y keeps native
231+
* scrolling; the fullscreen rule below repeats this with a comment on the
232+
* iOS-Safari preventDefault backstop. */
233+
touch-action: pan-x pan-y;
230234
}
231235

232236
.page-wrapper {
@@ -310,9 +314,20 @@ body {
310314
}
311315

312316
.main.fullscreen .toolbar {
313-
padding-top: calc(0.5rem + var(--safe-top, 0px));
314-
padding-left: calc(0.5rem + var(--safe-left, 0px));
315-
padding-right: calc(0.5rem + var(--safe-right, 0px));
317+
/* Hosts wrap fullscreen in their own header (title + close), which already
318+
* clears the top safe-area — so no --safe-top here, only sides. */
319+
padding: 0.25rem calc(0.5rem + var(--safe-right, 0px)) 0.25rem
320+
calc(0.5rem + var(--safe-left, 0px));
321+
min-height: 40px;
322+
/* Inline can wrap (narrow chat bubble); fullscreen has the width, and
323+
* wrapping would double the bar height + desync the search-bar offset. */
324+
flex-wrap: nowrap;
325+
}
326+
327+
.main.fullscreen .search-bar {
328+
/* -1px overlaps the toolbar border so the dropdown looks attached. */
329+
top: 39px;
330+
right: calc(var(--safe-right, 0px) - 1px);
316331
}
317332

318333
.main.fullscreen .viewer {

examples/pdf-server/src/mcp-app.ts

Lines changed: 134 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
type ImageAnnotation,
2828
type NoteAnnotation,
2929
type FreetextAnnotation,
30+
cssColorToRgb,
3031
serializeDiff,
3132
deserializeDiff,
3233
mergeAnnotations,
@@ -341,14 +342,24 @@ async function refitScale(): Promise<void> {
341342
// needs height changes too (rotation, browser chrome on mobile).
342343
let lastContainerW = 0;
343344
let lastContainerH = 0;
345+
/** One-shot: refit on the next resize even if it's a shrink in inline mode.
346+
* Set on fullscreen→inline so the page snaps to the new (smaller) width
347+
* once the host has actually resized the iframe — the inline `grewW` gate
348+
* would otherwise swallow that shrink. */
349+
let forceNextResizeRefit = false;
344350
const containerResizeObserver = new ResizeObserver(([entry]) => {
345351
const { width: w, height: h } = entry.contentRect;
346352
const grewW = w > lastContainerW + 1;
347353
const changed =
348354
Math.abs(w - lastContainerW) > 1 || Math.abs(h - lastContainerH) > 1;
349355
lastContainerW = w;
350356
lastContainerH = h;
351-
if (currentDisplayMode === "fullscreen" ? changed : grewW) refitScale();
357+
if (forceNextResizeRefit && changed) {
358+
forceNextResizeRefit = false;
359+
refitScale();
360+
} else if (currentDisplayMode === "fullscreen" ? changed : grewW) {
361+
refitScale();
362+
}
352363
});
353364
containerResizeObserver.observe(canvasContainerEl as HTMLElement);
354365

@@ -1933,13 +1944,20 @@ function renderAnnotation(
19331944
viewport: { width: number; height: number; scale: number },
19341945
): HTMLElement[] {
19351946
switch (def.type) {
1936-
case "highlight":
1947+
case "highlight": {
1948+
// Force translucency: def.color is an opaque hex (e.g. "#ffff00"), which
1949+
// would override the rgba()/mix-blend-mode in CSS and hide the text.
1950+
const rgb = def.color ? cssColorToRgb(def.color) : null;
1951+
const bg = rgb
1952+
? `rgba(${Math.round(rgb.r * 255)}, ${Math.round(rgb.g * 255)}, ${Math.round(rgb.b * 255)}, 0.35)`
1953+
: undefined;
19371954
return renderRectsAnnotation(
19381955
def.rects,
19391956
"annotation-highlight",
19401957
viewport,
1941-
def.color ? { background: def.color } : {},
1958+
bg ? { background: bg } : {},
19421959
);
1960+
}
19431961
case "underline":
19441962
return renderRectsAnnotation(
19451963
def.rects,
@@ -3322,9 +3340,13 @@ function zoomIn() {
33223340
renderPage().then(scrollSelectionIntoView);
33233341
}
33243342

3325-
function zoomOut() {
3343+
async function zoomOut() {
33263344
userHasZoomed = true;
3327-
scale = Math.max(scale - 0.25, ZOOM_MIN);
3345+
// Fullscreen floor is fit-to-page (anything smaller is dead margin).
3346+
const fit =
3347+
currentDisplayMode === "fullscreen" ? await computeFitScale() : null;
3348+
const floor = fit !== null ? Math.max(ZOOM_MIN, fit) : ZOOM_MIN;
3349+
scale = Math.max(scale - 0.25, floor);
33283350
renderPage().then(scrollSelectionIntoView);
33293351
}
33303352

@@ -3761,32 +3783,77 @@ let pinchStartScale = 1.0;
37613783
let previewScale = 1.0;
37623784
/** Debounce timer — wheel events have no end event, so we wait for quiet. */
37633785
let pinchSettleTimer: ReturnType<typeof setTimeout> | null = null;
3786+
/** computeFitScale() snapshot at gesture start (async — may be null briefly). */
3787+
let fitScaleAtPinchStart: number | null = null;
3788+
/** Guards against firing toggleFullscreen() once per wheel event during a
3789+
* single inline pinch-in gesture. */
3790+
let modeTransitionInFlight = false;
37643791

37653792
function beginPinch() {
37663793
pinchStartScale = scale;
37673794
previewScale = scale;
3795+
// Seed synchronously when we can (at fit ⇔ !userHasZoomed) so the very
3796+
// first updatePinch already has the right floor — avoids a one-frame
3797+
// jitter when the async computeFitScale resolves mid-gesture.
3798+
fitScaleAtPinchStart = userHasZoomed ? null : scale;
3799+
void computeFitScale().then((s) => (fitScaleAtPinchStart = s));
37683800
// transform-origin matches the flex layout's anchor (justify-content:
37693801
// center, align-items: flex-start) so the preview and the committed
37703802
// canvas grow from the same point — otherwise the page jumps on release.
37713803
pageWrapperEl.style.transformOrigin = "50% 0";
37723804
}
37733805

3806+
/** Fit-to-page floor for fullscreen (committed scale never goes below this).
3807+
* The preview is allowed to overshoot down to 0.75×fit for rubber-band
3808+
* feedback; release below 0.9×fit exits to inline, otherwise snaps to fit. */
3809+
function pinchFitFloor(): number | null {
3810+
return currentDisplayMode === "fullscreen" ? fitScaleAtPinchStart : null;
3811+
}
3812+
37743813
function updatePinch(nextScale: number) {
3775-
previewScale = Math.min(ZOOM_MAX, Math.max(ZOOM_MIN, nextScale));
3814+
const fit = pinchFitFloor();
3815+
// Rubber-band: preview may dip to 0.75×fit so the user sees the page pull
3816+
// away as they pinch out. Committed scale is clamped to fit in commitPinch.
3817+
const previewFloor = fit !== null ? fit * 0.75 : ZOOM_MIN;
3818+
previewScale = Math.min(ZOOM_MAX, Math.max(previewFloor, nextScale));
37763819
// Transform is RELATIVE to the rendered canvas (which sits at
37773820
// pinchStartScale), so a previewScale equal to pinchStartScale → ratio 1.
37783821
pageWrapperEl.style.transform = `scale(${previewScale / pinchStartScale})`;
37793822
zoomLevelEl.textContent = `${Math.round(previewScale * 100)}%`;
37803823
}
37813824

37823825
function commitPinch() {
3783-
if (Math.abs(previewScale - scale) < 0.01) {
3784-
// Dead-zone — no re-render. Clear here since renderPage won't run.
3826+
const fit = pinchFitFloor();
3827+
// Pinched out past fit (page visibly pulled away) → exit fullscreen.
3828+
// Only when the gesture *started* near fit, so a single big pinch-out
3829+
// from deep zoom lands at fit instead of ejecting unexpectedly.
3830+
if (
3831+
fit !== null &&
3832+
pinchStartScale <= fit * 1.05 &&
3833+
previewScale < fit * 0.9
3834+
) {
37853835
pageWrapperEl.style.transform = "";
3836+
userHasZoomed = false; // let refitScale() size the inline view
3837+
forceNextResizeRefit = true; // ResizeObserver inline path ignores shrinks
3838+
modeTransitionInFlight = true;
3839+
void toggleFullscreen().finally(() => {
3840+
setTimeout(() => (modeTransitionInFlight = false), 250);
3841+
});
3842+
return;
3843+
}
3844+
// Committed scale never below fit in fullscreen — overshoot snaps back.
3845+
const target =
3846+
fit !== null
3847+
? Math.max(fit, previewScale)
3848+
: Math.max(ZOOM_MIN, previewScale);
3849+
if (Math.abs(target - scale) < 0.01) {
3850+
// Snap-back / dead-zone — no re-render needed.
3851+
pageWrapperEl.style.transform = "";
3852+
zoomLevelEl.textContent = `${Math.round(scale * 100)}%`;
37863853
return;
37873854
}
37883855
userHasZoomed = true;
3789-
scale = previewScale;
3856+
scale = target;
37903857
// renderPage clears the transform in the same frame as the canvas
37913858
// resize (after its first await) so there's no snap-back.
37923859
renderPage().then(scrollSelectionIntoView);
@@ -3804,8 +3871,23 @@ canvasContainerEl.addEventListener(
38043871
// Trackpad pinch arrives as wheel with ctrlKey set (Chrome/FF/Edge on
38053872
// macOS+Windows, Safari on macOS). MUST check before the deltaX/deltaY
38063873
// comparison below — pinch deltas come through on deltaY.
3807-
if (e.ctrlKey && currentDisplayMode === "fullscreen") {
3874+
if (e.ctrlKey) {
38083875
e.preventDefault();
3876+
if (currentDisplayMode !== "fullscreen") {
3877+
// Inline: pinch-in (deltaY<0) is a request to go fullscreen.
3878+
// Pinch-out is ignored — nothing smaller than inline.
3879+
if (e.deltaY < 0 && !modeTransitionInFlight) {
3880+
modeTransitionInFlight = true;
3881+
void toggleFullscreen().finally(() => {
3882+
// Hold the latch through the settle window so the tail of the
3883+
// gesture doesn't immediately start zooming the new fullscreen
3884+
// view (or, worse, re-toggle).
3885+
setTimeout(() => (modeTransitionInFlight = false), 250);
3886+
});
3887+
}
3888+
return;
3889+
}
3890+
if (modeTransitionInFlight) return; // swallow gesture tail post-toggle
38093891
if (pinchSettleTimer === null) beginPinch();
38103892
// exp(-deltaY * k) makes equal-magnitude in/out deltas inverse —
38113893
// pinch out then back lands where you started. Clamp per event so a
@@ -3826,10 +3908,15 @@ canvasContainerEl.addEventListener(
38263908
// Only intercept horizontal scroll, let vertical scroll through
38273909
if (Math.abs(e.deltaX) <= Math.abs(e.deltaY)) return;
38283910

3829-
// When zoomed, let natural panning happen (no page changes)
3830-
if (scale > 1.0) return;
3911+
// If the page overflows horizontally, let native panning handle it
3912+
// (no page changes). Checking actual overflow rather than `scale > 1.0`
3913+
// because fullscreen fit-scale is often >100% with the page still fully
3914+
// visible — we want swipe-to-page there. +1 absorbs sub-pixel rounding.
3915+
if (canvasContainerEl.scrollWidth > canvasContainerEl.clientWidth + 1) {
3916+
return;
3917+
}
38313918

3832-
// At 100% zoom, handle page navigation
3919+
// No horizontal overflow → swipe changes pages.
38333920
e.preventDefault();
38343921
horizontalScrollAccumulator += e.deltaX;
38353922
if (horizontalScrollAccumulator > SCROLL_THRESHOLD) {
@@ -3858,7 +3945,7 @@ canvasContainerEl.addEventListener(
38583945
"touchstart",
38593946
(event) => {
38603947
const e = event as TouchEvent;
3861-
if (e.touches.length !== 2 || currentDisplayMode !== "fullscreen") return;
3948+
if (e.touches.length !== 2) return;
38623949
// No preventDefault here — keep iOS Safari happy. We block native
38633950
// pinch-zoom via touch-action CSS + preventDefault on touchmove.
38643951
touchStartDist = touchDist(e.touches);
@@ -3873,7 +3960,21 @@ canvasContainerEl.addEventListener(
38733960
const e = event as TouchEvent;
38743961
if (e.touches.length !== 2 || touchStartDist === 0) return;
38753962
e.preventDefault(); // stop the browser zooming the whole viewport
3876-
updatePinch(pinchStartScale * (touchDist(e.touches) / touchStartDist));
3963+
const ratio = touchDist(e.touches) / touchStartDist;
3964+
if (currentDisplayMode !== "fullscreen") {
3965+
// Inline: a clear pinch-in means "go fullscreen". 1.15× threshold
3966+
// avoids triggering on jittery two-finger taps/scrolls.
3967+
if (ratio > 1.15 && !modeTransitionInFlight) {
3968+
modeTransitionInFlight = true;
3969+
touchStartDist = 0; // end this gesture; fullscreen will refit
3970+
pageWrapperEl.style.transform = "";
3971+
void toggleFullscreen().finally(() => {
3972+
setTimeout(() => (modeTransitionInFlight = false), 250);
3973+
});
3974+
}
3975+
return;
3976+
}
3977+
updatePinch(pinchStartScale * ratio);
38773978
},
38783979
{ passive: false },
38793980
);
@@ -3884,6 +3985,11 @@ canvasContainerEl.addEventListener("touchend", (event) => {
38843985
// REMAINING set — lifting one of two leaves length 1.
38853986
if (touchStartDist === 0 || e.touches.length >= 2) return;
38863987
touchStartDist = 0;
3988+
if (currentDisplayMode !== "fullscreen") {
3989+
// Inline pinch that didn't cross the threshold — discard preview.
3990+
pageWrapperEl.style.transform = "";
3991+
return;
3992+
}
38873993
commitPinch();
38883994
});
38893995

@@ -4707,16 +4813,20 @@ function handleHostContextChanged(ctx: McpUiHostContext) {
47074813
if (panelState.open) {
47084814
setAnnotationPanelOpen(true);
47094815
}
4816+
if (!isFullscreen) {
4817+
// Fullscreen zoom level is meaningless inline — always refit on exit,
4818+
// however it was triggered (pinch, button, host Escape/×).
4819+
userHasZoomed = false;
4820+
// The iframe shrink lands after this handler; let the ResizeObserver
4821+
// do one refit on that shrink (its inline branch normally ignores
4822+
// shrinks to avoid a requestFitToContent feedback loop).
4823+
forceNextResizeRefit = true;
4824+
}
47104825
if (wasFullscreen !== isFullscreen) {
4711-
// Mode changed → refit. computeFitScale reads displayMode, so
4712-
// this scales UP to fill on enter and back DOWN to ≤1.0 on exit.
4713-
// refitScale → renderPage → requestFitToContent handles the
4714-
// host-resize on exit. If userHasZoomed, refit no-ops; on exit fall
4715-
// back to requestFitToContent so the iframe still shrinks to whatever
4716-
// scale the user left it at.
4717-
void refitScale().then(() => {
4718-
if (!isFullscreen && userHasZoomed) requestFitToContent();
4719-
});
4826+
// Fast-path refit (computeFitScale reads displayMode). The iframe may
4827+
// not have its final size yet — the ResizeObserver one-shot above
4828+
// covers the inline-shrink case once it does.
4829+
void refitScale();
47204830
}
47214831
updateFullscreenButton();
47224832
}

tests/e2e/pdf-annotations.spec.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,12 @@ test.describe("PDF Server - Annotations", () => {
151151
// Check that a highlight annotation element was rendered
152152
const highlightEl = appFrame.locator(".annotation-highlight");
153153
await expect(highlightEl.first()).toBeVisible({ timeout: 5000 });
154+
// Regression: highlight must be translucent (not opaque hex), so text
155+
// underneath remains readable.
156+
await expect(highlightEl.first()).toHaveCSS(
157+
"background-color",
158+
/rgba\(255, 255, 0, 0\.35\)/,
159+
);
154160
});
155161

156162
test("add_annotations renders multiple annotation types", async ({

tests/e2e/pdf-viewer-zoom.spec.ts

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,13 +142,13 @@ test.describe("PDF Viewer — fullscreen fit + pinch zoom", () => {
142142
.toBe("");
143143
});
144144

145-
test("trackpad pinch is ignored outside fullscreen", async ({ page }) => {
145+
test("trackpad pinch-in while inline enters fullscreen", async ({ page }) => {
146146
await page.setViewportSize({ width: 1400, height: 800 });
147147
await loadPdfServer(page);
148148
await waitForPdfRender(page);
149149
const app = getAppFrame(page);
150150

151-
const before = await readZoomPercent(page);
151+
await expect(app.locator(".main.fullscreen")).toHaveCount(0);
152152

153153
await app.locator(".canvas-container").evaluate((el) => {
154154
el.dispatchEvent(
@@ -161,8 +161,33 @@ test.describe("PDF Viewer — fullscreen fit + pinch zoom", () => {
161161
);
162162
});
163163

164-
// No settle timer should have started — zoom stays put.
164+
// Pinch-in should request fullscreen, not zoom the inline view.
165+
await expect(app.locator(".main.fullscreen")).toHaveCount(1, {
166+
timeout: 5000,
167+
});
168+
});
169+
170+
test("trackpad pinch-out while inline is a no-op", async ({ page }) => {
171+
await page.setViewportSize({ width: 1400, height: 800 });
172+
await loadPdfServer(page);
173+
await waitForPdfRender(page);
174+
const app = getAppFrame(page);
175+
176+
const before = await readZoomPercent(page);
177+
178+
await app.locator(".canvas-container").evaluate((el) => {
179+
el.dispatchEvent(
180+
new WheelEvent("wheel", {
181+
deltaY: 50, // pinch-out
182+
ctrlKey: true,
183+
bubbles: true,
184+
cancelable: true,
185+
}),
186+
);
187+
});
188+
165189
await page.waitForTimeout(300);
166190
expect(await readZoomPercent(page)).toBe(before);
191+
await expect(app.locator(".main.fullscreen")).toHaveCount(0);
167192
});
168193
});

0 commit comments

Comments
 (0)