feat: Neuroglancer Views embedded viewer (PR 6, ngviews-06-embedded-readonly) - #428
Draft
allison-truhlar wants to merge 20 commits into
Draft
feat: Neuroglancer Views embedded viewer (PR 6, ngviews-06-embedded-readonly)#428allison-truhlar wants to merge 20 commits into
allison-truhlar wants to merge 20 commits into
Conversation
Also stub useCreateViewFlow in FileBrowserCartItem/FileTableSelectColumn tests: FileBrowser now calls the hook unconditionally, and those tests' render trees don't set up a ViewsProvider (only the cart/select-column behavior is under test there).
FileBrowser's "View in Neuroglancer" row action (and SelectionBar's "New View from selection", latent since 5a) call useCreateViewFlow -> useCartCheckout -> useViewsContext, but ViewsProvider only wrapped the /ngviews route, so /browse threw ErrorFallback on every load. Move ViewsProvider into MainLayout's app-wide provider stack, alongside CartProvider, and drop the now-redundant route-scoped wrapper.
The checkout pipeline required OME-Zarr multiscale metadata for every cart dataset: getOmeZarrMetadata threw on a bare array, the entry was dropped, and the data-URL fallback was dead code that never ran. Manually adding array directories (affs, lsds, seg, ...) to the cart produced a 0-layer View and an error popup on channel expand. generateStateForDataset now falls back to a single plain-array layer (new generateStateForPlainZarr, probing v2 then v3) when OME detection fails, and getOmeZarrChannels returns [] instead of throwing for a plain array.
…'s picker Plain-array fallback emitted type:'new', which forced Neuroglancer's layer-type picker on open and rendered layers as raw grey. generateStateForPlainZarr now opens the array and picks a type from name+dtype (integer dtype + seg/label/mask name -> segmentation, else image) via generateNeuroglancerStateForZarrArray, so seg renders as segmentation and images resolve without a manual pick. Multi-channel arrays (affs/lsds) and a per-layer override UI are deferred to follow-up PRs; recorded in the plan.
…d rename The useCreateViewFlow refactor conflict (resolved during the ngviews-07 rebase) renamed the hook's returned consentDialog/consentOpen to dialog/open, since the dialog now always renders (name input + optional consent step) rather than only for the consent-needed case. - FileBrowser's row action still destructured the old field names. - Three test mocks (FileBrowserViewInNg, FileBrowserCartItem, FileTableSelectColumn) still stubbed useCreateViewFlow() with the old field names; none assert on those fields directly, so this was inert drift, not a failure, but left the mocks out of sync with the real hook's shape. Also picks up prettier's line-wrap for the consent copy.
allison-truhlar
force-pushed
the
ngviews-06-embedded-readonly
branch
from
August 12, 2026 19:29
7832a13 to
5a58365
Compare
/view/:readKey no longer opts out of MainLayout's navbar (the sole reason
isBareLayoutPath existed), and NeuroglancerView now shows an "NG Views ▸
{title}" breadcrumb back to the views list. Also retitles the no-title
fallback to "Untitled View" so it doesn't read as a duplicate of the
"Neuroglancer" chrome the breadcrumb already establishes.
…he cart runCheckout previously navigated cart-driven checkouts to /ngviews and never cleared the persisted cart, leaving stale items behind after a successful View creation. Cart callers (CartList, SelectionBar) now pass onCreated to jump straight to /view/:readKey, and clearCart() fires after every successful checkout regardless of caller.
A clearCart() rejection was falling into the same catch as checkout(), showing a false "Checkout failed" toast and leaving the dialog open with request still set after the View had already been created - risking a duplicate View on retry. clearCart() now runs after onCreated/navigate/setRequest(null) and is fire-and-forget with its own .catch, so a cart-clear failure can no longer reopen the checkout-failure path.
Once the view state loads, replaceState the app's own hash to the same url-encoded state used for the external Neuroglancer link, so copying the current page URL is a full-state shareable link. Copy link now copies window.location.href instead of the external instance URL. Display/share only: readKey stays the source of truth, hash is never read back.
Matches the brief's guidance: hash write is display/share only, not a state-restoration mechanism.
…-state hash clearCart() was called unconditionally inside useCreateViewFlow's runCheckout, but two of its three callers (SelectionBar "New View from selection", FileBrowser "View in Neuroglancer") build their request from something other than the persisted cart. Creating a View from either silently wiped whatever a user had curated in their Layer Cart. Move the clearCart() call into CartList's onCreated, the one caller that actually owns cart state. Also stop NeuroglancerView's "Copy link" from copying window.location.href, which now includes the full ng_state as a URL hash (10-14KB after encoding). RequireAuth re-encodes the full current path into a `next` query param when redirecting to /login, so an unauthenticated colleague opening a copied link would hit a header-size error instead of the login page. Copy the canonical `/view/:readKey` short link instead. Bundled in the same file: preserve window.history.state (was passing null, which drops React Router's scroll-restoration bookkeeping) in the hash-reflection replaceState call, drop the now-redundant `readKey` from that effect's deps (it can't change without ngState changing), and fix the handleFullscreen comment's stale "/view route is chrome-less" claim now that the route has a navbar.
…s trigger
startCreateView always opens the dialog now (no more silent-immediate
checkout path), so the trigger button only unmounted while `open` was
true - exactly when `pending` could also be true. `disabled={disabled ||
pending}`, `loading={pending}`, and `loadingText` were unreachable, and
`open` existed on useCreateViewFlow's return solely to feed that check.
FgDialog is a modal overlay, so leaving the trigger always rendered is
safe - the dialog's own Create/Continue button already has a working
loading state.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Neuroglancer Views — PR 6 (
ngviews-06-embedded-readonly)Stacked on #427 (
ngviews-05b-browser-chrome). Read-only scope. Final PR of the read-only stack — Views now open embedded inside Fileglancer instead of hopping to an external Neuroglancer tab. Frontend-only: the public read endpointGET /ngview/{read_key}(rawng_stateJSON) already shipped in PR 2.Every entry point shares one shape: a saved View's
read_key→ client route/view/:readKey→ fetch state → iframe NG with inline state (no CORS dependency).What's in it
useViewStateByReadKey(readKey)— fetches a View'sng_statefrom/ngview/{key}(404 →null), mirroring the repo's manual-status-branch fetch pattern.NeuroglancerViewpage — chrome-less, full-viewport<iframe>atconstructNeuroglancerUrl(ng_state, baseUrl)+ a thin top bar: Copy link · Download JSON · Open external · Fullscreen, with loading and "View not found" states./view/:readKey— underMainLayout+RequireAuth, rendered chrome-less via a newisBareLayoutPathguard on the existingbareflag. Client path is/view(kept distinct from the backend/ngview/{key}JSON endpoint);/ngviewadded to the dev proxy for same-origin fetch./view/:read_key(waswindow.opento external NG)./view/:read_key.⋯"View in Neuroglancer" (single dataset) → creates a Data Link + View (consent-gated, exactly like today's data-link flow) then opens it embedded. A peek creates a lightweight saved View — no ephemeral scratch path.useCreateViewFlowextracted fromCreateViewButton— one consent-gated checkout flow reused by the cart button and the new row action (no duplicated consent dialog).useInternalNeuroglancerBaseUrlresolves the deployment'sconfigured(Internal) NG template and the embedded viewer uses it, ignoring the user's Internal/External viewer-URL preference. The "External" preference resolves toneuroglancer-demo.appspot.com, which sendsX-Frame-Optionsand won't render in an iframe; the configured/Internal deployment (e.g.fileglancer.int.janelia.org/neuroglancer) is same-origin and frames fine. Portable — no hardcoded URL. External/quick-view surfaces (Saved Views Copy/Open-external, the file-browser quick-view icon, NG short links) keep honoring the preference via the unchangeduseDefaultNeuroglancerBaseUrl.Notable review catches (fixed on-branch)
ViewsProviderhoisted app-wide intoMainLayout(was scoped to/ngviews). The row action's checkout hooks run on/browse, which had noViewsProvider—/browsewould have renderedErrorFallback. Root-caused at the provider (mirrors 5a'sCartProviderhoist); the redundant route-scoped wrapper was removed. This also cured a latent 5a bug (the SelectionBar'sCreateViewButtonhad the same unmet dependency).Deferred / carry items
configured) NG, so at Janelia it usesfileglancer.int.janelia.org/neuroglancer(frameable) regardless of the user's preference. For embedding to render, a deployment must configure an Internal NG (viewers.config.yamlinstance_template_url); where none is configured, the base falls back to the appspot default, which won't frame ("Open external" still works). This is the design's "embedded NG hosting" question, now scoped down to "the deployment needs a frame-permitting Internal NG."/view/:readKeyrequires auth for now (per-user viewer-URL preference); true unauthenticated/public viewing +sharing_modeenforcement is the next (editable) stack — the backend already anticipates it.ViewsContext's Views-list query now fires app-wide rather than only on/ngviews(cheap/cached; gate it if page-load request volume matters).Testing
viewStateByReadKey,NeuroglancerView,isBareLayoutPath,ngViewsColumns(Open→navigate),AppearsInViews(links),useCreateViewFlow,FileBrowserViewInNg.node-prettier-check+node-eslint-checkclean (one pre-existing unrelated lint warning).node-checkshows the stack's 5 pre-existing type errors in untouched files — reconcile at the stack level.