diff --git a/docs/superpowers/plans/2026-08-11-ngviews-plain-array-layers.md b/docs/superpowers/plans/2026-08-11-ngviews-plain-array-layers.md new file mode 100644 index 00000000..20caacb0 --- /dev/null +++ b/docs/superpowers/plans/2026-08-11-ngviews-plain-array-layers.md @@ -0,0 +1,153 @@ +# Plan: Views checkout must handle plain Zarr arrays (one layer per cart dataset) + +Date: 2026-08-11 +Branch: `ngviews-06-embedded-readonly` +Status: for review + +## Problem + +Adding several plain Zarr array directories to the Layer Cart (e.g. `.../seed6/data.zarr/affs`, +`/lsds`, `/img_zyx`, `/seg`) and clicking **Create View** should produce one Neuroglancer +layer per added directory. It does not. The saved View reports **0 layers**, expanding a cart +row throws a `not found: v3 array or group` pop-up, and opening the View shows a black +Neuroglancer with a single `1 new layer` tab. + +This is **not** the "auto-detect all children of a `.zgroup`" feature requested on Slack. This is +the existing manual-add path failing for anything that is not an OME-Zarr multiscale group. + +## Root cause + +The checkout pipeline is OME-Zarr-first and never handles a bare array. + +- `frontend/src/utils/viewCheckout.ts:37` — `generateStateForDataset` calls + `getOmeZarrMetadata(ds.url)` for **every** dataset. +- `frontend/src/omezarr-helper.ts:598` — `getOmeZarrMetadata` calls + `omezarr.getMultiscaleWithArray(store, 0)`, which demands a multiscale group. A plain array + **throws** (`not found: v3 array or group`). +- The throw is caught at `viewCheckout.ts:53` and returns `null`, so the dataset is **dropped** + (→ 0 layers). +- The fallback `generateNeuroglancerStateForDataURL` at `viewCheckout.ts:51` is **dead code** + for plain arrays: it lives inside the ternary that only runs *after* `getOmeZarrMetadata` + succeeds. When line 37 throws, the fallback never executes. +- The same throw surfaces raw in the channel-expand UI (`getOmeZarrChannels` → + `getOmeZarrMetadata`) → error pop-up. + +## Fix + +Make `generateStateForDataset` fall back to a single plain-array layer when OME multiscale +detection fails, instead of returning `null`. + +### 1. Add a plain-array state helper (`omezarr-helper.ts`) + +```ts +// Open a plain Zarr array with auto-detected storage version and emit a +// single-layer NG state. Used when a cart dataset is a bare array, not an +// OME-Zarr multiscale group. +export async function generateStateForPlainZarr(dataUrl: string): Promise { + const store = new zarr.FetchStore(dataUrl, { overrides: { credentials: 'include' } }); + const arr = await omezarr.getArray(store, '/', undefined); // undefined = probe v2/v3 + const zarrVersion = arr.metadata.zarr_format as 2 | 3; + return generateNeuroglancerStateForDataURL(dataUrl, zarrVersion); +} +``` + +Reuses existing `generateNeuroglancerStateForDataURL` (emits `type:'new'`, correct `|zarrN:` +source, layout `4panel-alt`). No new render logic. + +### 2. Fall back in `generateStateForDataset` (`viewCheckout.ts`) + +```ts +async function generateStateForDataset(ds): Promise { + try { + const metadata = await getOmeZarrMetadata(ds.url); + const multiscale = metadata.multiscales?.[0]; + const encoded = multiscale + ? generateNeuroglancerStateForOmeZarr(ds.url, metadata.zarrVersion, 'image', + multiscale, metadata.arr, metadata.labels, metadata.omero) + : generateNeuroglancerStateForDataURL(ds.url, metadata.zarrVersion); + return decodeState(encoded); + } catch (omeError) { + // Not an OME-Zarr multiscale group. Try a plain array before giving up. + try { + return decodeState(await generateStateForPlainZarr(ds.url)); + } catch (plainError) { + log.error(`Failed to generate NG state for ${ds.url}`, omeError, plainError); + return null; // genuinely broken (moved/deleted/not zarr) → skip, keep the rest + } + } +} +``` + +Result: N added array dirs → N layers. `null` now means "not a Zarr array at all", not +"not OME-Zarr". + +### 3. Stop the channel-expand pop-up (`getOmeZarrChannels`, `omezarr-helper.ts:642`) + +Plain arrays have no channels. `getOmeZarrMetadata` throws inside `getOmeZarrChannels`. Wrap so +a plain array returns `[]` instead of throwing; the cart row already shows the +"Channels load after the View is created." / single-array hint. + +```ts +async function getOmeZarrChannels(dataUrl: string): Promise { + let metadata; + try { + metadata = await getOmeZarrMetadata(dataUrl); + } catch { + return []; // plain array / no multiscale → no channels to pick + } + // ...unchanged... +} +``` + +## Tests + +- `frontend/src/__tests__/` unit test for `buildViewState`: mock `getOmeZarrMetadata` to throw + and `generateStateForPlainZarr` to return a one-layer state; assert a 3-dataset cart yields + `layers.length === 3` and `ng_state.layers.length === 3` (self-check for the branch/loop logic). +- Manual on dev: add `affs`, `lsds`, `img_zyx`, `seg` under `seed6/data.zarr`, Create View, + confirm 4 layers in the Saved Views table and 4 tabs in the embedded viewer. + +## Follow-up (deferred to later PRs in the stack) + +QA on dev after the initial fix surfaced these; user chose to keep this branch to +the type-default fix and split the rest: + +- **Per-layer type override in the Layer Cart**: image / segmentation / + multi-channel selector per cart dataset, plumbed through `ViewLayerInput.opts` + into the generated NG state. Covers segmentation-by-choice and the multichannel + case below. +- **Multi-channel arrays (affs, lsds)**: a bare multi-channel float array renders + as one grey channel because there is no channel dimension/shader. Needs the NG + state to emit a local `c` dimension + shader; Neuroglancer cannot infer it + without OME axis metadata. Overlaps the override work. +- **Show data paths on the view page**: `/view/:readKey` address bar is the short + app route by design; **Copy link already yields the full Neuroglancer-style URL + with every layer's data source embedded**. Optional: a panel on `/view` that + lists each layer's full data URL, mirroring how a data link shows its path. + +## Type default (done in this branch) + +Plain-array fallback no longer emits `type:'new'` (which forces Neuroglancer's +layer-type picker and renders raw grey). `generateStateForPlainZarr` now opens the +array, guesses a type from name + dtype (`guessPlainLayerType`: integer dtype + +name matching `seg|label|mask` -> `segmentation`, else `image`), and uses the +explicit-type generator `generateNeuroglancerStateForZarrArray`. The existing +thumbnail-edge heuristic (`determineLayerType`) is not usable here — checkout has +no rendered thumbnail (see `viewCheckout.ts:39`). + +## Out of scope (separate items, noted not fixed) + +- **Misalignment**: cross-array coordinate spaces are not reconciled — first dataset's + dimensions win (`viewCheckout.ts:75` ceiling). Arrays with differing scale/offset render + misaligned. Expected; document, don't fix here. +- **Segmentation typing**: plain-array fallback emits `type:'new'`; `seg` shows as image, not a + labels layer. Neuroglancer lets the user switch. Follow-up if auto-typing wanted. +- **UX (#1/#2/#3)**: "Create View" button semantics, channels-load-after-create ordering, and + duplicate-View-on-repeat-click are real but independent of this data bug. Track separately. +- **`.zgroup` auto fan-out** (the Slack feature): not this. Explicitly not doing it. + +## Risk + +Low. Additive fallback; OME-Zarr path unchanged. Worst case a genuinely broken dir is still +skipped (same as today). `omezarr.getArray(store, '/', undefined)` version-probe is the one +external assumption — verify it resolves v2 arrays on dev before merge. diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index f018dd39..3598a692 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -32,7 +32,7 @@ import Notifications from '@/components/Notifications'; import SSHKeys from '@/components/SSHKeys'; import ErrorFallback from '@/components/ErrorFallback'; import NGViews from '@/components/NGViews'; -import { ViewsProvider } from '@/contexts/ViewsContext'; +import NeuroglancerView from '@/components/NeuroglancerView'; function RequireAuth({ children }: { readonly children: ReactNode }) { const { loading, authStatus } = useAuthContext(); @@ -122,9 +122,7 @@ const AppComponent = () => { - - - + } path="ngviews" @@ -214,6 +212,14 @@ const AppComponent = () => { /> } path="relaunch/:owner/:repo" /> + + + + } + path="view/:readKey" + /> diff --git a/frontend/src/__tests__/componentTests/AppearsInViews.test.tsx b/frontend/src/__tests__/componentTests/AppearsInViews.test.tsx index ecc4ab8b..3470f6da 100644 --- a/frontend/src/__tests__/componentTests/AppearsInViews.test.tsx +++ b/frontend/src/__tests__/componentTests/AppearsInViews.test.tsx @@ -1,5 +1,6 @@ import { describe, it, expect, vi } from 'vitest'; import { render, screen } from '@testing-library/react'; +import { MemoryRouter } from 'react-router'; const { useViewsForDataLinkQuery } = vi.hoisted(() => ({ useViewsForDataLinkQuery: vi.fn() @@ -12,16 +13,26 @@ describe('AppearsInViews', () => { it('lists the dependent Views with a count', () => { useViewsForDataLinkQuery.mockReturnValue({ data: [ - { short_key: 'v1', name: 'Alpha' }, - { short_key: 'v2', name: 'Beta' } + { short_key: 'v1', name: 'Alpha', read_key: 'rk1' }, + { short_key: 'v2', name: 'Beta', read_key: 'rk2' } ], isPending: false, isError: false }); - render(); + render( + + + + ); expect(screen.getByText(/appears in 2 views/i)).toBeInTheDocument(); - expect(screen.getByText('Alpha')).toBeInTheDocument(); - expect(screen.getByText('Beta')).toBeInTheDocument(); + expect(screen.getByRole('link', { name: 'Alpha' })).toHaveAttribute( + 'href', + '/view/rk1' + ); + expect(screen.getByRole('link', { name: 'Beta' })).toHaveAttribute( + 'href', + '/view/rk2' + ); }); it('renders nothing when there are no dependent Views', () => { diff --git a/frontend/src/__tests__/componentTests/CartList.test.tsx b/frontend/src/__tests__/componentTests/CartList.test.tsx index e0dbe147..f8ff6e6f 100644 --- a/frontend/src/__tests__/componentTests/CartList.test.tsx +++ b/frontend/src/__tests__/componentTests/CartList.test.tsx @@ -1,15 +1,31 @@ -import { describe, it, expect, vi } from 'vitest'; -import { render, screen } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import type { CartItem } from '@/contexts/CartContext'; +import type { View } from '@/queries/viewQueries'; const cartA: CartItem = { fsp_name: 'f', path: '/a', label: 'Dataset A' }; const cartB: CartItem = { fsp_name: 'f', path: '/b', label: 'Dataset B' }; +const createdView: View = vi.hoisted(() => ({ + short_key: 'v1', + read_key: 'rk1', + name: 'New View', + ng_state: {}, + sharing_mode: 'read', + owner: 'me', + created_at: '2026-08-01T00:00:00Z', + updated_at: '2026-08-01T00:00:00Z', + layers: [] +})); let cart: CartItem[] = []; +const navigate = vi.hoisted(() => vi.fn()); +const clearCart = vi.hoisted(() => vi.fn()); +vi.mock('react-router', () => ({ useNavigate: () => navigate })); vi.mock('@/contexts/CartContext', () => ({ useCartContext: () => ({ cart, - clearCart: vi.fn().mockResolvedValue(undefined) + clearCart }) })); vi.mock('@/queries/proxiedPathQueries', () => ({ @@ -21,13 +37,26 @@ vi.mock('@/components/ui/Views/CartDatasetRow', () => ({ ) })); vi.mock('@/components/ui/Views/CreateViewButton', () => ({ - default: ({ label }: { label?: string }) => ( - + default: ({ + label, + onCreated + }: { + label?: string; + onCreated?: (view: View) => void; + }) => ( + ) })); import CartList from '@/components/ui/Views/CartList'; +beforeEach(() => { + navigate.mockClear(); + clearCart.mockReset().mockResolvedValue(undefined); +}); + describe('CartList', () => { it('shows the empty state when the cart is empty', () => { cart = []; @@ -46,4 +75,18 @@ describe('CartList', () => { screen.getByRole('button', { name: /clear cart/i }) ).toBeInTheDocument(); }); + + it('navigates to the embedded viewer and clears the cart when a View is created', async () => { + cart = [cartA, cartB]; + const user = userEvent.setup(); + render(); + + await user.click(screen.getByRole('button', { name: /create view/i })); + + expect(navigate).toHaveBeenCalledWith('/view/rk1'); + // CartList's datasets come from the persisted cart, so it - unlike + // SelectionBar/FileBrowser - is the one caller that should clear it + // after a successful checkout. + await waitFor(() => expect(clearCart).toHaveBeenCalled()); + }); }); diff --git a/frontend/src/__tests__/componentTests/CreateViewButton.test.tsx b/frontend/src/__tests__/componentTests/CreateViewButton.test.tsx index f43f0006..4818f8c1 100644 --- a/frontend/src/__tests__/componentTests/CreateViewButton.test.tsx +++ b/frontend/src/__tests__/componentTests/CreateViewButton.test.tsx @@ -19,6 +19,9 @@ vi.mock('@/queries/proxiedPathQueries', () => ({ useAllProxiedPathsQuery: () => ({ data: [] }) // nothing exists → 1 new link })); vi.mock('react-router', () => ({ useNavigate: () => vi.fn() })); +vi.mock('@/contexts/CartContext', () => ({ + useCartContext: () => ({ clearCart: vi.fn().mockResolvedValue(undefined) }) +})); import CreateViewButton from '@/components/ui/Views/CreateViewButton'; diff --git a/frontend/src/__tests__/componentTests/FileBrowserCartItem.test.tsx b/frontend/src/__tests__/componentTests/FileBrowserCartItem.test.tsx index a24abc64..1a0c75d4 100644 --- a/frontend/src/__tests__/componentTests/FileBrowserCartItem.test.tsx +++ b/frontend/src/__tests__/componentTests/FileBrowserCartItem.test.tsx @@ -26,6 +26,24 @@ vi.mock('@/contexts/CartContext', async importOriginal => { }; }); +// FileBrowser also renders a "View in Neuroglancer" item that depends on +// useCreateViewFlow, which in turn needs a ViewsProvider this test's render +// tree doesn't set up. This suite only cares about the cart item, so stub +// the hook rather than wiring up ViewsProvider. +vi.mock('@/hooks/useCreateViewFlow', async importOriginal => { + const actual = + await importOriginal(); + return { + ...actual, + useCreateViewFlow: () => ({ + startCreateView: vi.fn(), + dialog: null, + open: false, + pending: false + }) + }; +}); + // FileTable virtualizes rows in a way that doesn't render meaningfully in // jsdom. Stub it with plain buttons that invoke the same // handleContextMenuClick callback FileBrowser wires up, so the test can diff --git a/frontend/src/__tests__/componentTests/FileBrowserViewInNg.test.tsx b/frontend/src/__tests__/componentTests/FileBrowserViewInNg.test.tsx new file mode 100644 index 00000000..e6411af0 --- /dev/null +++ b/frontend/src/__tests__/componentTests/FileBrowserViewInNg.test.tsx @@ -0,0 +1,152 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { http, HttpResponse } from 'msw'; +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import { render } from '../test-utils'; +import { server } from '@/__tests__/mocks/node'; +import FileBrowser from '@/components/ui/BrowsePage/FileBrowser'; +import type { FileOrFolder } from '@/shared.types'; + +const startCreateView = vi.fn(); + +vi.mock('@/hooks/useCreateViewFlow', async importOriginal => { + const actual = + await importOriginal(); + return { + ...actual, + useCreateViewFlow: () => ({ + startCreateView, + dialog: null, + open: false, + pending: false + }) + }; +}); + +// FileTable virtualizes rows in a way that doesn't render meaningfully in +// jsdom. Stub it with plain buttons that invoke the same +// handleContextMenuClick callback FileBrowser wires up, so the test can +// drive FileBrowser's real context-menu-item logic (name/shouldShow/action) +// without depending on virtualized-row DOM mechanics. +vi.mock('@/components/ui/BrowsePage/FileTable', () => ({ + default: ({ + data, + handleContextMenuClick + }: { + data: FileOrFolder[]; + handleContextMenuClick: (e: unknown, file: FileOrFolder) => void; + }) => ( +
+ {data.map(file => ( + + ))} +
+ ) +})); + +const noop = vi.fn(); + +function renderFileBrowser() { + return render( + , + { initialEntries: ['/browse/test_fsp/my_folder'] } + ); +} + +describe('FileBrowser row context menu - View in Neuroglancer', () => { + beforeEach(() => { + startCreateView.mockClear(); + + server.use( + http.get('/api/files/:fspName', ({ params, request }) => { + const { fspName } = params; + if (fspName !== 'test_fsp') { + return HttpResponse.json({ error: 'Not found' }, { status: 404 }); + } + const url = new URL(request.url); + const subpath = url.searchParams.get('subpath') ?? 'my_folder'; + return HttpResponse.json({ + info: { + name: subpath.split('/').pop(), + path: subpath, + size: 0, + is_dir: true, + permissions: 'drwxr-xr-x', + owner: 'testuser', + group: 'testgroup', + last_modified: 1647855213 + }, + files: [ + { name: 'subfolder', is_dir: true, path: `${subpath}/subfolder` }, + { name: 'file1.txt', is_dir: false, path: `${subpath}/file1.txt` }, + { + name: 'linked_folder', + is_dir: true, + is_symlink: true, + symlink_target_fsp: null, + path: `${subpath}/linked_folder` + } + ] + }); + }) + ); + }); + + it('calls startCreateView with a single-dataset CartItem for the clicked folder', async () => { + const user = userEvent.setup(); + renderFileBrowser(); + + const menuButton = await screen.findByText('menu-subfolder'); + await user.click(menuButton); + + const viewItem = await screen.findByText('View in Neuroglancer'); + await user.click(viewItem); + + expect(startCreateView).toHaveBeenCalledTimes(1); + const [datasets, name, onCreated] = startCreateView.mock.calls[0]; + expect(datasets).toEqual([ + { fsp_name: 'test_fsp', path: 'my_folder/subfolder', label: 'subfolder' } + ]); + expect(name).toBe('subfolder'); + expect(typeof onCreated).toBe('function'); + }); + + it('does not show the item for a plain file', async () => { + const user = userEvent.setup(); + renderFileBrowser(); + + const menuButton = await screen.findByText('menu-file1.txt'); + await user.click(menuButton); + + await waitFor(() => { + expect(screen.getByText('Download')).toBeInTheDocument(); + }); + expect(screen.queryByText('View in Neuroglancer')).not.toBeInTheDocument(); + expect(startCreateView).not.toHaveBeenCalled(); + }); + + it('does not show the item for a symlinked folder', async () => { + const user = userEvent.setup(); + renderFileBrowser(); + + const menuButton = await screen.findByText('menu-linked_folder'); + await user.click(menuButton); + + await waitFor(() => { + expect(screen.getByText('Rename')).toBeInTheDocument(); + }); + expect(screen.queryByText('View in Neuroglancer')).not.toBeInTheDocument(); + expect(startCreateView).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/__tests__/componentTests/FileTableSelectColumn.test.tsx b/frontend/src/__tests__/componentTests/FileTableSelectColumn.test.tsx index 27f5607b..1dc185a7 100644 --- a/frontend/src/__tests__/componentTests/FileTableSelectColumn.test.tsx +++ b/frontend/src/__tests__/componentTests/FileTableSelectColumn.test.tsx @@ -32,6 +32,24 @@ vi.mock('react-router', async () => { }; }); +// FileBrowser renders a "View in Neuroglancer" item that depends on +// useCreateViewFlow, which needs a ViewsProvider this test's render tree +// doesn't set up. This suite only cares about the select column, so stub +// the hook rather than wiring up ViewsProvider. +vi.mock('@/hooks/useCreateViewFlow', async importOriginal => { + const actual = + await importOriginal(); + return { + ...actual, + useCreateViewFlow: () => ({ + startCreateView: vi.fn(), + dialog: null, + open: false, + pending: false + }) + }; +}); + describe('FileTable select column', () => { it('renders a "Select all" header checkbox that toggles on click', async () => { server.use( diff --git a/frontend/src/__tests__/componentTests/MainLayout.test.tsx b/frontend/src/__tests__/componentTests/MainLayout.test.tsx new file mode 100644 index 00000000..0c4e0db0 --- /dev/null +++ b/frontend/src/__tests__/componentTests/MainLayout.test.tsx @@ -0,0 +1,97 @@ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import { MemoryRouter, Route, Routes } from 'react-router'; +import type { ReactNode } from 'react'; + +// MainLayout composes ~a dozen context providers unrelated to this test; +// stub them all as passthroughs so we can assert on the one thing that +// changed: the navbar is no longer skipped for /view/:readKey. +// vi.mock factories are hoisted above imports, so the shared stub must be +// created via vi.hoisted rather than a plain top-level const. +const { passthrough } = vi.hoisted(() => ({ + passthrough: ({ children }: { children: ReactNode }) => children +})); + +vi.mock('@/components/ui/Navbar/Navbar', () => ({ + default: () =>
+})); +vi.mock('@/components/ui/Notifications/Notifications', () => ({ + default: () => null +})); +vi.mock('@/components/ui/Dialogs/ServerDownOverlay', () => ({ + ServerDownOverlay: () => null +})); +vi.mock('react-shepherd', () => ({ + ShepherdJourneyProvider: passthrough +})); +vi.mock('react-hot-toast', () => ({ + __esModule: true, + default: Object.assign(() => {}, { success: vi.fn(), error: vi.fn() }), + Toaster: () => null +})); +vi.mock('@/contexts/ServerHealthContext', () => ({ + ServerHealthProvider: passthrough, + useServerHealthContext: () => ({ + showWarningOverlay: false, + checkHealth: vi.fn(), + nextRetrySeconds: 0 + }) +})); +vi.mock('@/contexts/ZonesAndFspMapContext', () => ({ + ZonesAndFspMapContextProvider: passthrough +})); +vi.mock('@/contexts/FileBrowserContext', () => ({ + FileBrowserContextProvider: passthrough +})); +vi.mock('@/contexts/PreferencesContext', () => ({ + PreferencesProvider: passthrough +})); +vi.mock('@/contexts/CartContext', () => ({ CartProvider: passthrough })); +vi.mock('@/contexts/ViewsContext', () => ({ ViewsProvider: passthrough })); +vi.mock('@/contexts/OpenFavoritesContext', () => ({ + OpenFavoritesProvider: passthrough +})); +vi.mock('@/contexts/TicketsContext', () => ({ TicketProvider: passthrough })); +vi.mock('@/contexts/ProxiedPathContext', () => ({ + ProxiedPathProvider: passthrough +})); +vi.mock('@/contexts/ExternalBucketContext', () => ({ + ExternalBucketProvider: passthrough +})); +vi.mock('@/contexts/ProfileContext', () => ({ + ProfileContextProvider: passthrough +})); +vi.mock('@/contexts/NotificationsContext', () => ({ + NotificationProvider: passthrough +})); +vi.mock('@/contexts/ViewersContext', () => ({ ViewersProvider: passthrough })); + +import { MainLayout } from '@/layouts/MainLayout'; + +describe('MainLayout', () => { + it('renders the navbar on the embedded viewer route (/view/:readKey)', () => { + render( + + + } path="/*"> + viewer
} path="view/:readKey" /> +
+ + + ); + expect(screen.getByTestId('navbar')).toBeInTheDocument(); + }); + + it('still renders the navbar on an ordinary route', () => { + render( + + + } path="/*"> + browse} path="browse" /> + + + + ); + expect(screen.getByTestId('navbar')).toBeInTheDocument(); + }); +}); diff --git a/frontend/src/__tests__/componentTests/NeuroglancerView.test.tsx b/frontend/src/__tests__/componentTests/NeuroglancerView.test.tsx new file mode 100644 index 00000000..ba9b154a --- /dev/null +++ b/frontend/src/__tests__/componentTests/NeuroglancerView.test.tsx @@ -0,0 +1,131 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { createElement } from 'react'; +import type { ReactNode } from 'react'; + +const { useViewStateByReadKey } = vi.hoisted(() => ({ + useViewStateByReadKey: vi.fn() +})); +vi.mock('@/queries/viewQueries', () => ({ useViewStateByReadKey })); +vi.mock('@/hooks/useDefaultNeuroglancerBaseUrl', () => ({ + useInternalNeuroglancerBaseUrl: () => 'https://ng.example/' +})); +vi.mock('react-router', () => ({ + useParams: () => ({ readKey: 'rk1' }), + Link: ({ to, children }: { to: string; children: ReactNode }) => + createElement('a', { href: to }, children) +})); + +const { copyToClipboard } = vi.hoisted(() => ({ + copyToClipboard: vi.fn() +})); +vi.mock('@/utils/copyText', () => ({ copyToClipboard })); + +import NeuroglancerView from '@/components/NeuroglancerView'; + +describe('NeuroglancerView', () => { + beforeEach(() => { + copyToClipboard.mockReset(); + copyToClipboard.mockResolvedValue({ success: true }); + window.history.replaceState(null, '', '/'); + }); + + it('shows a loading state while pending', () => { + useViewStateByReadKey.mockReturnValue({ + data: undefined, + isPending: true, + isError: false + }); + render(); + expect(screen.getByText(/loading/i)).toBeInTheDocument(); + }); + + it('shows "not found" when the key resolves to null', () => { + useViewStateByReadKey.mockReturnValue({ + data: null, + isPending: false, + isError: false + }); + render(); + expect(screen.getByText(/view not found/i)).toBeInTheDocument(); + }); + + it('iframes Neuroglancer with the inline state and shows the export actions', () => { + useViewStateByReadKey.mockReturnValue({ + data: { title: 'My View', layers: [{ name: 'L0' }] }, + isPending: false, + isError: false + }); + render(); + const iframe = screen.getByTitle(/neuroglancer/i) as HTMLIFrameElement; + expect(iframe.src).toContain('https://ng.example/#!'); + expect(iframe.src).toContain( + encodeURIComponent( + JSON.stringify({ title: 'My View', layers: [{ name: 'L0' }] }) + ) + ); + expect(screen.getAllByText('My View').length).toBeGreaterThan(0); + expect( + screen.getByRole('button', { name: /copy link/i }) + ).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: /download json/i }) + ).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: /open external/i }) + ).toBeInTheDocument(); + }); + + it('shows a breadcrumb linking back to the NG Views list', () => { + useViewStateByReadKey.mockReturnValue({ + data: { title: 'My View', layers: [{ name: 'L0' }] }, + isPending: false, + isError: false + }); + render(); + const crumbLink = screen.getByRole('link', { name: /ng views/i }); + expect(crumbLink).toHaveAttribute('href', '/ngviews'); + }); + + it('falls back to "Untitled View" when the view has no title', () => { + useViewStateByReadKey.mockReturnValue({ + data: { layers: [{ name: 'L0' }] }, + isPending: false, + isError: false + }); + render(); + expect(screen.getAllByText('Untitled View').length).toBeGreaterThan(0); + }); + + it('reflects the full state into the app URL hash after load', () => { + const data = { title: 'My View', layers: [{ name: 'L0' }] }; + useViewStateByReadKey.mockReturnValue({ + data, + isPending: false, + isError: false + }); + render(); + expect(window.location.hash).toBe( + '#!' + encodeURIComponent(JSON.stringify(data)) + ); + }); + + it('copies the canonical short link (not the full-state hash URL or the external URL) when "Copy link" is clicked', async () => { + const user = userEvent.setup(); + useViewStateByReadKey.mockReturnValue({ + data: { title: 'My View', layers: [{ name: 'L0' }] }, + isPending: false, + isError: false + }); + render(); + await user.click(screen.getByRole('button', { name: /copy link/i })); + expect(copyToClipboard).toHaveBeenCalledWith( + `${window.location.origin}/view/rk1` + ); + expect(copyToClipboard).not.toHaveBeenCalledWith(window.location.href); + expect(copyToClipboard).not.toHaveBeenCalledWith( + expect.stringContaining('https://ng.example/') + ); + }); +}); diff --git a/frontend/src/__tests__/componentTests/SelectionBar.test.tsx b/frontend/src/__tests__/componentTests/SelectionBar.test.tsx index 2807a57e..aa64c980 100644 --- a/frontend/src/__tests__/componentTests/SelectionBar.test.tsx +++ b/frontend/src/__tests__/componentTests/SelectionBar.test.tsx @@ -2,9 +2,25 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import toast from 'react-hot-toast'; +import type { View } from '@/queries/viewQueries'; const clearChecked = vi.fn(); const addToCart = vi.fn().mockResolvedValue(undefined); +const clearCart = vi.fn().mockResolvedValue(undefined); +const navigate = vi.hoisted(() => vi.fn()); +const createdView: View = vi.hoisted(() => ({ + short_key: 'v1', + read_key: 'rk1', + name: 'New View', + ng_state: {}, + sharing_mode: 'read', + owner: 'me', + created_at: '2026-08-01T00:00:00Z', + updated_at: '2026-08-01T00:00:00Z', + layers: [] +})); + +vi.mock('react-router', () => ({ useNavigate: () => navigate })); const twoCheckedFiles = [ { name: 'a.txt', path: '/dir/a.txt' }, @@ -21,11 +37,15 @@ vi.mock('@/contexts/FileBrowserContext', () => ({ })); vi.mock('@/contexts/CartContext', () => ({ - useCartContext: () => ({ addToCart }) + useCartContext: () => ({ addToCart, clearCart }) })); vi.mock('@/components/ui/Views/CreateViewButton', () => ({ - default: () => + default: ({ onCreated }: { onCreated?: (view: View) => void }) => ( + + ) })); import SelectionBar from '@/components/ui/BrowsePage/SelectionBar'; @@ -33,6 +53,8 @@ import SelectionBar from '@/components/ui/BrowsePage/SelectionBar'; beforeEach(() => { clearChecked.mockClear(); addToCart.mockClear(); + clearCart.mockClear(); + navigate.mockClear(); vi.mocked(toast.success).mockClear(); vi.mocked(toast.error).mockClear(); checkedFiles = twoCheckedFiles; @@ -70,6 +92,21 @@ describe('SelectionBar', () => { ).toBeInTheDocument(); }); + it('navigates to the embedded viewer when a View is created, without clearing the cart', async () => { + const user = userEvent.setup(); + render(); + + await user.click( + screen.getByRole('button', { name: /new view from selection/i }) + ); + + expect(navigate).toHaveBeenCalledWith('/view/rk1'); + // Datasets here come from fileBrowserState.checkedFiles, not the cart - + // a View created from a file-browser selection must never wipe an + // unrelated Layer Cart. + expect(clearCart).not.toHaveBeenCalled(); + }); + it('clears the selection when Clear is clicked', async () => { const user = userEvent.setup(); render(); diff --git a/frontend/src/__tests__/componentTests/ngViewsColumns.test.tsx b/frontend/src/__tests__/componentTests/ngViewsColumns.test.tsx index d381ad22..7ff683a9 100644 --- a/frontend/src/__tests__/componentTests/ngViewsColumns.test.tsx +++ b/frontend/src/__tests__/componentTests/ngViewsColumns.test.tsx @@ -7,7 +7,16 @@ import { flexRender } from '@tanstack/react-table'; -import { useNGViewsColumns } from '@/components/ui/Table/ngViewsColumns'; +const navigate = vi.hoisted(() => vi.fn()); +vi.mock('react-router', async importOriginal => { + const actual = await importOriginal(); + return { ...actual, useNavigate: () => navigate }; +}); + +import { + useNGViewsColumns, + ActionsCell +} from '@/components/ui/Table/ngViewsColumns'; import type { View } from '@/queries/viewQueries'; import { formatDateString } from '@/utils'; @@ -96,4 +105,21 @@ describe('useNGViewsColumns', () => { await user.click(await screen.findByText('Delete')); expect(onDelete).toHaveBeenCalledWith(view); }); + + it('navigates to the embedded viewer when "Open in Neuroglancer" is clicked', async () => { + const user = userEvent.setup(); + render( + + ); + const trigger = screen.getByRole('button'); + + await user.click(trigger); + await user.click(await screen.findByText('Open in Neuroglancer')); + expect(navigate).toHaveBeenCalledWith(`/view/${view.read_key}`); + }); }); diff --git a/frontend/src/__tests__/componentTests/useCreateViewFlow.test.tsx b/frontend/src/__tests__/componentTests/useCreateViewFlow.test.tsx new file mode 100644 index 00000000..85cca967 --- /dev/null +++ b/frontend/src/__tests__/componentTests/useCreateViewFlow.test.tsx @@ -0,0 +1,117 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, act, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import toast from 'react-hot-toast'; + +const checkout = vi.fn(); +let automatic = true; +vi.mock('@/hooks/useCartCheckout', () => ({ + useCartCheckout: () => ({ checkout }) +})); +vi.mock('@/contexts/PreferencesContext', () => ({ + usePreferencesContext: () => ({ + areDataLinksAutomatic: automatic, + dataLinkSubpathMode: 'full_path', + toggleAutomaticDataLinks: vi.fn() + }) +})); +vi.mock('@/queries/proxiedPathQueries', () => ({ + useAllProxiedPathsQuery: () => ({ data: [] }) +})); +vi.mock('react-router', () => ({ useNavigate: () => vi.fn() })); + +import { useCreateViewFlow } from '@/hooks/useCreateViewFlow'; + +const datasets = [{ fsp_name: 'f', path: '/a', label: 'A' }]; + +type FlowApi = ReturnType; + +// Renders the hook for real (rather than via renderHook) so its returned +// `dialog` node stays reactive across state updates, the same way +// CreateViewButton consumes it. +function Harness({ apiRef }: { apiRef: { current: FlowApi | null } }) { + const api = useCreateViewFlow(); + apiRef.current = api; + return <>{api.dialog}; +} + +beforeEach(() => { + checkout + .mockReset() + .mockResolvedValue({ short_key: 'v1', read_key: 'rk1', name: 'A' }); + automatic = true; +}); + +describe('useCreateViewFlow', () => { + it('never checks out immediately when links are automatic; opens a dialog with the prefilled name instead', async () => { + const onCreated = vi.fn(); + const apiRef: { current: FlowApi | null } = { current: null }; + render(); + + act(() => { + apiRef.current!.startCreateView(datasets, 'A', onCreated); + }); + + expect(checkout).not.toHaveBeenCalled(); + + const input = await screen.findByRole('textbox', { name: /view name/i }); + expect(input).toHaveValue('A'); + expect( + screen.queryByText(/are you sure you want to create a data link/i) + ).not.toBeInTheDocument(); + + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /^create$/i })); + + expect(checkout).toHaveBeenCalledWith(datasets, 'A'); + expect(onCreated).toHaveBeenCalledWith( + expect.objectContaining({ read_key: 'rk1' }) + ); + // Dialog closes on success; the hook does not own cart state, so it has + // nothing further to do here (see CartList/SelectionBar for cart + // clearing, which only happens where the cart was actually populated). + await waitFor(() => + expect( + screen.queryByRole('textbox', { name: /view name/i }) + ).not.toBeInTheDocument() + ); + }); + + it('opens the dialog with data-link consent copy when links are not automatic, and waits for Continue', async () => { + automatic = false; + const apiRef: { current: FlowApi | null } = { current: null }; + render(); + + act(() => { + apiRef.current!.startCreateView(datasets, 'A'); + }); + + expect(checkout).not.toHaveBeenCalled(); + expect( + await screen.findByText(/are you sure you want to create a data link/i) + ).toBeInTheDocument(); + + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /continue/i })); + expect(checkout).toHaveBeenCalledWith(datasets, 'A'); + }); + + it('keeps the dialog open and shows an error toast when checkout fails', async () => { + checkout.mockReset().mockRejectedValue(new Error('checkout failed')); + const apiRef: { current: FlowApi | null } = { current: null }; + render(); + + act(() => { + apiRef.current!.startCreateView(datasets, 'A'); + }); + + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: /^create$/i })); + + expect(checkout).toHaveBeenCalledWith(datasets, 'A'); + expect(toast.error).toHaveBeenCalledWith('checkout failed'); + expect( + screen.getByRole('textbox', { name: /view name/i }) + ).toBeInTheDocument(); + }); +}); diff --git a/frontend/src/__tests__/unitTests/viewCheckout.test.ts b/frontend/src/__tests__/unitTests/viewCheckout.test.ts index 48805149..ae148724 100644 --- a/frontend/src/__tests__/unitTests/viewCheckout.test.ts +++ b/frontend/src/__tests__/unitTests/viewCheckout.test.ts @@ -5,12 +5,14 @@ const encoded = (state: unknown) => encodeURIComponent(JSON.stringify(state)); vi.mock('@/omezarr-helper', () => ({ getOmeZarrMetadata: vi.fn(), generateNeuroglancerStateForOmeZarr: vi.fn(), - generateNeuroglancerStateForDataURL: vi.fn() + generateNeuroglancerStateForDataURL: vi.fn(), + generateStateForPlainZarr: vi.fn() })); import { getOmeZarrMetadata, - generateNeuroglancerStateForOmeZarr + generateNeuroglancerStateForOmeZarr, + generateStateForPlainZarr } from '@/omezarr-helper'; import { buildViewState } from '@/utils/viewCheckout'; @@ -18,6 +20,9 @@ const md = { multiscales: [{}], arr: {}, zarrVersion: 2 }; beforeEach(() => { (getOmeZarrMetadata as any).mockReset().mockResolvedValue(md); + (generateStateForPlainZarr as any) + .mockReset() + .mockRejectedValue(new Error('not a plain array')); (generateNeuroglancerStateForOmeZarr as any) .mockReset() .mockImplementation((url: string) => @@ -67,8 +72,31 @@ describe('buildViewState', () => { expect((ng_state as any).layers).toHaveLength(1); }); - it('skips a dataset whose metadata fetch throws and keeps the rest', async () => { + it('falls back to a plain-array layer when the dataset is not OME-Zarr', async () => { + // Both cart entries are bare Zarr arrays: OME metadata throws, plain-array + // generation succeeds, so each still contributes one layer. + (getOmeZarrMetadata as any).mockRejectedValue(new Error('not a group')); + (generateStateForPlainZarr as any).mockImplementation((url: string) => + encoded({ layers: [{ name: `${url}-plain` }] }) + ); + const { ng_state, layers } = await buildViewState([ + { url: 'a', sharing_key: 'ka', fsp_name: 'f', path: '/a', label: 'A' }, + { url: 'b', sharing_key: 'kb', fsp_name: 'f', path: '/b', label: 'B' } + ]); + expect((ng_state as any).layers).toHaveLength(2); + expect(layers).toEqual([ + { sharing_key: 'ka', layer_index: 0, channel: null, opts: null }, + { sharing_key: 'kb', layer_index: 1, channel: null, opts: null } + ]); + }); + + it('skips a dataset only when it is neither OME-Zarr nor a plain array', async () => { + // OME metadata throws AND plain-array generation throws (genuinely broken / + // moved / not a Zarr array) → drop just that entry, keep the rest. (getOmeZarrMetadata as any).mockRejectedValueOnce(new Error('gone')); + (generateStateForPlainZarr as any).mockRejectedValueOnce( + new Error('also gone') + ); const { ng_state, layers } = await buildViewState([ { url: 'a', sharing_key: 'ka', fsp_name: 'f', path: '/a', label: 'A' }, { url: 'b', sharing_key: 'kb', fsp_name: 'f', path: '/b', label: 'B' } diff --git a/frontend/src/__tests__/unitTests/viewStateByReadKey.test.tsx b/frontend/src/__tests__/unitTests/viewStateByReadKey.test.tsx new file mode 100644 index 00000000..c803c1b4 --- /dev/null +++ b/frontend/src/__tests__/unitTests/viewStateByReadKey.test.tsx @@ -0,0 +1,61 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { renderHook, waitFor } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import type { ReactNode } from 'react'; + +const { sendFetchRequest } = vi.hoisted(() => ({ sendFetchRequest: vi.fn() })); +vi.mock('@/utils', () => ({ + sendFetchRequest, + buildUrl: (base: string, seg: string | null) => `${base}${seg ?? ''}` +})); + +import { useViewStateByReadKey } from '@/queries/viewQueries'; + +const fakeResponse = (status: number, body: unknown) => + ({ + ok: status < 300, + status, + statusText: String(status), + json: async () => body + }) as unknown as Response; + +function wrapper({ children }: { children: ReactNode }) { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } } + }); + return {children}; +} + +beforeEach(() => sendFetchRequest.mockReset()); + +describe('useViewStateByReadKey', () => { + it('returns the raw ng_state on success', async () => { + sendFetchRequest.mockResolvedValue( + fakeResponse(200, { layers: [{ name: 'L0' }] }) + ); + const { result } = renderHook(() => useViewStateByReadKey('rk1'), { + wrapper + }); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(result.current.data).toEqual({ layers: [{ name: 'L0' }] }); + }); + + it('returns null on 404', async () => { + sendFetchRequest.mockResolvedValue( + fakeResponse(404, { detail: 'View not found' }) + ); + const { result } = renderHook(() => useViewStateByReadKey('rk1'), { + wrapper + }); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(result.current.data).toBeNull(); + }); + + it('is disabled without a read key', () => { + const { result } = renderHook(() => useViewStateByReadKey(undefined), { + wrapper + }); + expect(result.current.fetchStatus).toBe('idle'); + expect(sendFetchRequest).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/components/NeuroglancerView.tsx b/frontend/src/components/NeuroglancerView.tsx new file mode 100644 index 00000000..fe6b9fda --- /dev/null +++ b/frontend/src/components/NeuroglancerView.tsx @@ -0,0 +1,138 @@ +import { useEffect, useRef } from 'react'; +import { useParams } from 'react-router'; +import { Typography } from '@material-tailwind/react'; +import toast from 'react-hot-toast'; +import { + HiOutlineDuplicate, + HiOutlineDownload, + HiOutlineExternalLink, + HiOutlineArrowsExpand +} from 'react-icons/hi'; + +import { useViewStateByReadKey } from '@/queries/viewQueries'; +import { useInternalNeuroglancerBaseUrl } from '@/hooks/useDefaultNeuroglancerBaseUrl'; +import { constructNeuroglancerUrl } from '@/utils/neuroglancerUrl'; +import { downloadTextFile } from '@/utils'; +import { copyToClipboard } from '@/utils/copyText'; +import FgButton from '@/components/designSystem/atoms/FgButton'; +import FgIcon from '@/components/designSystem/atoms/FgIcon'; +import FgLink from '@/components/designSystem/atoms/FgLink'; + +export default function NeuroglancerView() { + const { readKey } = useParams(); + const stateQuery = useViewStateByReadKey(readKey); + const baseUrl = useInternalNeuroglancerBaseUrl(); + const containerRef = useRef(null); + const ngState = stateQuery.data; + + // Reflect the full state into the app's own URL hash so copy-pasting the + // current page URL is a full-state shareable link, matching Neuroglancer's + // own address-bar convention. + // ponytail: hash is display/share only; if it should ever drive editable + // state, that's the editable stack. + useEffect(() => { + if (!ngState) { + return; + } + window.history.replaceState( + window.history.state, + '', + '#!' + encodeURIComponent(JSON.stringify(ngState)) + ); + }, [ngState]); + + if (stateQuery.isPending) { + return ( +
+ Loading View… +
+ ); + } + + if (stateQuery.isError || !ngState) { + return ( +
+ View not found +
+ ); + } + + const title = (ngState.title as string) || 'Untitled View'; + const externalUrl = constructNeuroglancerUrl(ngState, baseUrl); + + const handleCopy = async () => { + const shortLink = `${window.location.origin}/view/${readKey}`; + const result = await copyToClipboard(shortLink); + if (result.success) { + toast.success('Neuroglancer link copied'); + } else { + toast.error(`Failed to copy: ${result.error}`); + } + }; + + const handleFullscreen = () => { + // ponytail: native Fullscreen API on the container, deliberately scoped + // to this component (excluding the app navbar) rather than the whole + // route; no custom fullscreen state machine. + void containerRef.current?.requestFullscreen?.(); + }; + + return ( +
+
+
+ + NG Views + + / + + {title} + +
+
+ + {title} + +
+ void handleCopy()} variant="ghost"> + Copy link + + + downloadTextFile( + JSON.stringify(ngState, null, 2), + `${title}.json` + ) + } + variant="ghost" + > + Download JSON + + + window.open(externalUrl, '_blank', 'noopener,noreferrer') + } + variant="ghost" + > + Open external + + + Fullscreen + +
+
+
+