Repository navigation
feat(ui): replace Ant Design with the Slim v2 design system - #451
Open
igoroctaviano wants to merge 212 commits into
Open
igoroctaviano wants to merge 212 commits into
igoroctaviano wants to merge 212 commits into
Conversation
Move the whole app from Ant Design and Less to Tailwind, Radix primitives and Material Symbols, following the Slim Modern v2 mockups in light and dark mode. - Header with study breadcrumb, server pill, debug, tag browser and user menu - Worklist with search, date filter, grid table and pagination footer - Viewer with collapsible side panels, centered tool pill, zoom, scale and overview overlays, and a status footer with tile and memory usage - Right panel sections for label, specimens, equipment, optical paths, presentation states, annotations, groups, categories, segmentations and parametric maps, with per-section display options replacing the settings drawer - Dialogs rebuilt on a shared header and footer - Jest now uses the automatic JSX runtime, matching tsconfig and the build
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| JavaScript |  |
Oct 26, 2026 5:10p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
…ndings Pure, unit-tested modules now hold the DICOM parsing, formatting, filtering, persistence and viewer math that lived inside components. Components render props and call callbacks. Fixes: - annotation group measurement codes containing hyphens were truncated - clustering settings moved to annotation groups; threshold accepts decimals - annotation modal fields are controlled and evaluations can be cleared - worklist no longer jumps to page 1 during modality enrichment - free-text search is no longer persisted (may contain patient data) - preferences apply live; stroke style and confirm-before-remove are wired - config masking covers authorization/bearer/credential keys - tile counts are scoped to the active volume and count failures - cursor readout shows slide coordinates in mm, matching Go to - save/remove/ICC messages surface as toasts instead of console logs - restored show/hide all, partial category state and optical path stains - slider thumbs, preference controls and pagination have accessible names
- Draw new ROIs with the user's stroke color and width, store styles per ROI so finding styles and re-renders no longer override them - Convert lengths and areas to the preferred unit in the annotation list and ROI details - Remove the keydown listener on unmount - Show the viewer layout with skeletons while the study loads and a centered indicator until the first slide tiles arrive - Replace the worklist spinner with skeleton rows, drop the patient name icon and align the page padding with the header
- Move measurement units to the Annotations tab and drop the unsupported ROI label option - Widen the configuration search, highlight matches and show a count - Redesign the About tab with links and copyable build information - Show the full server URL on hover and add a copy button to the server dialog - Hide the Worklist button on the worklist - Open dialogs from the center instead of sliding from the corner - Replace the S tile with the microscope mark and regenerate favicons
Port #438 into the redesigned UI instead of picking a side: - Server dialog: optional OIDC config section with validation; the OIDC logic lives in useServerSelection (covered by hook tests) - Viewer: Tailwind ViewerMessage for load errors (with retry), "No slides found" and "Series not found"; study loading keeps the skeleton layout and series loading uses the viewport indicator - Segments: absent segments get an "Absent" badge with zoom, color, visibility and opacity controls disabled
📦 Firebase Preview - Using Published DMVThis preview is using the published
|
|
Visit the preview URL for this PR (updated for commit fcbd187): https://idc-external-006--pr451-feat-slim-modern-red-qeu2j1mu.web.app (expires Mon, 02 Nov 2026 17:10:44 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 88aacecd98ba54d2f9c8d201a9444e43d1ad8307 |
- typescript 6.0 (the last release with the JS API that CRA's type checker needs), ES2022 target, explicit types list, ignoreDeprecations for the moduleResolution CRA forces - copy the tailwind/postcss/babel configs into the Docker image so the build runs Tailwind; the container test now fails on unprocessed CSS - drop unused deps (classnames, react-test-renderer, user-event v7, sonarqube-scanner, @types/react-router-dom, three Radix primitives) and their dead ui wrappers; build tools move to devDependencies - add lucide-react - remove the PostCSS block craco ignores, CRA's duplicate ESLint pass, the duplicate .npmrc and two type files for removed libraries - lint test files with Biome, run tests in UTC, target current Node in Babel - browserslist targets WebGL2-capable browsers, which DMV requires
- restore the comments documenting the token-disclosure policy (configured origins, consent gate, insecure-origin refusal, sibling-manager grants, unforwarded token on server switch, GH-320 mapping, auth recovery remount) - a server URL stored before the mode key existed is applied as custom, as on master; App reads it through loadServerSelection so the dialog and the active client agree - drop eslint suppressions for rules no configured linter runs
- preferences dialog, hooks and utils move to features/preferences with a public index; SlideViewer, AnnotationItem and the worklist no longer reach into the header feature - delete the components/Header and components/Worklist shims, the unused ui and slim barrels; move the worklist test next to its component - SlimCollapsibleSection uses the ui/collapsible wrapper - drop eslint suppressions for rules no configured linter runs
- Icon renders typed lucide-react SVGs (IconName union); the Google Fonts Material Symbols and IBM Plex requests are gone, so icons work offline - shared CopyButton and SearchInput; Button gains xs/page sizes and a selected variant; Slider forwards onValueCommit; SlimDialogHeader gains a tone; visible focus rings on dialog close buttons - Tailwind tokens for the recurring arbitrary sizes and radii; cn() merges the custom tokens; unused tokens/keyframes and CSS utilities removed - preferences dialog split into one component per tab with the WAI-ARIA tabs pattern, runtime info injected, config view memoized; usePreferences caches its snapshot - DICOM tag browser selects by series UID and derives display sets with a pure buildDisplaySets, so metadata events no longer reset the selection - worklist: "Last 7 days" covers 7 days, legacy dotted dates parse, page index is clamped, empty state reflects filters, deferred search, table roles, patient name is a link - OIDC config storage goes through the safe storage helpers; notifications keep the original error object; one keyboard shortcut table
- every sidebar panel is a function component; Item folds into SpecimenItem - components/panel holds the shared layer controls: visibility toggle, settings popover, inline opacity row, useLayerStyle, useLimitWindow, useResettableState and a frame-throttled callback - sliders preview locally and commit on release, except optical path windowing, which streams to the viewer once per animation frame - report parsing, measurement lookups, colour, swatch, visibility summary, display option binding and style equality move to tested pure utils - shared layer style types in types/layerStyles
- toasts publish through a small store rendered by ToastHost, so NotificationMiddleware TOAST messages and config.messages show again - ROI styling restored: selected finding colours, measurement styles - DMV and DOM listeners are registered through subscribe helpers and torn down on unmount; debounced handlers are cancelled - SlideViewer render is split into section components; go-to, ROI removal, pixel statistics, ICC profile, presentation state, hovered ROIs and referenced slide logic move to tested utils - typed dicom-microscopy-viewer events and styles; fewer unsafe casts - AppInfo is built once; reports encode through encodeDicomDataset - App renders through a single renderPage helper; the viewport no longer forces a minimum width
- DICOM date/time, person name and coded concept helpers live under utils/dicom; displayFormat reuses them; values.ts, formatDicomDate.ts and the old dicomDate/personName modules are gone - magnification prefers Objective Lens Power and otherwise uses geometric midpoints between objectives, so 0.25-0.26 um/px slides read 40x, not 20x - study description is filled from the first instance when empty (the check was inverted) - impossible DICOM dates are returned as-is instead of rolling over - mapWithConcurrency runs serially for NaN or sub-1 limits - memory warning thresholds have one source in memoryWarning.ts - display option descriptors are generic over their ids; the unused segment clustering branch is removed - colour conversions live in color.ts; segment colour helpers take narrow object params; sr.tsx becomes sr.ts without duplicate helpers; CustomError is TypeScript - tests for viewerUtils, overview clamping, cn(), study metadata, overlay colormaps, SR helpers and CustomError
- viewer shortcut hints are built from KEYBOARD_SHORTCUTS and shown with formatShortcutHint - CaseViewer redirects a bare study URL to its first series, replacing SlideList's mount-only effect and its lint suppression - bulk visibility and category recolouring update SlideViewer once through batch handlers; default ROI and panel style maps keep their identity while unchanged - StyleOptions is replaced by AnnotationStyle - series path, series label, optical path order, ROI selection and visibility change logic move to tested utils - dcmjs naturalize/denaturalize are typed, removing casts at call sites - icon props use IconName; a global ResizeObserver stub replaces the per-test ones; remaining line comments use block style
- CONTRIBUTING covers lucide icons, Tailwind token registration, the panel kit and publishToast - README: toasts stack bottom-centre unless messages.top is set; test script description is accurate; typecheck script listed - CONFIGURATION: legacy stored server URLs stay custom; read-only pill - toasts default to 5 seconds again, as documented
- App is a small function component; session setup, client mapping, token disclosure, routes and pages live in src/app with tests. The auth rationale comments move next to the code they describe - oidc-client 1.x is replaced by oidc-client-ts 3. Authorization code with PKCE stays the default; the implicit grant that Google Cloud Healthcare clients need is implemented in auth/implicitGrant, verifying the ID token signature, at_hash, issuer, audience/azp, nonce and expiry, with single-use state per flow. OIDC sign-in now needs a secure context - silent renew must return the same subject; sign-out revokes only when the provider has a revocation endpoint - the bare study URL redirect keeps the query and hash (?gcp=, ?access_token)
- SlideViewer is a thin function component over focused hooks: viewer session lifecycle, DMV/keyboard subscriptions with latest handlers, slide population, optical paths, presentation states, ROIs, annotation draft and tools, reports, annotation groups, segmentations and parametric maps - viewers are created in an effect and always torn down; results that arrive after teardown are dropped instead of leaking into the next slide - segment palettes rebuild only when segments, colours or gamma change - finding keys use codedConceptKey (SCHEME:VALUE); buildKey is removed - unread class state (config measurements, duplicate clustering handlers, an unreachable pixel statistics branch) is dropped - reducers, style registry and session services are pure and tested; a SlideViewer smoke test and hook tests cover lifecycle and cleanup
- Vite replaces CRA/craco: root index.html, CRA placeholders (%PUBLIC_URL%, %REACT_APP_CONFIG%) filled by a small plugin, base from PUBLIC_URL, REACT_APP_* still exposed, output stays in build/ - dicom-microscopy-viewer's dynamic-import bundle is pre-bundled and its worker and codec files are served from static/js in dev and build; linked DMV checkouts re-optimize when their bundle changes - Vitest replaces Jest with the same jsdom setup and mock reset semantics - React 19.3 with the React Compiler (babel-plugin-react-compiler 1.0); check:compiler fails on any component or hook the compiler skips and runs in CI and pre-commit; WorklistTable opts out because TanStack Table is an incompatible library - react-router 8, react-error-boundary 6, Testing Library 16, TypeScript 7 - IBM Plex fonts are bundled with @fontsource - CRA, craco, Babel presets, ESLint and overrides that only applied to CRA's dependency tree are removed; Biome is the only linter - Dockerfile, CI and docs follow the new toolchain; .env now controls REACT_APP_CONFIG (default local)
- @tailwindcss/vite compiles a CSS-first @theme in src/styles/globals.css; tailwind.config.js, postcss.config.js and PostCSS are gone - dark mode is a class-based custom variant; tw-animate-css replaces tailwindcss-animate - v4 renames applied (outline-hidden, shadow-xs, rounded-sm, important suffix, linear gradients kept in sRGB, data-/has- shorthands); bare arbitrary sizes become the existing tokens - base styles keep v3 behaviour the UI relied on: pointer cursor on buttons, 1px table cell padding; OpenLayers CSS sits in the base layer - tailwind-merge knows the custom shadow, sidebar and bar-height tokens - computed styles of the worklist, viewer and dialogs match the v3 build in light and dark themes - CI rejects unprocessed Tailwind at-rules in the built CSS
scripts/profile-bulk-annotations.mjs drove Ant Design selectors that no longer exist and imported Playwright, which is not a dependency. Block comment style in inject-slim-env.mjs.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Memory formatting and warning levels move to utils/memoryWarning with tests, and the unused MemoryMonitor status helpers are removed. Also replaces for-in loops, dedupes the Pyramid UID errors (fixing a missing space in one message) and tidies the implicit grant helpers.
Check the configuration before mounting and show which server lacks a URL and which setting to change. The crash fallback is now a full page with the message, a reload button and the details dialog. The dev server serves config/env.js from its own environment, so starting another server or a build with a different config no longer breaks it.
…remounts DMV's style getter reports neither fill nor its opacity, so they are kept per group, like the measurement a group is colored by, and a collapsed and reopened section shows the fill the slide still draws.
The headroom above a full window no longer pushes the slider past the value the limits are clamped to.
Radix dialogs set no aria-modal; an open modal traps focus, so the check on the key's target already covers it, and the test now uses dialogs as the app renders them.
Accession and Study ID sort alphanumerically, names and sex as text, dates, times and slide counts as plain values, so a small search result sorts like a full page.
The study list and the server search results record which server they came from, so the frame right after a server switch shows loading instead of the previous server's rows.
igoroctaviano
added
run-e2e
A deep link can select a SEG or ANN series whose items never reached the viewer, such as one that does not match the slide; the list then shows the loaded series instead of staying empty with no picker to leave the filter.
igoroctaviano
added
run-e2e
- Keep other URL parameters when picking or resetting a presentation state - Do not rebuild the viewer when a mangled series UID is corrected - Do not restart a derived-series lookup on query changes - Look derived series up on every configured store - Forget held keys when the window loses focus - Notify a 500 once, whatever the number of configured messages - Reject zero magnification in Go To - Send wildcard worklist queries only when exact ones find nothing - Restore the itemsForSeries doc comment
- Find derived series by search before retrieving, so stores without it do not reach the app's error handling; remember the store that has it - Settle a failed derived-series lookup as not found - Send exact and partial worklist queries together, exact matches first - Only undo trailing .0 mangling in routes, so derived series named after a slide series are looked up - Correct a mangled series UID in the first URL too - Use the route's image slide while rendering, so a slide switch does not build viewers for the previous slide - Add a history entry for presentation state picks and show the URL's state after Back and Forward - Drop sign-in callback parameters when writing the state parameter
- Never write sign-in callback parameters or the hash back to the URL, and ignore the OAuth state of any callback as a presentation state - Search derived series on every client together, skipping one whose search fails, and retrieve from the first in order that has it - Remember the first configured store with a series, not the fastest - Ignore Back and Forward to another route in the presentation states - Add no history entry for the state already shown - Match the slide list highlight like the viewer route - Remove an unreachable rejection handler
- Keep other URL parameters as written when setting the state, and add no history entry when the URL does not change - Treat sign-in error redirects as callbacks, and drop an implicit response from the hash instead of every hash - Detect Back and Forward to another route without relying on loads - Look derived series up one client at a time again, so stores after the one with the series are never contacted - Highlight the slide a derived series route resolved to - Match the viewer's series UID like the route - Prefer the first configured store with a series across searches - Send wildcard worklist queries only when exact ones find nothing
- Resolve the route's slide in the study view, so the slide list and the viewer share it: no highlight while a derived series is looked up or when it is not found - Keep the store a retrieve found working instead of replacing it on each series search - Take an error parameter for a sign-in response only with its state, and share one check with the app shell - Show the slide series a derived series is named after
- One rule for sign-in responses, shared by the app shell and the URL rewrites, so a stateless error is never written back - Pass the route's series UID down instead of reading it twice, so a percent-encoded series that is not found says so - Clear an earlier "not found" while a derived series is looked up again - Ask the next client when one lists a derived series but cannot serve it - Send the wildcard worklist queries unless every exact one failed - Let the route resolution name the highlighted series, and remove the separate highlight helper and an unused option
- Ignore the previous study's slides for the render after a study change, so the route is not looked up as a derived series of the new study - Start the route resolution over for another study, server or slide list, so a retry or a server change resolves derived series again - Keep a hash that holds no sign-in response when correcting a series UID - Highlight no slide explicitly while a derived series is unresolved
- Drop sign-in responses in the slide selection and default series redirect too, so no URL writer puts tokens back - Never return the previous study's or server's slides from useSlides, so every consumer of the study view sees the new study only - Remove the route resolution's own study guard and reset its state in one place - Use one query editor and one hash parser for sign-in responses
- Look a derived series up again after another one was looked up, so returning to it shows its dataset; keep the guard in a ref so it costs no extra render - Give a study without clients no slides instead of another study's - Return a cached study in the first render - Strip sign-in responses and the state parameter in one pass, and share the hash parser
- Read the slide cache in useSlides through an external store whose snapshot is the study's cache entry, so every render and every instance sees the cache as it is, with expiry applied - Report loading from the fetch for the current study only, so a cached study returns as loaded while another study's fetch runs - Answer a study without clients directly, not from a stored result - Ask the next client when one lists a derived series but returns no metadata - Give a worklist test that renders a full page more time on a loaded run
- Keep the slides each useSlides instance delivered, so expiry, eviction or a refetch by another instance never changes what a mounted view shows; the cache only spares fetches and answers the first render - Replace the delivered result on cache hits too, so returning to a study never shows its old error or empty result - Clear the pending fetch whenever the effect runs again
- Drop what a useSlides instance delivered when its study or server changes, so coming back never shows an old result or evicted slides - Do not cache the answer of a fetch a retry has superseded - Follow the cache again when called without a study - Keep the delivered object on a cache hit the first render answered
- Count retries per study and server, so a retry on one server does not keep another server's fetch of the study out of the cache - Read the latest cache entry only without a study, so other instances do not re-render on every cache write, and apply expiry to it - Build the study and server key once and use it in the effect
firebase-tools 15.34.1 queries the Service Usage API before deploying, which the deploy service account is not allowed to read, so every preview deploy fails with HTTP 403.
The viewport status pill says "Applying color profile" from the first image of a slide whose image has an ICC profile until the map has drawn the color corrected tiles, and stops after 20 seconds if the profile never arrives.
ICC1 compares the first view of a slide with an ICC profile against the same slide with the profile requests blocked, and against a load where the profile arrives after the first tiles. ICC2 samples the viewport on a Display P3 screen and fails if it goes blank while the profile is applied. ICC3 covers the status pill and ICC4 the ICC profiles switch.
|
This branch has not been deployed
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
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.
DMV pairing (optional)
dmv-branch: feat/markup-measurement-labels
Context
Slim still used Ant Design 4, Less, Create React App (craco), Jest, React 18 class components and
oidc-client. This PR moves the whole UI to the Slim Modern v2 design (light and dark) and updates the toolchain in the same pass. Existing viewer behavior is kept.Changes & Results
Toolchain
pnpm startandpnpm run buildkeep their names, the output still goes tobuild/, andREACT_APP_*variables andPUBLIC_URLstill work. Thedicom-microscopy-viewerworkers and wasm files are copied tostatic/js/, the same path as before.pnpm run check:compilerruns in CI and in the pre-commit hook and fails on bailouts. The only opt-out isWorklistTable(TanStack Table).tsc --noEmit). Vitest 5 with jsdom and Testing Library replaces Jest.e2e/(pnpm test:e2e) run against public IDC slides through the read-only proxy (public/config/e2e.js).src/styles/globals.css(@theme). The design uses lucide-react icons and self-hosted IBM Plex Sans/Mono fonts.Architecture
AppandSlideViewerare now function components. Viewer state and side effects live in hooks undersrc/features/viewer/hooks, and DMV wiring lives in services undersrc/features/viewer/services.src/utilsandsrc/features/*/utils, each with unit tests.oidc-client-tsreplaces the unmaintainedoidc-client. The implicit flow, whichoidc-client-tsdropped, has its own small module (src/auth/implicitGrant.ts). It validatesstate,nonce,at_hash,aud/azp, the issuer and expiry, and checks that the response matches the request type (redirect or silent).UI
Removed:
antd,react-icons,craco,craco-less, all Less files, Jest, ESLint,oidc-client, Material Symbols and the Ant Design profiling script.Follow-up refactor on this branch (pre-doctor)
useRouteSlideResolution.derivedDataLoaderssplit by IOD (ANN / SEG / PM / SR) with a thinloadDerivedDatafacade.useSlideViewerModelowns SlideViewer hook orchestration; JSX is render-only.drawnRoi,roiSelectionStyles,annotationToolModes) with thinner hooks.AnnotationChip, optical-path limits util).tagRows/tagTree/seriesDisplay).implicitAuthorize,idToken,silentFrame).useEffectEventinuseDmvEvents,startTransitionfor bulk visibility.verify-slimskill rewritten for Vite + Slim v2 selectors; full pstack journey suite green on this branch.pnpm test:e2e); Save→reload against docker-compose dcm4chee viapnpm seed:local-archive+pnpm test:e2e:local-archive.Fixes found in review
localStorage.*wildcard*values are both sent: Google Healthcare ignores wildcards and dcm4chee needs them.TrackingIdentifierwhen building a measurement group, sopatches/dcmjs@0.52.0.patchfixes that check until upstream does. Failed saves now show a toast instead of only logging to the console.ModalitiesInStudy, so this cuts the series requests on load from about 100 to 20.disableAnnotationToolsis set. Escape and Go to still do.?state=in the URL shows the Default display instead of a blank presentation state picker.#6a9cf7) with dark button labels; white on the old blue reached 3.5:1. Light theme muted text is darker.The Slides column in the worklist needs
NumberOfStudyRelatedSeries. The IDC proxy does not return it, so the column shows "—" there; dcm4chee and Orthanc do return it.Testing
Pre-doctor automated gate (2026-10-16,
125b773)pnpm run typecheckpnpm test— 195 files, 1495 testspnpm run check:compiler— 208 components/hooks, WorklistTable only opt-out, no bailoutspnpm test:e2e— 103 passed (includes D7/D8)pnpm test:e2e:local-archive— D9 Save→reload on compose dcm4chee (passed)pnpm run build— DMV workers/wasm present underbuild/static/jsManual (optional before inviting doctors)
pnpm seed:local-archive+pnpm test:e2e:local-archivepnpm test:e2eShared master quirks (do not block doctor invite unless they ask): SEG deep-link slide highlight, PR Back, "Unknown Series" tooltip, tag-browser copy, memory warning toast-only-in-Debug.
Manual checklist with IDC study links for each feature:
e2e/TEST-PLAN.mdChecklist
PR
Code
pnpm run lint,pnpm run fmt)Docs
Tested environment