feat(menu): brand viewer menu and NVDocument save/load split button#250
Merged
Conversation
Replace the standalone Home button with a brand dropdown, and turn the
"Save Scene" button into an NVDocument Save/Load split button.
- The niivue logo and wordmark is now a dropdown on standalone hosts
(web, desktop): Reset Viewer (the old Home action) and About. The
About dialog covers the app's purpose, NiiVue and NeuroDesk credits,
the data privacy note, and the build version linking to its commit.
- The brand stays a static logo on embedded hosts (VS Code, and
Streamlit which sets menuItems.home: false), so their behavior is
unchanged. The existing `home` flag is repurposed to gate the
dropdown, avoiding host config churn.
- "Save Scene" is renamed NVDocument and gains a chevron: clicking the
label still saves the active scene as a .nvd by default, while the
dropdown offers Save and a new Load (a .nvd file picker).
- Hosts can pass an optional appInfo ({ version, buildDate, repoUrl })
to Menu; the PWA wires in its build-time git metadata. The About
dialog omits the version line when no appInfo is supplied.
New unit tests cover default-save, Save/Load wiring, brand-menu open,
and About opening the dialog; the two PWA e2e specs are updated to match.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
🚀 PWA Preview DeploymentYour PWA preview has been deployed! Preview URL: https://niivue.github.io/niivue-vscode/pr-250/ This preview will be updated automatically when you push new commits to this PR. |
…tton
The brand is now a <button> with accessible name "niivue Viewer", so the
overflow spec's getByRole('button', { name: 'View' }) (default substring
match) resolved to two elements (the View menu and "Viewer") and failed
strict mode. Make that assertion exact, and drop the stale "Home" mention
from a comment now that Reset Viewer lives in the brand menu.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Coverage ReportOverall line coverage: 42.8% (+0.7) vs
|
Adds direct unit coverage for AboutDialog's appInfo handling, the one branchy part of the new brand menu that lacked a test: version + repoUrl builds a commit URL, version alone falls back to the default repo link (no build date), and no appInfo omits the version line entirely. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The brand menu's About dialog already carries the data-privacy guarantee and the NiiVue/NeuroDesk credits, so the duplicate "Data Privacy" section (privacy note + credits paragraph) is removed from the home screen. The version footer is kept. HomeScreen unit tests (bookmarklet/MNI links) and the @dom e2e tests still pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sktop privacy Answers "why two home screens?": they're the empty-state landing for the two standalone hosts (PWA, desktop); embedded hosts (VS Code, Streamlit, Jupyter) load content directly and render none. The copy is genuinely host-specific, so keep two thin home screens but stop duplicating the presentation. - New shared `HomeSection` (titled block) in @niivue/react; both PWA and desktop home screens compose it instead of repeating the heading/body classes. Both apps' tailwind configs already scan the package source, so the utility classes resolve. - Drop the PWA version footer (now in the About dialog). - Drop the desktop "Data Privacy" section (now in About, same as the PWA earlier); its unit test asserts the intro + formats and that the privacy note is gone. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…the build HomeScreen now composes the shared HomeSection from @niivue/react. The PWA's vitest config (unlike its vite config) doesn't alias the package to source, so the test pulled in the real package - which in CI resolved to niivue-react source, failed to render the bookmarklet/MNI links, and tripped a coverage parse error on NiiVueCanvas. Mock @niivue/react with a HomeSection stub, as the desktop home-screen test already does, so this stays a self-contained unit test independent of build order and module resolution. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
🧹 PWA Preview CleanupThe preview deployment for this PR has been removed. |
korbinian90
added a commit
that referenced
this pull request
Jun 19, 2026
Reconcile #250 (brand menu + NVDocument Save/Load split button) and #251 (WebGL2 context-error hint) with the v1.0 core migration. Resolution (Menu.tsx conflicts + the auto-merged document path, kept on the v1 API): - Keep #250's "NVDocument" Save/Load split button, but Save uses nv.serializeDocument() (v1 CBOR), not the removed nv.json(). The dropdown becomes Save / Save as JSON / Load, combining #250's Load entry with the migration's JSON export. - events.ts and Volume.tsx auto-merged; #250's loadDocumentEvent coexists with the v1 ExtendedNiivue, loaders, and event wiring. - Migrate #250's new MenuBrandDocument.test.tsx to the v1 niivue surface (default NiiVueGPU export, serializeDocument, header-based metadata) and add a Save-as-JSON assertion. turbo type-check 8/8; @niivue/react 157 unit tests pass; react + pwa build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
korbinian90
added a commit
that referenced
this pull request
Jun 19, 2026
#250's Document.spec.ts asserted the old 0.68 JSON .nvd shape (JSON.parse + encodedImageBlobs), which the v1 migration replaces with CBOR (the bytes from nv.serializeDocument). Rewrite the assertions to be format-correct: the facade getDocument() and the UI download are non-empty CBOR (not JSON), and the export -> drop -> re-import round-trip reconstructs the scene (the round-trip assertion is format-agnostic). Validated by the project type-check; the WebGL lane runs in CI (no local GL). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
korbinian90
added a commit
that referenced
this pull request
Jun 20, 2026
…#252) * feat(react): migrate NiiVue core to @niivue/niivue v1.0 (niivue/mono) Upgrade the NiiVue core from 0.68.2 to the v1.0 release candidate (@niivue/niivue@1.0.0-rc.9), a WebGPU/WebGL2 rewrite from the new niivue/mono monorepo. Almost all direct niivue usage is centralized in @niivue/react, so the apps inherit the migration through that package. Foundation: - Pin @niivue/niivue to 1.0.0-rc.9 across all manifests - docs/niivue-v1-migration.md: full old->new API mapping + phased plan - scripts/migration/niivue-v1-scan.mjs: read-only old-API scanner Core package (@niivue/react + @niivue/viewer-protocol): - Niivue -> default export NiiVueGPU; setter methods -> accessor properties; callbacks -> addEventListener events - Loaders rewritten to addVolume/loadVolumes/addMesh with File-wrapped buffers; NVMeshLoaders.readLayer -> addMeshLayer - Delete the ExtendedNiivue.mouseMoveListener sync override and rely on the now-public broadcastTo for 2D/3D cross-canvas sync - Documents: niivue-native CBOR (serializeDocument/loadDocument), plus a JSON form (nvd-json.ts) so .nvd/.nvd.json scenes can be authored in an editor and viewed directly; "Save Scene" gains a "Scene as JSON" export Tests: reworked Menu/document tests; new characterization tests for the risky transforms and the JSON<->CBOR transcoder. type-check, build, and 146 unit tests pass; PWA/Tauri/Jupyter type-check clean. Pending (follow-up, not in this change): manual PWA confirmation of load paths and the .nvd round-trip; Phase 2 apps (streamlit direct usage). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(streamlit): migrate direct niivue usage to v1.0 API (Phase 2) The Streamlit frontend hook uses the niivue core directly (the only app that does). Migrate it to @niivue/niivue v1.0: - Drop `import { Niivue }`; instances infer as ExtendedNiivue - removeVolume(obj)/removeMesh(obj) -> nv.model.removeVolume/removeMesh(i) + updateGLVolume() (index-based synchronous model ops) - Clear mesh layers via removeMeshLayer(0, i) so mesh colors recomposite (mutating mesh.layers directly leaves stale overlay colors baked in) - The voxel-click feedback used `nv.onLocationChange = fn`, which is dead in v1 (cast through `as any`, so type-check missed it). Switch to addEventListener('locationChange', ...); extract the payload builder to buildVoxelClickPayload and unit-test it. All apps (PWA, Tauri, Jupyter, VS Code, Streamlit) now type-check and build against the migrated core: turbo type-check 8/8, TypeScript test suites pass, syncpack clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(pwa/e2e): update .nvd Document e2e for the v1 CBOR format #250's Document.spec.ts asserted the old 0.68 JSON .nvd shape (JSON.parse + encodedImageBlobs), which the v1 migration replaces with CBOR (the bytes from nv.serializeDocument). Rewrite the assertions to be format-correct: the facade getDocument() and the UI download are non-empty CBOR (not JSON), and the export -> drop -> re-import round-trip reconstructs the scene (the round-trip assertion is format-agnostic). Validated by the project type-check; the WebGL lane runs in CI (no local GL). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(react): correct overlay URL-load; align e2e with niivue v1 behavior Fix the three failures the e2e WebGL lane caught (reproduced and verified locally with WARP WebGL): - addOverlay built `url = ''` for a URL-load overlay: the empty-string data case fell through `typeof data === 'string'`, so niivue threw "prepareVolume requires a url or an NVImage object" and the overlay never loaded. Fall back to item.uri (the pre-migration `item.data || item.uri` semantics) for both the volume and mesh branches; add a regression unit test. - WithMessage cal_min/cal_max: niivue v1's default window is the full data range [0, 3] (0.68 used a ~2% robust max ~2.001). Assert v1's value. - probe-mhd: niivue v1 computes a null x-axis affine for a transform-less MHD (sphere.mhd has only ElementSpacing), so the crosshair POS/VAL readout is NaN (upstream limitation). Verify the detached .raw voxels loaded (img length, global max) instead of the affine-dependent readout. Full e2e WebGL lane 38/38; react unit 158 pass; type-check + lint clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(react): restore left-button crosshair after niivue v1 migration niivue v1 turned crosshair movement into its own DRAG_MODE and made it the default for the primary (left) mouse button. The migration mapped the old `dragMode: contrast` to `primaryDragMode: DRAG_MODE.contrast`, which suppressed crosshair movement on left-click/drag (a regression from 0.68, where a left click always moved the crosshair). Set the primary drag mode to `crosshair` in both the ExtendedNiivue constructor (growNvArrayBy) and the Menu zoom-mode effect (applyDragMode); windowing stays on the right button via niivue's default secondaryDragMode. Update the affected unit tests and DRAG_MODE mocks. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(react): stop canvas presses from starting a pane reorder drag The volume pane is draggable for reorder. niivue 0.68 called preventDefault on canvas pointerdown, which kept that native drag confined to the top drag-handle strip. niivue v1 no longer suppresses it, so a press on the canvas (e.g. a double-click or a trackpad drag gesture) started a native card drag, dragging a "screenshot" of the whole pane instead of moving the crosshair. Cancel the drag in handleDragStart when it begins over the canvas so canvas presses reach niivue and only the chrome reorders. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(react): drop migration scaffolding before merging to main Remove the one-off migration aids that have no value once the v1.0 migration lands on main: - scripts/migration/niivue-v1-scan.mjs: a read-only old-API scanner whose only job was tracking migration progress ("when it prints nothing, the source is clean") - it permanently reports nothing post-merge. Nothing in CI or the package scripts invokes it. - docs/niivue-v1-migration.md: the phased-plan / status / scanner-usage guide, tied to the migration moment (and the only file in docs/). The durable knowledge lives in the PR history; the changeset still summarizes the break. Drop the now-dangling doc reference from the changeset. No product code or test imports either file, so the type-check/build/test gates are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.
What
Two menu-bar changes.
Brand "viewer" menu (replaces the Home button)
menuItems.home: false), so their behavior is unchanged. The existinghomeflag is repurposed to gate the dropdown, so no host config changes were needed.appInfoprop ({ version, buildDate, repoUrl }); the PWA wires in its build-time git globals. The dialog omits the version line for hosts that supply none.NVDocument save/load split button
MenuItempattern:.nvdby default (unchanged behavior)..nvdfile picker (loadDocumentEvent), complementing the existing drag-and-drop import.Why
The top bar had a redundant Home button and a single-purpose Save button. Folding Reset into the brand frees bar space and gives the logo an obvious affordance, while the NVDocument split button surfaces Load alongside Save now that
.nvdimport/export exists (VHP Phase 1a).Tests
@niivue/react: type-check, lint, and 130 unit tests pass, including a newMenuBrandDocument.test.tsx(default-save, Save/Load wiring, brand-menu open, About opening the dialog).@niivue/pwa: type-check, lint, unit tests, and production build all pass.tauri/streamlit/vscodetype-check clean (all consume@niivue/react).Document.spec.tsclicks theNVDocumentbutton;Menu.spec.tsdrops theHomeassertions and adds a brand-menu/About check). e2e was not run locally (the WebGL lane is unreliable on this Windows setup); the new brand test is@dom-tagged for the fast CI lane.Notes for reviewers
.changeset/brand-menu-nvdocument.md):@niivue/reactminor,@niivue/pwaminor, the other apps patch.homeflag off orisVscode).🤖 Generated with Claude Code