Fix/update branch cherry pick (#2039)

* Update all the icons

* Turn off global tool sync; watch as the world burns

* Shift a bunch of things around so we can start tracking/setting per element

* ToolBarService to initiate one of three different calls; callback passed to all button types

* SplitButton and Toolbar Button to use new `onInteraction` prop and new toolbar state

* Changes to toolbar button interface and config

* Fix broken layout selector

* Update SR viewport to activate tools in viewport component

* Duplicate activation logic in measurement tracking extension

* Add alternative/dashed variants for SR Viewport

* pass through "setToolActive" commands for other viewport types

* fix small overlay bugs (no wwwc or scale info)

* Show SpacingBetweenSlices instead of PixelSpacing in patient information dialog

* Fix prop-types

* Update tracked viewport to have alternative tracked styling

* alt styling for SR viewports

* Update to support isLocked + isRehydratable

* fix broken logic

* fix broken logic

* switch icon style

* fix icon styles

* hover and click to start flow

* expedited workflow when data is not dirty (just after SR hydration)

* fix: setting elliptical roi tool

* fix arrow annotate dialog

* fix: do not show learn more button for now

* remove dead code

* simpler cache invalidation

* simpler cache invalidation part 2

* Fix for unable to spand study cards on separate pages

platform/viewer/src/appInit.js

@@ -12,7 +12,7 @@ import {
   ToolBarService,
   ViewportGridService,
   HangingProtocolService,
-  CineService
+  CineService,
   // utils,
   // redux as reduxOHIF,
 } from '@ohif/core';
@@ -33,13 +33,17 @@ function appInit(appConfigOrFunc, defaultExtensions) {
   // TODO: Wire this up to Rodrigo's basic Context "ContextService"
   const commandsManagerConfig = {
     /** Used by commands to inject `viewports` from "redux" */
-    getAppState: () => { },
+    getAppState: () => {},
     /** Used by commands to determine active context */
-    getActiveContexts: () => ['VIEWER', 'DEFAULT', 'ACTIVE_VIEWPORT::CORNERSTONE'],
+    getActiveContexts: () => [
+      'VIEWER',
+      'DEFAULT',
+      'ACTIVE_VIEWPORT::CORNERSTONE',
+    ],
   };
 
-  const servicesManager = new ServicesManager();
   const commandsManager = new CommandsManager(commandsManagerConfig);
+  const servicesManager = new ServicesManager(commandsManager);
   const hotkeysManager = new HotkeysManager(commandsManager, servicesManager);
   const extensionManager = new ExtensionManager({
     commandsManager,
@@ -58,7 +62,7 @@ function appInit(appConfigOrFunc, defaultExtensions) {
     ToolBarService,
     ViewportGridService,
     HangingProtocolService,
-    CineService
+    CineService,
   ]);
 
   /**

platform/viewer/src/routes/DataSourceWrapper.jsx

@@ -6,9 +6,6 @@ import { MODULE_TYPES } from '@ohif/core';
 import { useAppConfig } from '@state';
 import { extensionManager } from '../App.jsx';
 
-let cacheMap = {};
-let total = {};
-
 /**
  * Uses route properties to determine the data source that should be passed
  * to the child layout template. In some instances, initiates requests and
@@ -48,64 +45,56 @@ function DataSourceWrapper(props) {
   // But only for LayoutTemplate type of 'list'?
   // Or no data fetching here, and just hand down my source
   const STUDIES_LIMIT = 101;
-  const [data, setData] = useState({ studies: [], total: 0 });
+  const [data, setData] = useState({
+    studies: [],
+    total: 0,
+    resultsPerPage: 25,
+    pageNumber: 1,
+  });
   const [isLoading, setIsLoading] = useState(false);
 
   useEffect(() => {
+    const queryFilterValues = _getQueryFilterValues(
+      history.location.search,
+      STUDIES_LIMIT
+    );
+
     // 204: no content
     async function getData() {
       setIsLoading(true);
 
-      const limit = STUDIES_LIMIT - 1;
-      const queryFilterValues = _getQueryFilterValues(history.location.search);
-      const { resultsPerPage = 25, pageNumber = 1 } = queryFilterValues;
-      const reachedLimits = parseInt((resultsPerPage * pageNumber) / STUDIES_LIMIT);
-      const cacheKey = `${pageNumber}-${resultsPerPage}`;
-
-      const getFromCache = async ({ cacheKey, pageNumber, resultsPerPage, limit, options }) => {
-        const pagesAmount = limit / resultsPerPage;
-        const pageToRequest = parseInt((resultsPerPage * pageNumber) / STUDIES_LIMIT);
-
-        let length = 0;
-        if (!cacheMap[cacheKey]) {
-          length = pageToRequest > 0 ? (pageToRequest * STUDIES_LIMIT) : 1;
-          const studiesPromise = dataSource.query.studies.search(options);
-
-          for (let pageNum = 0; pageNum < pagesAmount; pageNum++) {
-            const currentPageNumber = (pageNum + 1) + (pageToRequest * pagesAmount);
-            cacheMap[`${currentPageNumber}-${resultsPerPage}`] = studiesPromise.then(function (results) {
-              const slicedResult = results.slice((pageNum * resultsPerPage), ((pageNum + 1) * resultsPerPage));
-              length += slicedResult.length;
-              return slicedResult;
-            });
-          }
-        }
-
-        const cache = await cacheMap[cacheKey];
-        return { cache, length, index: pageToRequest };
-      };
-
-      const { cache: studies, index, length } = await getFromCache({
-        cacheKey,
-        pageNumber,
-        resultsPerPage,
-        limit,
-        options: { ...queryFilterValues, ...{ offset: reachedLimits * limit } }
-      });
-
-      const totalKey = `${resultsPerPage}-${index}`;
-      total[totalKey] = total[totalKey] ? total[totalKey] + length : length;
-      const totals = Object.keys(total).map(key => total[key]);
-      const biggestIndex = totals.indexOf(Math.max(...totals));
-      const biggestKey = Object.keys(total)[biggestIndex];
-      const biggestTotal = total[biggestKey];
+      const studies = await dataSource.query.studies.search(queryFilterValues);
 
       setIsLoading(false);
-      setData({ studies, total: biggestTotal });
+      setData({
+        studies,
+        total: studies.length,
+        resultsPerPage: queryFilterValues.resultsPerPage,
+        pageNumber: queryFilterValues.pageNumber,
+      });
     }
 
     try {
-      getData();
+      // Cache invalidation :thinking:
+      // - Anytime change is not just next/previous page
+      // - And we didn't cross a result offset range
+      const isFirstLoad = data.studies.length === 0;
+      const isSamePage = data.pageNumber === queryFilterValues.pageNumber;
+      const previousOffset =
+        Math.floor((data.pageNumber * data.resultsPerPage) / STUDIES_LIMIT) *
+        (STUDIES_LIMIT - 1);
+      const newOffset =
+        Math.floor(
+          (queryFilterValues.pageNumber * queryFilterValues.resultsPerPage) /
+            STUDIES_LIMIT
+        ) *
+        (STUDIES_LIMIT - 1);
+      const isDataInvalid =
+        isFirstLoad || isSamePage || newOffset !== previousOffset;
+
+      if (isDataInvalid) {
+        getData();
+      }
     } catch (ex) {
       console.warn(ex);
     }
@@ -138,25 +127,32 @@ export default DataSourceWrapper;
  * Need generic that can be shared? Isn't this what qs is for?
  * @param {*} query
  */
-function _getQueryFilterValues(query) {
+function _getQueryFilterValues(query, queryLimit) {
   query = new URLSearchParams(query);
 
+  const pageNumber = _tryParseInt(query.get('pageNumber'), 1);
+  const resultsPerPage = _tryParseInt(query.get('resultsPerPage'), 25);
+
   const queryFilterValues = {
     // DCM
     patientId: query.get('mrn'),
     patientName: query.get('patientName'),
     studyDescription: query.get('description'),
-    modalitiesInStudy: query.get('modalities') && query.get('modalities').split(','),
+    modalitiesInStudy:
+      query.get('modalities') && query.get('modalities').split(','),
     accessionNumber: query.get('accession'),
     //
     startDate: query.get('startDate'),
     endDate: query.get('endDate'),
     page: _tryParseInt(query.get('page'), undefined),
-    pageNumber: _tryParseInt(query.get('pageNumber'), undefined),
-    resultsPerPage: _tryParseInt(query.get('resultsPerPage'), undefined),
+    pageNumber,
+    resultsPerPage,
     // Rarely supported server-side
     sortBy: query.get('sortBy'),
     sortDirection: query.get('sortDirection'),
+    // Offset...
+    offset:
+      Math.floor((pageNumber * resultsPerPage) / queryLimit) * (queryLimit - 1),
   };
 
   // patientName: good

platform/viewer/src/routes/WorkList/WorkList.jsx

@@ -24,7 +24,7 @@ import {
   Header,
   useModal,
   AboutModal,
-  UserPreferences
+  UserPreferences,
 } from '@ohif/ui';
 
 const seriesInStudiesMap = new Map();
@@ -33,7 +33,14 @@ const seriesInStudiesMap = new Map();
  * TODO:
  * - debounce `setFilterValues` (150ms?)
  */
-function WorkList({ history, data: studies, dataTotal: studiesTotal, isLoadingData, dataSource, hotkeysManager }) {
+function WorkList({
+  history,
+  data: studies,
+  dataTotal: studiesTotal,
+  isLoadingData,
+  dataSource,
+  hotkeysManager,
+}) {
   const { hotkeyDefinitions, hotkeyDefaults } = hotkeysManager;
   const { show, hide } = useModal();
   const { t } = useTranslation();
@@ -94,7 +101,6 @@ function WorkList({ history, data: studies, dataTotal: studiesTotal, isLoadingDa
   const [expandedRows, setExpandedRows] = useState([]);
   const [studiesWithSeriesData, setStudiesWithSeriesData] = useState([]);
   const numOfStudies = studiesTotal;
-  const totalPages = Math.floor(numOfStudies / resultsPerPage) + 1;
 
   const setFilterValues = val => {
     if (filterValues.pageNumber === val.pageNumber) {
@@ -105,7 +111,15 @@ function WorkList({ history, data: studies, dataTotal: studiesTotal, isLoadingDa
   };
 
   const onPageNumberChange = newPageNumber => {
-    if (newPageNumber > totalPages) {
+    debugger;
+    const oldPageNumber = filterValues.pageNumber;
+    const rollingPageNumberMod = Math.floor(101 / filterValues.resultsPerPage);
+    const rollingPageNumber = oldPageNumber % rollingPageNumberMod;
+    const isNextPage = newPageNumber > oldPageNumber;
+    const hasNextPage =
+      Math.max(rollingPageNumber, 1) * resultsPerPage < numOfStudies;
+
+    if (isNextPage && !hasNextPage) {
       return;
     }
 
@@ -170,9 +184,13 @@ function WorkList({ history, data: studies, dataTotal: studiesTotal, isLoadingDa
   // Query for series information
   useEffect(() => {
     const fetchSeries = async studyInstanceUid => {
+      debugger;
       try {
         const series = await dataSource.query.series.search(studyInstanceUid);
-        seriesInStudiesMap.set(studyInstanceUid, utils.sortBySeriesDate(series));
+        seriesInStudiesMap.set(
+          studyInstanceUid,
+          utils.sortBySeriesDate(series)
+        );
         setStudiesWithSeriesData([...studiesWithSeriesData, studyInstanceUid]);
       } catch (ex) {
         // TODO: UI Notification Service
@@ -199,6 +217,10 @@ function WorkList({ history, data: studies, dataTotal: studiesTotal, isLoadingDa
     return !isEqual(filterValues, defaultFilterValues);
   };
 
+  const rollingPageNumberMod = Math.floor(101 / resultsPerPage);
+  const rollingPageNumber = (pageNumber - 1) % rollingPageNumberMod;
+  const offset = resultsPerPage * rollingPageNumber;
+  const offsetAndTake = offset + resultsPerPage;
   const tableDataSource = sortedStudies.map((study, key) => {
     const rowKey = key + 1;
     const isExpanded = expandedRows.some(k => k === rowKey);
@@ -229,8 +251,8 @@ function WorkList({ history, data: studies, dataTotal: studiesTotal, isLoadingDa
           content: patientName ? (
             <TooltipClipboard>{patientName}</TooltipClipboard>
           ) : (
-              <span className="text-gray-700">(Empty)</span>
-            ),
+            <span className="text-gray-700">(Empty)</span>
+          ),
           gridCol: 4,
         },
         {
@@ -294,13 +316,13 @@ function WorkList({ history, data: studies, dataTotal: studiesTotal, isLoadingDa
           seriesTableDataSource={
             seriesInStudiesMap.has(studyInstanceUid)
               ? seriesInStudiesMap.get(studyInstanceUid).map(s => {
-                return {
-                  description: s.description || '(empty)',
-                  seriesNumber: s.seriesNumber || '',
-                  modality: s.modality || '',
-                  instances: s.numSeriesInstances || '',
-                };
-              })
+                  return {
+                    description: s.description || '(empty)',
+                    seriesNumber: s.seriesNumber || '',
+                    modality: s.modality || '',
+                    instances: s.numSeriesInstances || '',
+                  };
+                })
               : []
           }
         >
@@ -317,7 +339,7 @@ function WorkList({ history, data: studies, dataTotal: studiesTotal, isLoadingDa
               <Link
                 key={i}
                 to={`${mode.id}?StudyInstanceUIDs=${studyInstanceUid}`}
-              // to={`${mode.id}/dicomweb?StudyInstanceUIDs=${studyInstanceUid}`}
+                // to={`${mode.id}/dicomweb?StudyInstanceUIDs=${studyInstanceUid}`}
               >
                 <Button
                   rounded="full"
@@ -325,7 +347,7 @@ function WorkList({ history, data: studies, dataTotal: studiesTotal, isLoadingDa
                   disabled={false}
                   endIcon={<Icon name="launch-arrow" />} // launch-arrow | launch-info
                   className={classnames('font-bold', { 'ml-2': !isFirst })}
-                  onClick={() => { }}
+                  onClick={() => {}}
                 >
                   {mode.displayName}
                 </Button>
@@ -348,25 +370,28 @@ function WorkList({ history, data: studies, dataTotal: studiesTotal, isLoadingDa
     {
       title: t('Header:About'),
       icon: 'info',
-      onClick: () => show({ content: AboutModal, title: 'About OHIF Viewer' })
+      onClick: () => show({ content: AboutModal, title: 'About OHIF Viewer' }),
     },
     {
       title: t('Header:Preferences'),
       icon: 'settings',
-      onClick: () => show({
-        title: t('UserPreferencesModal:User Preferences'),
-        content: UserPreferences,
-        contentProps: {
-          hotkeyDefaults: hotkeysManager.getValidHotkeyDefinitions(hotkeyDefaults),
-          hotkeyDefinitions,
-          onCancel: hide,
-          onSubmit: ({ hotkeyDefinitions }) => {
-            hotkeysManager.setHotkeys(hotkeyDefinitions);
-            hide();
+      onClick: () =>
+        show({
+          title: t('UserPreferencesModal:User Preferences'),
+          content: UserPreferences,
+          contentProps: {
+            hotkeyDefaults: hotkeysManager.getValidHotkeyDefinitions(
+              hotkeyDefaults
+            ),
+            hotkeyDefinitions,
+            onCancel: hide,
+            onSubmit: ({ hotkeyDefinitions }) => {
+              hotkeysManager.setHotkeys(hotkeyDefinitions);
+              hide();
+            },
+            onReset: () => hotkeysManager.restoreDefaultBindings(),
           },
-          onReset: () => hotkeysManager.restoreDefaultBindings()
-        }
-      })
+        }),
     },
   ];
 
@@ -378,7 +403,7 @@ function WorkList({ history, data: studies, dataTotal: studiesTotal, isLoadingDa
     >
       <Header isSticky menuOptions={menuOptions} isReturnEnabled={false} />
       <StudyListFilter
-        numOfStudies={numOfStudies}
+        numOfStudies={pageNumber * resultsPerPage > 100 ? 101 : numOfStudies}
         filtersMeta={filtersMeta}
         filterValues={{ ...filterValues, ...defaultSortValues }}
         onChange={setFilterValues}
@@ -388,7 +413,7 @@ function WorkList({ history, data: studies, dataTotal: studiesTotal, isLoadingDa
       {hasStudies ? (
         <>
           <StudyListTable
-            tableDataSource={tableDataSource}
+            tableDataSource={tableDataSource.slice(offset, offsetAndTake)}
             numOfStudies={numOfStudies}
             filtersMeta={filtersMeta}
           />
@@ -400,10 +425,10 @@ function WorkList({ history, data: studies, dataTotal: studiesTotal, isLoadingDa
           />
         </>
       ) : (
-          <div className="flex flex-col items-center justify-center pt-48">
-            <EmptyStudies isLoading={isLoadingData} />
-          </div>
-        )}
+        <div className="flex flex-col items-center justify-center pt-48">
+          <EmptyStudies isLoading={isLoadingData} />
+        </div>
+      )}
     </div>
   );
 }