feat(WorkListUINext): New Study List (WorkListUINext)#6005
Conversation
Between 3 and 9
…try loop, and add Retry action to the connection-error modal.
|
@greptileai I believe even your comments outside diff have been addressed. |
|
You're right — the fix is in place. The |
| getGetThumbnailSrc: function (instance, imageId) { | ||
| if (dicomWebConfig.thumbnailRendering === 'wadors') { | ||
| getGetThumbnailSrc: function (thumbnailContext: ThumbnailContext, imageId) { | ||
| const thumbnailRendering = dicomWebConfig.thumbnailRendering; |
There was a problem hiding this comment.
can we refactor this out, it is adding noise to data source
There was a problem hiding this comment.
What is your suggestion for what part remains in datasource? The problem is that different datasources end up with different methods/capabilities here. I'm ok with it moving out, but would like to know what pieces go where and how the choices are made.
There was a problem hiding this comment.
I refactored all of the thumbnail retrieval logic/code to a file retrieveThumbnail.ts that lives next to the index.ts file. I hope you both find this adequate.
| }); | ||
| } | ||
|
|
||
| function buildThumbnailEndpointPath( |
| maxNumRequests: { | ||
| interaction: 100, | ||
| thumbnail: 75, | ||
| thumbnail: 5, |
There was a problem hiding this comment.
this should get communicated as breaking change in the docs, same as below
There was a problem hiding this comment.
I would say these are NOT breaking changes. Those values should never have been that high for thumbnail requests.
There was a problem hiding this comment.
maybe don't label as breaking change but communicate we have changed these
| : 'all'; | ||
| const seriesView: PreviewSeriesView = forceListView ? 'list' : configuredSeriesView; | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
this is massive useEffect, it should be its own useSeriesFetch hook
There was a problem hiding this comment.
That will also make it effectively useable by other components.
There was a problem hiding this comment.
same here this useEffect better be refactored as a hook
| * @param {object} b - Second object | ||
| * @returns {boolean} True if the two are equal under the rules above. | ||
| */ | ||
| export function shallowEqualIgnoringArrayOrder(a, b): boolean { |
| startDate, | ||
| endDate, | ||
| onChange, | ||
| inputClassName, |
There was a problem hiding this comment.
lots of props mean we are not following shadcn composition correctly, can we rethink this with composition instead of props
| * @param time - Raw time string (HH, HHmm, HHmmss, or HHmmss.SSS format) | ||
| * @returns Formatted date string (DD-MMM-YYYY HH:mm) or empty string if invalid | ||
| */ | ||
| export function formatStudyDate(date?: string, time?: string): string { |
There was a problem hiding this comment.
this feels like it should have been implemented elsewhere already, please make sure we are not adding a new one
| "cmdk": "1.1.1", | ||
| "date-fns": "3.6.0", | ||
| "framer-motion": "6.2.4", | ||
| "moment": "2.30.1", |
There was a problem hiding this comment.
i really don't want to add a new time-related dependency, can we use the new Temporal api that is widely supported
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Temporal
see also other blog posts on how to migrate
https://www.smashingmagazine.com/2026/03/moving-from-moment-to-temporal-api/
There was a problem hiding this comment.
moment is already a dependency in core, ui and devDependencies. I am simply adding it also to ui-next. So it is not new at all.
| customizationService: [ | ||
| { | ||
| 'workList.columns': { | ||
| // Hide the MRN column and move Instances to the front. |
There was a problem hiding this comment.
Can you include an example of adding a column?
|
|
||
| ## `workList.columns` | ||
|
|
||
| Builds the column set for the WorkList table. The customization is a function with the same shape as `StudyList.defaultColumns` — it receives the default `ColumnDef[]` and must return a `ColumnDef[]`. Use it to reorder, hide, or insert columns without rewriting the defaults. |
There was a problem hiding this comment.
Can this be done as a fixed list rather than a function returning a new column def? That could then be modified directly by the invariant $ functions using straight properties, at least as long as there was a basic column type that could display a column.
| COLUMN_IDS.DESCRIPTION, | ||
| ] as const; | ||
|
|
||
| export function defaultColumns(): ColumnDef<StudyRow, unknown>[] { |
There was a problem hiding this comment.
Is there a reason this has to be a function? It looks more or less like a constant which could be re-used/re-created.
Ideally the shadcn component composition could be used to create a completely different layout by including alternate header functions/render body functions, or the default columns could be applied.
Add documentation for the queryLimit configuration.
Context
This PR replaces the legacy
WorkListstudy-list route with a newWorkListUINextroute, built on a freshui-nextstudy-list system (StudyList,DataTable,PreviewContainer,PreviewPatientSummary, etc.). The legacyWorkListremains importable for fallback, but/now resolves toWorkListUINextby default. The change set spans 138 commits and ~5,600 lines added.Key drivers:
sessionStoragevia the newuseStudyListStateSynchook.DataSourceWrappersimplified to client-side pagination — one server query per filter change instead of per page change.thumbnailRequestStrategy(bulkDataRetrievedefault, orfetch) and a series-level QIDO fallback for thumbnail fetching.workListUINext.previewSeriesViewcustomization ('all' | 'thumbnails' | 'list') with documentation.Changes & Results
Basics
Workflows/modes
Preview side panel
1. Routing (
platform/app/src/routes/index.tsx)/route now rendersWorkListUINextviaDataSourceWrapper.WorkListimport is retained and commented as a swap-in alternative.2.
DataSourceWrapper(platform/app/src/routes/DataSourceWrapper.tsx)appConfig.queryLimit ?? 101) instead of refetching on every page/offset change.WorkListUINextviauseStudyListStateSync.hasFetchedOnceflag is plumbed through to the layout so the worklist can suppress the empty state until the first query resolves; it resets on data-source change and ononRefresh.pageNumber/resultsPerPage/offset/locationshape is dropped from internal state, along with theareLocationsTheSameheuristic, replaced by aqueryFilterValuesdeep-equality check (areQueryFiltersEqual) that handles array filters likemodalitiesInStudy.defaultDataSourceNamenow reads fromuseAppConfig()rather thanwindow.configdirectly.3. WorkListUINext route (
platform/app/src/routes/WorkListUINext/)WorkListUINext.tsxanduseWorkListToolbarActions.tsx.datasourcesquery param is preserved across navigation.4. Study list state sync (
platform/app/src/hooks/useStudyListStateSync.ts)sessionStorageis the fallback.5. ui-next StudyList system (
platform/ui-next/src/components/StudyList/)Layout,Table,PreviewContainer,PreviewContent,PreviewPatientSummary,PreviewSeriesList,PreviewToggleButton,SettingsPopover,WorkflowsProvider,WorkflowMenu.useDefaultWorkflow.defaultColumns,formatStudyDate,tokenizeModalities.6. ui-next primitives
DataTablecompound component (ColumnHeader,FilterRow,Pagination,Toolbar,ViewOptions,ActionOverlayCell).InputMultiSelect,Badge, and genericTablecomponents.DateRangeis expanded with placeholder editing and is sized to match other filters.Thumbnailis refined with blob-URL revoke on error and a viewport query param for quality.Iconsare added:OHIFLogoHorizontal,PanelRight,PatientStudyList,SeriesPlaceholder,SettingsStudyList,SortingNew/Ascending/Descending,CloudSettings.7. DICOMWeb data source (
extensions/default/src/DicomWebDataSource/index.ts)thumbnailRequestStrategy(bulkDataRetrievedefault, orfetch).getGetThumbnailSrcnow accepts aThumbnailContextand falls back to a single QIDOinstances?limit=1query when a series-level fetch fails.dicom-web.md.8. Customization
workListUINext.previewSeriesView('all' | 'thumbnails' | 'list') is registered viagetCustomizationModule.tsx.'list'when the data source useswadors/thumbnailDirectrendering orbulkDataRetrieveretrieval.WorkListUINext.md.9. Misc
DataSourceConfigurationComponent.tsx).DataTableare added.10. Removed
OHIFStudyList.spec.jsis removed.tests/Worklist.spec.tsis removed (no longer applicable to the new study list).Testing
1. Smoke test
/loads the newWorkListUINextagainst the default data source.2. State sync
/(no params); sessionStorage state should restore until tab close.3.
DataSourceWrapperrefetch behaviorappConfig.queryLimitto a small value (e.g. 25) and confirm only that many studies come back.4. Preview panel
workListUINext.previewSeriesViewcustomization values ('all','thumbnails','list') hide/lock the toggle as documented.5. Thumbnail strategies
wadorssource (e.g.local_orthanc,local_dcm4chee) and confirm the preview is forced to list view.thumbnail/renderedsource verify bothbulkDataRetrieveandfetchstrategies render thumbnails.limit=1lookup without crashing.6. Workflow buttons & default mode
7. Toolbar/extras
8. Pagination persistence
a292d59f7a).9. Legacy fallback
routes/index.tsxback toWorkListand confirm the legacy view still renders.Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
System:
Binaries:
Browsers:
Greptile Summary
This PR replaces the legacy study list with a new
WorkListUINextsystem built onui-nextcomponents, covering the full worklist surface: table state synced to URL andsessionStorage, client-side pagination (single server query per filter change), a preview side panel with bounded-concurrency thumbnail fetching, and a newthumbnailRequestStrategyfor the DICOMweb data source.DataSourceWrapperis simplified to one query per filter change with ahasFetchedOncegate, properfinally-block loading-state reset, and a retry button on failure;DEFAULT_DATAis now module-scoped.useStudyListStateSyncandstudyListFilterContractprovide a typed, drift-proof URL contract for sorting, pagination, and all filter keys \u2014 including the previously flaggedpatientNamemismatch, which is now resolved.SidePanelPreviewmanages thumbnail fetching with anAbortController, a capped worker pool, and explicitURL.revokeObjectURLtracking viaownedBlobUrlsRef, addressing prior blob-leak concerns.Confidence Score: 4/5
Safe to merge after the rows-per-page dropdown close bug is fixed; the rest of the change is well-structured with no data-loss paths.
The rows-per-page dropdown in Pagination.tsx calls e.preventDefault() inside onSelect, which per Radix UI docs prevents the menu from auto-closing after every page-size selection. All other findings are non-blocking style or edge-case correctness issues.
platform/ui-next/src/components/DataTable/Pagination.tsx — the rows-per-page dropdown; platform/app/src/routes/DataSourceWrapper.tsx — the pre-existing stale in-flight query on data-source switch (open from prior review round).
Important Files Changed
Comments Outside Diff (3)
platform/app/src/routes/DataSourceWrapper.tsx, line 141-155 (link)getData()setsisLoading = trueon entry but only callssetIsLoading(false)on the happy-path completion. WhendataSource.query.studies.search()throws, the.catch()handler logs the error or shows a modal but never resetsisLoading. BecauseisDataInvalidis guarded by!isLoading, every subsequent location/filter change will be ignored — no further queries fire until the page is reloaded.The new
!hasFetchedOncecondition inshowStudyListLoadingmakes this worse: after a failed first fetch both the loading spinner and the empty-state are suppressed indefinitely, leaving users with a blank, unresponsive worklist.Prompt To Fix With AI
platform/app/src/routes/DataSourceWrapper.tsx, line 141-156 (link)getData()callssetIsLoading(true)on entry but only callssetIsLoading(false)on the happy path. IfdataSource.query.studies.search()throws, the.catch()error handler shows a modal but never resets the flag. BecauseisDataInvalid = !isLoading && filtersChanged, the wrapper refuses to fire any further query onceisLoadingis stuck attrue. Combined with!hasFetchedOnceremainingfalse, both the loading spinner and the empty state are suppressed indefinitely after a failed first fetch, leaving users with a blank, unresponsive worklist until they reload the page.Prompt To Fix With AI
platform/app/src/routes/DataSourceWrapper.tsx, line 128-144 (link)dataSourceChangedCallbackcallsrefresh()(which setsdata.queryFilterValues = null) to ensure a new query fires, but any in-flightgetData()from the old data source is not cancelled. If the old query completes after the new data source has already fetched and populateddata.queryFilterValues, the old query'ssetData({ ..., queryFilterValues: oldQFV })overwrites the new results. The effect then re-evaluatesfiltersChanged = !shallowEqualIgnoringArrayOrder(oldQFV, newQFV)— since both derive from the same URL, they are equal →filtersChanged = false→ no corrective refetch fires. The user sees the old data source's studies with no indication that anything is wrong, and no further query is issued.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (21): Last reviewed commit: "Refactor the thumbnail retrieval to a se..." | Re-trigger Greptile