Repository navigation
feat(ui): replace Ant Design with the Slim v2 design system - #451
Open
igoroctaviano wants to merge 196 commits into
Open
igoroctaviano wants to merge 196 commits into
igoroctaviano wants to merge 196 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 22, 2026 10:46a.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 049add9): https://idc-external-006--pr451-feat-slim-modern-red-qeu2j1mu.web.app (expires Wed, 28 Oct 2026 19:37:38 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.
A failed Contains search says so even with search text typed, the table shows it is searching while Search more runs with nothing listed, and the header shows no Contains count before a server is picked.
…ICOM A login page or wrong path still reads as no matches, and a warning now says why in the Debug panel.
…failure throws A failed retrieve settles its slot after reporting, whatever the notification subscribers do.
Worklist waits look for a study link, since the loading placeholder and the empty state are rows too, and the tile wait settles once every tile loaded or failed, then reports how many failed instead of timing out.
With a Contains filter and no server yet, the header no longer says the entry can't be searched on this server while the table asks for a server.
The warning names the server and its origin and is shown once, instead of once per lookup, so a login page on one store does not flood the Debug panel.
igoroctaviano
added
run-e2e
The search state records which server its result came from, so right after a server switch the list shows searching instead of the old matches, and no row checks start for them on the new server.
A click outside, such as on a toast, no longer declines a token consent or confirms nothing, as with the previous confirm dialogs; the server pill tooltip no longer offers a copy it does not have.
…failing silently An error while showing a derived dataset of one kind is logged again, and a notification that throws cannot disturb choosing the default presentation state.
Until the link selects its series, All Series holds several TILs Present segments and a strict locator fails at once, so the test waits for a single one first.
Series are listed as their data arrives, so a name can match one, then several rows before the link narrows the list. Each test now waits in one retrying check for the toggle the link turned on, instead of reading a state that may still change.
VS2-VS4 wait for the overlay the link turned on before the screenshot, W4 orders with the table's comparator, S4 checks that the series actually changed, W2 checks every row once the filter applied, and the snapshot helper describes the one-baseline-per-name layout.
The order no longer depends on which rows the table library samples to pick a comparator.
Escape that closes a dialog, such as the Remove confirm, no longer also cancels the active tool, and Alt shortcuts are ignored while a modal is open.
…at cannot be read A deep-linked dataset that throws while it is shown is logged and reported instead of taking the viewer down from an effect, and one malformed presentation state search result no longer stops the others.
…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
|
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
2 participants
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