diff --git a/apps/web/tests/playwright.config.ts b/apps/web/tests/playwright.config.ts index c63f4a35..42469399 100644 --- a/apps/web/tests/playwright.config.ts +++ b/apps/web/tests/playwright.config.ts @@ -109,6 +109,14 @@ export default defineConfig({ }, testMatch: /brush-selection\.spec\.ts/, }, + { + name: 'zoom-indicator', + use: { + ...devices['Desktop Chrome'], + viewport: { width: 1280, height: 720 }, + }, + testMatch: /zoom-indicator\.spec\.ts/, + }, { name: 'url-view-state', use: { diff --git a/apps/web/tests/zoom-indicator.spec.ts b/apps/web/tests/zoom-indicator.spec.ts new file mode 100644 index 00000000..f98022a1 --- /dev/null +++ b/apps/web/tests/zoom-indicator.spec.ts @@ -0,0 +1,33 @@ +import { expect, test } from '@playwright/test'; +import { waitForExploreDataLoad, waitForExploreInteractionReady } from './helpers/explore'; + +test.describe('scatterplot zoom indicator (#343)', () => { + test('shows after wheel zoom and disappears after double-click reset', async ({ page }) => { + await page.goto('/explore'); + await waitForExploreDataLoad(page); + await waitForExploreInteractionReady(page); + + const plot = page.locator('#myPlot'); + const bounds = await plot.boundingBox(); + expect(bounds).not.toBeNull(); + + const center = { + x: bounds!.x + bounds!.width / 2, + y: bounds!.y + bounds!.height / 2, + }; + const pointCountChip = plot.getByRole('status'); + await expect(pointCountChip).toHaveAttribute('aria-live', 'polite'); + await expect(pointCountChip).toHaveText(/^\s*\d+ points\s*$/); + + await page.mouse.move(center.x, center.y); + await page.mouse.wheel(0, -500); + await expect + .poll(() => plot.evaluate((element: any) => element._transform.k)) + .toBeGreaterThan(1); + await expect(pointCountChip).toHaveText(/^\s*\d+ points · Zoomed in\s*$/); + + await page.mouse.dblclick(center.x, center.y); + await expect.poll(() => plot.evaluate((element: any) => element._transform.k)).toBe(1); + await expect(pointCountChip).toHaveText(/^\s*\d+ points\s*$/); + }); +}); diff --git a/openspec/changes/show-zoom-indicator/.openspec.yaml b/openspec/changes/show-zoom-indicator/.openspec.yaml new file mode 100644 index 00000000..11d5d1ae --- /dev/null +++ b/openspec/changes/show-zoom-indicator/.openspec.yaml @@ -0,0 +1,4 @@ +schema: spec-driven +created: 2026-08-01 +goal: Show an on-plot indicator whenever the scatterplot is zoomed in and remove + it when the view returns to identity. diff --git a/openspec/changes/show-zoom-indicator/design.md b/openspec/changes/show-zoom-indicator/design.md new file mode 100644 index 00000000..e5349f6d --- /dev/null +++ b/openspec/changes/show-zoom-indicator/design.md @@ -0,0 +1,51 @@ +## Context + +`PlotInteractionController.applyZoom()` forwards every D3 transform through the scatterplot host's `onTransform` callback. The host deliberately stores `_transform` as a plain field because making the full transform reactive caused an unnecessary Lit update and WebGL render on every gesture frame (the F-48 invariant). The existing bottom-left `.plot-indicator` renders the visible point count and is already the established persistent status surface for the plot. + +Issue #343 requires a visible signal specifically for zooming in. The current `zoomExtent` also permits zooming out, so the indicator must distinguish `k > 1` from both identity (`k === 1`) and zoomed-out (`k < 1`) views. + +## Goals / Non-Goals + +**Goals:** + +- Show `Zoomed in` next to the existing point count while the active D3 scale is meaningfully greater than `1`, ignoring near-identity floating-point residue from symmetric wheel gestures. +- Remove the marker once reset reaches identity. +- Announce zoom and reset status changes to assistive technology without moving focus. +- Preserve the non-reactive `_transform` performance invariant by scheduling Lit updates only when the boolean zoomed-in state changes. +- Cover the boundary in component tests and the real wheel/reset interaction in the Explore app. + +**Non-Goals:** + +- Add a new reset button, change the existing double-click reset gesture, or display a numeric zoom percentage. +- Show an indicator for panning at identity scale or for zooming out below identity. +- Refactor the interaction controller, point-count computation, or overlay layout. + +## Decisions + +### Reuse the existing point-count chip + +The marker will render as the single text run `N points · Zoomed in` inside the existing bottom-left `.plot-indicator`. The chip reuses the component's established `role="status" aria-live="polite"` pattern so its changing content is announced without focus movement. A separate overlay was rejected because it would need new collision rules for no additional user value. + +### Store only a reactive boolean boundary + +The scatterplot will add a reactive `_isZoomedIn` boolean. The host callback will continue assigning every transform to the plain `_transform` field, then compare the scale against identity plus a small fixed tolerance before assigning the boolean. The tolerance treats multiplicative wheel round-trip residue as identity without suppressing a perceptible zoom. Lit's default change detection ignores equal boolean values, so it enqueues a template render once when zoom crosses above identity and once when reset reaches identity, rather than once per D3 frame. Because the marker is light-DOM text and the controller already renders transformed plot content imperatively, marker-only updates will be excluded from the `updated()` path that redraws WebGL and rebuilds selection overlays. + +Keeping the boolean on the scatterplot host, rather than the interaction controller, preserves the controller's role as a generic transform dispatcher and keeps rendering state beside the template that consumes it. + +### Verify both state propagation and user-visible behavior + +A focused jsdom test will drive the real host bridge, assert the exact count/marker text renders as a polite status, observe `isUpdatePending` across repeated same-side transforms, verify marker-only state changes do not redraw WebGL, and cover near-identity wheel residue. A Playwright regression will locate the chip by its status role, wheel over the real Explore scatterplot, verify the complete `N points · Zoomed in` presentation, double-click reset, and assert the status returns to the point count alone. + +## Risks / Trade-offs + +- **Multiplicative wheel accumulation can finish a symmetric gesture a few ULPs above `1`** → Treat scales within a small fixed tolerance of identity as identity; the exact reset transform remains covered as well. +- **A marker appended to the count chip is less prominent than a dedicated badge** → The count chip is persistent, unobtrusive, and has no collision risk; the text remains visible throughout the zoomed state. +- **Reactive state could regress zoom performance** → Exercise consecutive same-side transforms through Lit's scheduling signal and independently assert that marker-only updates do not enter the WebGL/overlay redraw path. + +## Migration Plan + +No migration is required. The change is additive UI behavior with no public API or persisted state. Rollback consists of removing the boolean derivation and conditional template text. + +## Open Questions + +None. The count-chip presentation and non-interactive scope were approved before implementation. diff --git a/openspec/changes/show-zoom-indicator/proposal.md b/openspec/changes/show-zoom-indicator/proposal.md new file mode 100644 index 00000000..c3c3531b --- /dev/null +++ b/openspec/changes/show-zoom-indicator/proposal.md @@ -0,0 +1,27 @@ +## Why + +The scatterplot can be zoomed substantially without any persistent UI signal that the current view differs from the fitted identity view. This makes a zoomed subset easy to mistake for the complete dataset view and leaves users without visible confirmation that double-click reset is applicable. + +## What Changes + +- Show a concise `Zoomed in` marker inside the existing point-count chip whenever the scatterplot scale is greater than the identity scale. +- Remove the marker when the view returns to identity, including after the existing double-click and programmatic reset paths complete. +- Update the marker only when zoom state crosses the identity boundary so ordinary zoom frames do not trigger unnecessary Lit renders. +- Add focused component and browser regression coverage for the zoomed and reset states. + +## Capabilities + +### New Capabilities + +- `scatterplot-zoom-indicator`: Defines how the scatterplot exposes zoomed-versus-identity view state in its existing point-count indicator. + +### Modified Capabilities + +None. + +## Impact + +- `packages/core/src/components/scatter-plot/scatter-plot.ts`: track the zoom-state boundary and render the conditional marker. +- `packages/core/src/components/scatter-plot/interaction/plot-interaction-controller.test.ts` and/or a focused scatterplot render test: cover transform propagation and rendered state without mocking the behavior under test. +- `apps/web/tests/`: cover the user-visible wheel-zoom and reset journey in the real Explore page. +- No public API, dependency, persisted-data, or bundle-format changes. diff --git a/openspec/changes/show-zoom-indicator/specs/scatterplot-zoom-indicator/spec.md b/openspec/changes/show-zoom-indicator/specs/scatterplot-zoom-indicator/spec.md new file mode 100644 index 00000000..88c5ecbd --- /dev/null +++ b/openspec/changes/show-zoom-indicator/specs/scatterplot-zoom-indicator/spec.md @@ -0,0 +1,56 @@ +## ADDED Requirements + +### Requirement: Scatterplot indicates a zoomed-in view + +The scatterplot SHALL append the text `Zoomed in` to its existing visible point-count indicator whenever the active view scale is greater than the identity scale of `1` by more than the implementation's small numerical tolerance. The scatterplot SHALL NOT show that marker at identity scale, for near-identity floating-point residue, while zoomed out below identity, or for translation alone at identity scale. The point-count indicator SHALL expose its changing content as a polite status message so assistive technology can announce zoom and reset changes without moving focus. + +#### Scenario: User zooms in with the wheel + +- **WHEN** the user wheel-zooms the scatterplot to a scale greater than `1` +- **THEN** the point-count indicator includes `Zoomed in` + +#### Scenario: User zooms out without crossing identity + +- **WHEN** the user changes between two scales that are both greater than `1` +- **THEN** the point-count indicator continues to include `Zoomed in` +- **AND** the scatterplot does not schedule a new Lit update solely because the zoomed-in boolean remained true + +#### Scenario: User resets the zoom + +- **WHEN** the existing reset behavior returns the view to identity scale `1` +- **THEN** the point-count indicator no longer includes `Zoomed in` + +#### Scenario: Symmetric wheel gesture returns near identity + +- **WHEN** accumulated wheel transforms leave the scale only within the numerical tolerance above `1` +- **THEN** the point-count indicator does not include `Zoomed in` + +#### Scenario: Assistive technology receives zoom-state changes + +- **WHEN** the point-count indicator changes between identity and zoomed-in content +- **THEN** the updated content is exposed as a polite status message +- **AND** the change does not require focus to move to the indicator + +#### Scenario: User pans or zooms out + +- **WHEN** the view is translated at scale `1` or has a scale below `1` +- **THEN** the point-count indicator does not include `Zoomed in` + +### Requirement: Zoom indication preserves transform rendering performance + +The scatterplot MUST keep the full D3 transform non-reactive and SHALL derive a separate reactive boolean for the zoomed-in boundary. Transform updates SHALL schedule marker-related Lit rendering only when that boolean changes. Marker-only Lit updates SHALL NOT redraw WebGL content or rebuild selection overlays already handled by the imperative zoom path. + +#### Scenario: Zoom gesture emits multiple zoomed-in frames + +- **WHEN** consecutive transform frames all have scales greater than `1` +- **THEN** only the first frame that crosses above identity changes the reactive zoom-indicator state + +#### Scenario: Reset transition reaches identity + +- **WHEN** a reset transition emits zoomed-in frames followed by its final identity frame +- **THEN** the reactive zoom-indicator state changes once on the final identity frame + +#### Scenario: Zoom-indicator state crosses its boundary + +- **WHEN** a transform changes only the reactive zoom-indicator state +- **THEN** the status text updates without an additional WebGL redraw or selection-overlay rebuild diff --git a/openspec/changes/show-zoom-indicator/tasks.md b/openspec/changes/show-zoom-indicator/tasks.md new file mode 100644 index 00000000..cce4e0f4 --- /dev/null +++ b/openspec/changes/show-zoom-indicator/tasks.md @@ -0,0 +1,25 @@ +## 1. Regression Tests (RED) + +- [x] 1.1 Add a focused jsdom component test that drives the real scatterplot host transform callback, verifies the full count/marker presentation as a polite status, and observes that additional zoomed-in frames do not run another plot render. +- [x] 1.2 Add a focused Playwright project and test that locates the chip by its status role, wheel-zooms the loaded Explore scatterplot, verifies the complete spaced `N points · Zoomed in` presentation, double-clicks to reset, and observes the status return to the point count alone. +- [x] 1.3 Run both focused tests against the unmodified implementation and record that they fail because the zoom marker is absent. +- [x] 1.4 Run the accessibility regression against the review head and record that it fails because the chip has no polite status semantics. + +## 2. Minimal Implementation (GREEN) + +- [x] 2.1 Add a reactive boolean zoom-boundary state while keeping `_transform` non-reactive, and rely on Lit's boolean change detection to deduplicate repeated same-side transforms. +- [x] 2.2 Append `· Zoomed in` to the existing point-count chip only while the boolean state is true, and expose the chip as a polite status. +- [x] 2.3 Run both focused tests and record that they pass. + +## 3. Verification and Publication + +- [x] 3.1 Repeat the original browser reproduction and confirm the marker appears above identity and disappears after reset with no new relevant console errors. +- [x] 3.2 Run the affected core and Playwright test projects, then run `pnpm precommit`. +- [x] 3.3 Validate the OpenSpec change and review the final diff for issue-only scope. + +## 4. Adversarial Review Follow-up + +- [x] 4.1 Rewrite the boundary-scheduling regression around Lit's `isUpdatePending` signal and add a failing regression showing marker-only updates must not redraw WebGL. +- [x] 4.2 Add a failing regression for near-identity floating-point residue from symmetric wheel accumulation. +- [x] 4.3 Render the count and conditional marker as one exact text run and remove the marker-only gap styling and query-hook spans. +- [x] 4.4 Run focused component and browser tests, strict OpenSpec validation, and the repository precommit checks. diff --git a/packages/core/src/components/scatter-plot/scatter-plot.ts b/packages/core/src/components/scatter-plot/scatter-plot.ts index ae0297ae..c9d5e030 100644 --- a/packages/core/src/components/scatter-plot/scatter-plot.ts +++ b/packages/core/src/components/scatter-plot/scatter-plot.ts @@ -83,6 +83,10 @@ const VIRTUALIZATION_PADDING = 100; const HIT_TEST_SEARCH_RADIUS_PX = 15; const POINT_RADIUS_SIZE_DIVISOR = 3; +// D3 wheel zoom accumulates scale multiplicatively, so a symmetric round trip +// can finish a few ULPs above identity even though the view is visually reset. +const ZOOM_IDENTITY_EPSILON = 1e-6; + /** Default number of bins for numeric→categorical materialization. Mirrors * materializeVisualizationData's `defaultBinCount = 10` default. */ const DEFAULT_NUMERIC_BIN_COUNT = 10; @@ -161,6 +165,7 @@ export class ProtspaceScatterplot extends LitElement { @state() private _canvasKey = 0; @state() private _numericRecomputeRunning = false; @state() private _connectorStatus: ProvenanceConnectorStatus | null = null; + @state() private _isZoomedIn = false; // Queries @query('canvas') private _canvas?: HTMLCanvasElement; @@ -865,12 +870,13 @@ export class ProtspaceScatterplot extends LitElement { this._webglRenderer?.invalidateStyleCache(); this._renderPlot(); } - // Render for other changes - const selectionKeys = ['selectedProteinIds', 'highlightedProteinIds']; + // These keys affect only the template or are rendered by the selection block + // above. Zoom transforms already redraw through the interaction controller's RAF. + const noAdditionalRenderKeys = ['selectedProteinIds', 'highlightedProteinIds', '_isZoomedIn']; const changedKeys = Array.from(changedProperties.keys()).map(String); - const onlySelectionChanged = - changedKeys.length > 0 && changedKeys.every((k) => selectionKeys.includes(k)); - if (!onlySelectionChanged) { + const onlyNoAdditionalRenderKeysChanged = + changedKeys.length > 0 && changedKeys.every((k) => noAdditionalRenderKeys.includes(k)); + if (!onlyNoAdditionalRenderKeysChanged) { this._renderPlot(); this._updateSelectionOverlays(); } @@ -1225,6 +1231,7 @@ export class ProtspaceScatterplot extends LitElement { resolveSlotsToIds: (slots) => this._slotsToInteractiveIds(slots), onTransform: (t) => { this._transform = t; + this._isZoomedIn = t.k > 1 + ZOOM_IDENTITY_EPSILON; this._connectorOverlay.updateZoomScale(t.k); }, onSelect: (ids, clearVisual) => this._commitSelection(ids, clearVisual), @@ -1980,7 +1987,11 @@ export class ProtspaceScatterplot extends LitElement { ` : ''} ${this.data - ? html`
${this._getVisiblePointCount()} points
` + ? html` +
+ ${`${this._getVisiblePointCount()} points${this._isZoomedIn ? ' · Zoomed in' : ''}`} +
+ ` : ''} ${this._numericRecomputeRunning ? html` diff --git a/packages/core/src/components/scatter-plot/scatter-plot.zoom-indicator.test.ts b/packages/core/src/components/scatter-plot/scatter-plot.zoom-indicator.test.ts new file mode 100644 index 00000000..7ea9e4e4 --- /dev/null +++ b/packages/core/src/components/scatter-plot/scatter-plot.zoom-indicator.test.ts @@ -0,0 +1,125 @@ +/** + * @vitest-environment jsdom + * + * Issue #343: the plot's existing point-count chip exposes whether the active + * D3 view is zoomed in. The full transform remains non-reactive (F-48); only + * crossings between identity and k > 1 schedule a Lit update for the marker. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as d3 from 'd3'; +import type { VisualizationData } from '@protspace/utils'; +import type { PlotInteractionHost } from './interaction/plot-interaction-controller'; + +vi.hoisted(() => { + if (!('ResizeObserver' in globalThis)) { + (globalThis as unknown as { ResizeObserver: unknown }).ResizeObserver = class { + observe() {} + unobserve() {} + disconnect() {} + }; + } + vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockImplementation(() => null); +}); + +import './scatter-plot'; + +type ZoomIndicatorInternals = HTMLElement & { + data: VisualizationData; + selectedAnnotation: string; + isUpdatePending: boolean; + updateComplete: Promise; + firstUpdated(): void; + _interactionHost(): PlotInteractionHost; + _renderPlot(): void; + _updateSelectionOverlays(): void; +}; + +function makeData(): VisualizationData { + return { + protein_ids: ['p0'], + projections: [{ name: 'umap', data: new Float32Array([0, 0]), dimension: 2 }], + annotations: { + family: { + values: ['A'], + colors: ['#ff0000'], + shapes: ['circle'], + }, + }, + annotation_data: { family: [[0]] }, + } as unknown as VisualizationData; +} + +async function makePlot(): Promise { + const plot = document.createElement('protspace-scatterplot') as ZoomIndicatorInternals; + // Avoid WebGL/controller startup; these tests drive the real host bridge directly. + plot.firstUpdated = () => {}; + plot.data = makeData(); + plot.selectedAnnotation = 'family'; + document.body.appendChild(plot); + while (!(await plot.updateComplete)) { + // Lit reports false while an update triggered by the previous cycle is pending. + } + return plot; +} + +describe('scatterplot zoom indicator (#343)', () => { + beforeEach(() => { + vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockImplementation(() => null); + }); + + afterEach(() => { + document.body.replaceChildren(); + vi.restoreAllMocks(); + }); + + it('renders the point count and zoom label as a polite status', async () => { + const plot = await makePlot(); + const host = plot._interactionHost(); + host.onTransform(d3.zoomIdentity.scale(2)); + await plot.updateComplete; + + const chip = plot.shadowRoot?.querySelector('[role="status"][aria-live="polite"]'); + expect(chip?.textContent?.trim()).toBe('1 points · Zoomed in'); + }); + + it('schedules a Lit update only when the zoomed-in boundary changes', async () => { + const plot = await makePlot(); + const host = plot._interactionHost(); + + host.onTransform(d3.zoomIdentity.scale(2)); + expect(plot.isUpdatePending).toBe(true); + await plot.updateComplete; + + host.onTransform(d3.zoomIdentity.scale(3)); + expect(plot.isUpdatePending).toBe(false); + + host.onTransform(d3.zoomIdentity.translate(30, 20)); + expect(plot.isUpdatePending).toBe(true); + await plot.updateComplete; + expect(plot.shadowRoot?.querySelector('[role="status"]')?.textContent?.trim()).toBe('1 points'); + + host.onTransform(d3.zoomIdentity.scale(0.5)); + expect(plot.isUpdatePending).toBe(false); + }); + + it('does not redraw plot content for a zoom-indicator-only update', async () => { + const plot = await makePlot(); + const renderPlot = vi.spyOn(plot, '_renderPlot').mockImplementation(() => {}); + const updateOverlays = vi.spyOn(plot, '_updateSelectionOverlays').mockImplementation(() => {}); + + plot._interactionHost().onTransform(d3.zoomIdentity.scale(2)); + await plot.updateComplete; + + expect(renderPlot).not.toHaveBeenCalled(); + expect(updateOverlays).not.toHaveBeenCalled(); + }); + + it('treats floating-point wheel residue as identity', async () => { + const plot = await makePlot(); + + plot._interactionHost().onTransform(d3.zoomIdentity.scale(1.0000000000000002)); + await plot.updateComplete; + + expect(plot.shadowRoot?.querySelector('[role="status"]')?.textContent?.trim()).toBe('1 points'); + }); +});