From 696e365e74920637fde18302459289312901bc30 Mon Sep 17 00:00:00 2001 From: Joe Boccanfuso Date: Wed, 25 Oct 2023 16:15:33 -0400 Subject: [PATCH 1/5] feat(filters): save worklist query filters to session storage so that they persist between navigation to the viewer and back Co-authored-by: ladeirarodolfo <39910206+ladeirarodolfo@users.noreply.github.com> --- .../study-list/OHIFStudyList.spec.js | 60 ++++++++ platform/app/src/routes/WorkList/WorkList.tsx | 14 +- platform/ui/jest.config.js | 13 ++ platform/ui/package.json | 4 +- .../ui/src/components/DateRange/DateRange.tsx | 2 +- platform/ui/src/components/Header/Header.tsx | 1 + .../StudyListFilter/StudyListFilter.tsx | 4 +- .../StudyListTable/StudyListTableRow.tsx | 8 +- platform/ui/src/hooks/index.ts | 3 +- .../ui/src/hooks/useSessionStorage.test.js | 129 ++++++++++++++++++ platform/ui/src/hooks/useSessionStorage.tsx | 63 +++++++++ platform/ui/src/index.js | 2 + yarn.lock | 42 +++++- 13 files changed, 336 insertions(+), 9 deletions(-) create mode 100644 platform/ui/jest.config.js create mode 100644 platform/ui/src/hooks/useSessionStorage.test.js create mode 100644 platform/ui/src/hooks/useSessionStorage.tsx diff --git a/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js b/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js index 9809a1b7c51..fb134315869 100644 --- a/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js +++ b/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js @@ -33,6 +33,21 @@ describe('OHIF Study List', function () { }); }); + it('maintains Patient Name filter upon return from viewer', function () { + cy.get('@PatientName').type('Juno'); + //Wait result list to be displayed + cy.waitStudyList(); + cy.get('[data-cy="studyRow-1.3.6.1.4.1.25403.345050719074.3824.20170125113417.1"]').click(); + cy.get( + '[data-cy="mode-basic-test-1.3.6.1.4.1.25403.345050719074.3824.20170125113417.1"]' + ).click(); + cy.get('[data-cy="return-to-work-list"]').click(); + cy.get('@searchResult2').should($list => { + expect($list.length).to.be.eq(1); + expect($list).to.contain('Juno'); + }); + }); + it('searches MRN with exact string', function () { cy.get('@MRN').type('0000003'); //Wait result list to be displayed @@ -43,6 +58,21 @@ describe('OHIF Study List', function () { }); }); + it('maintains MRN filter upon return from viewer', function () { + cy.get('@MRN').type('0000003'); + //Wait result list to be displayed + cy.waitStudyList(); + cy.get('[data-cy="studyRow-1.3.6.1.4.1.25403.345050719074.3824.20170125113417.1"]').click(); + cy.get( + '[data-cy="mode-basic-test-1.3.6.1.4.1.25403.345050719074.3824.20170125113417.1"]' + ).click(); + cy.get('[data-cy="return-to-work-list"]').click(); + cy.get('@searchResult2').should($list => { + expect($list.length).to.be.eq(1); + expect($list).to.contain('0000003'); + }); + }); + it('searches Accession with exact string', function () { cy.get('@AccessionNumber').type('321'); //Wait result list to be displayed @@ -53,6 +83,21 @@ describe('OHIF Study List', function () { }); }); + it('maintains Accession filter upon return from viewer', function () { + cy.get('@AccessionNumber').type('321'); + //Wait result list to be displayed + cy.waitStudyList(); + cy.get('[data-cy="studyRow-1.3.6.1.4.1.25403.345050719074.3824.20170125113417.1"]').click(); + cy.get( + '[data-cy="mode-basic-test-1.3.6.1.4.1.25403.345050719074.3824.20170125113417.1"]' + ).click(); + cy.get('[data-cy="return-to-work-list"]').click(); + cy.get('@searchResult2').should($list => { + expect($list.length).to.be.eq(1); + expect($list).to.contain('321'); + }); + }); + it('searches Description with exact string', function () { cy.get('@StudyDescription').type('PETCT'); //Wait result list to be displayed @@ -63,6 +108,21 @@ describe('OHIF Study List', function () { }); }); + it('maintains Description filter upon return from viewer', function () { + cy.get('@StudyDescription').type('PETCT'); + //Wait result list to be displayed + cy.waitStudyList(); + cy.get('[data-cy="studyRow-1.3.6.1.4.1.25403.345050719074.3824.20170125113417.1"]').click(); + cy.get( + '[data-cy="mode-basic-test-1.3.6.1.4.1.25403.345050719074.3824.20170125113417.1"]' + ).click(); + cy.get('[data-cy="return-to-work-list"]').click(); + cy.get('@searchResult2').should($list => { + expect($list.length).to.be.eq(1); + expect($list).to.contain('PETCT'); + }); + }); + /* Todo: fix react select it('searches Modality with camel case', function() { cy.get('@modalities').type('Ct'); diff --git a/platform/app/src/routes/WorkList/WorkList.tsx b/platform/app/src/routes/WorkList/WorkList.tsx index 865c6fbb9ec..503a2c87e4f 100644 --- a/platform/app/src/routes/WorkList/WorkList.tsx +++ b/platform/app/src/routes/WorkList/WorkList.tsx @@ -26,6 +26,7 @@ import { AboutModal, UserPreferences, LoadingIndicatorProgress, + useSessionStorage, } from '@ohif/ui'; import i18n from '@ohif/i18n'; @@ -60,9 +61,17 @@ function WorkList({ const navigate = useNavigate(); const STUDIES_LIMIT = 101; const queryFilterValues = _getQueryFilterValues(searchParams); + const [sessionQueryFilterValues, saveSessionQueryFilterValues] = useSessionStorage({ + key: 'queryFilterValues', + defaultValue: queryFilterValues, + // ToDo: useSessionStorage currently uses an unload listener to clear the filters from session storage + // so on systems that do not support unload events a user will NOT be able to alter any existing filter + // in the URL, load the page and have it apply. + clearOnUnload: true, + }); const [filterValues, _setFilterValues] = useState({ ...defaultFilterValues, - ...queryFilterValues, + ...sessionQueryFilterValues, }); const debouncedFilterValues = useDebounce(filterValues, 200); @@ -119,6 +128,7 @@ function WorkList({ val.pageNumber = 1; } _setFilterValues(val); + saveSessionQueryFilterValues(val); setExpandedRows([]); }; @@ -251,6 +261,7 @@ function WorkList({ moment(time, ['HH', 'HHmm', 'HHmmss', 'HHmmss.SSS']).format('hh:mm A'); return { + dataCY: `studyRow-${studyInstanceUid}`, row: [ { key: 'patientName', @@ -377,6 +388,7 @@ function WorkList({ disabled={!isValidMode} endIcon={} // launch-arrow | launch-info onClick={() => {}} + data-cy={`mode-${mode.routeName}-${studyInstanceUid}`} > {t(`Modes:${mode.displayName}`)} diff --git a/platform/ui/jest.config.js b/platform/ui/jest.config.js new file mode 100644 index 00000000000..2978b062ed1 --- /dev/null +++ b/platform/ui/jest.config.js @@ -0,0 +1,13 @@ +const base = require('../../jest.config.base.js'); +const pkg = require('./package'); + +module.exports = { + ...base, + name: pkg.name, + displayName: pkg.name, + // rootDir: "../.." + // testMatch: [ + // //`/platform/${pack.name}/**/*.spec.js` + // "/platform/app/**/*.test.js" + // ] +}; diff --git a/platform/ui/package.json b/platform/ui/package.json index a90b8c84948..6d9109336d7 100644 --- a/platform/ui/package.json +++ b/platform/ui/package.json @@ -50,7 +50,9 @@ "react-window": "^1.8.9", "react-with-direction": "^1.3.1", "swiper": "^8.4.2", - "webpack": "^5.81.0" + "webpack": "^5.81.0", + "@testing-library/react-hooks": "^3.2.1", + "react-test-renderer": "^16.12.0" }, "devDependencies": { "@babel/core": "^7.21.4", diff --git a/platform/ui/src/components/DateRange/DateRange.tsx b/platform/ui/src/components/DateRange/DateRange.tsx index 589ad30ebe9..93d81bef18d 100644 --- a/platform/ui/src/components/DateRange/DateRange.tsx +++ b/platform/ui/src/components/DateRange/DateRange.tsx @@ -175,7 +175,7 @@ DateRange.propTypes = { /** YYYYMMDD (19921022) */ startDate: PropTypes.string, /** YYYYMMDD (19921022) */ - endDate: PropTypes.object, + endDate: PropTypes.string, /** Callback that received { startDate: string(YYYYMMDD), endDate: string(YYYYMMDD)} */ onChange: PropTypes.func.isRequired, }; diff --git a/platform/ui/src/components/Header/Header.tsx b/platform/ui/src/components/Header/Header.tsx index a780b231c59..3bf7a69268a 100644 --- a/platform/ui/src/components/Header/Header.tsx +++ b/platform/ui/src/components/Header/Header.tsx @@ -43,6 +43,7 @@ function Header({ isReturnEnabled && 'cursor-pointer' )} onClick={onClickReturn} + data-cy="return-to-work-list" > {isReturnEnabled && ( )} -
+
{/* TODO revisit the completely rounded style of button used for clearing the study list filter - for now use LegacyButton*/} {isFiltering && ( {t('Studies')} diff --git a/platform/ui/src/components/StudyListTable/StudyListTableRow.tsx b/platform/ui/src/components/StudyListTable/StudyListTableRow.tsx index 079a2985f4d..2235f277d1c 100644 --- a/platform/ui/src/components/StudyListTable/StudyListTableRow.tsx +++ b/platform/ui/src/components/StudyListTable/StudyListTableRow.tsx @@ -7,10 +7,13 @@ import Icon from '../Icon'; const StudyListTableRow = props => { const { tableData } = props; - const { row, expandedContent, onClickRow, isExpanded } = tableData; + const { row, expandedContent, onClickRow, isExpanded, dataCY } = tableData; return ( <> - + { + beforeEach(() => { + window.sessionStorage.removeItem(SESSION_STORAGE_KEY); + }); + + it('hook should return state and setState', () => { + const data = { test: 1 }; + const { result } = renderHook(() => + useSessionStorage({ key: SESSION_STORAGE_KEY, defaultValue: data }) + ); + const [hookState, setHookState] = result.current; + expect(hookState).toStrictEqual(data); + expect(typeof setHookState).toBe('function'); + }); + + it('hook should store data on sessionStorage', () => { + const data = { test: 2 }; + renderHook(() => useSessionStorage({ key: SESSION_STORAGE_KEY, defaultValue: data })); + + const dataStr = JSON.stringify(data); + const dataSessionStorage = window.sessionStorage.getItem(SESSION_STORAGE_KEY); + expect(dataSessionStorage).toEqual(dataStr); + }); + + it('hook should return stored data from sessionStorage', () => { + const data = { test: 3 }; + const dataToCompare = { test: 4 }; + + window.sessionStorage.setItem(SESSION_STORAGE_KEY, JSON.stringify(dataToCompare)); + + const { result } = renderHook(() => + useSessionStorage({ key: SESSION_STORAGE_KEY, defaultValue: data }) + ); + const [hookState, setHookState] = result.current; + + expect(hookState).toStrictEqual(dataToCompare); + }); + + it('hook should provide a setState method which updates its state', () => { + const data = { test: 5 }; + const dataToCompare = { test: 6 }; + const { result } = renderHook(() => + useSessionStorage({ key: SESSION_STORAGE_KEY, defaultValue: data }) + ); + const [hookState, setHookState] = result.current; + + act(() => { + setHookState(dataToCompare); + }); + + const dataToCompareStr = JSON.stringify(dataToCompare); + const dataSessionStorage = window.sessionStorage.getItem(SESSION_STORAGE_KEY); + + const [hookStateToCompare] = result.current; + expect(dataSessionStorage).toEqual(dataToCompareStr); + expect(hookStateToCompare).toStrictEqual(dataToCompare); + }); + + it('hook state must be preserved in case rerender', () => { + const data = { test: 7 }; + const { result, rerender } = renderHook(() => + useSessionStorage({ key: SESSION_STORAGE_KEY, defaultValue: data }) + ); + + rerender(); + + const [hookState, setHookState] = result.current; + + const dataToCompareStr = JSON.stringify(data); + const dataSessionStorage = window.sessionStorage.getItem(SESSION_STORAGE_KEY); + + expect(dataSessionStorage).toEqual(dataToCompareStr); + expect(hookState).toStrictEqual(data); + }); + + it('hook state must be preserved in case multiple operations and rerender', () => { + const data = { test: 8 }; + const dataToCompare = { test: 9 }; + const { result, rerender } = renderHook(() => + useSessionStorage({ key: SESSION_STORAGE_KEY, defaultValue: data }) + ); + const [hookState, setHookState] = result.current; + + act(() => { + setHookState(dataToCompare); + }); + + rerender(); + + const dataToCompareStr = JSON.stringify(dataToCompare); + const dataSessionStorage = window.sessionStorage.getItem(SESSION_STORAGE_KEY); + + const [hookStateToCompare] = result.current; + expect(dataSessionStorage).toEqual(dataToCompareStr); + expect(hookStateToCompare).toStrictEqual(dataToCompare); + }); + + it('session storage item should be cleared on unload when clearOnUnload is true', () => { + const data = { test: 10 }; + + renderHook(() => + useSessionStorage({ key: SESSION_STORAGE_KEY, defaultValue: data, clearOnUnload: true }) + ); + + window.dispatchEvent(new Event('unload')); + + const dataSessionStorage = window.sessionStorage.getItem(SESSION_STORAGE_KEY); + + expect(dataSessionStorage).toBeNull(); + }); + + it('session storage item should not be cleared on unload when clearOnUnload is defaulted false', () => { + const data = { test: 11 }; + + renderHook(() => useSessionStorage({ key: SESSION_STORAGE_KEY, defaultValue: data })); + + window.dispatchEvent(new Event('unload')); + + const dataToCompareStr = JSON.stringify(data); + const dataSessionStorage = window.sessionStorage.getItem(SESSION_STORAGE_KEY); + + expect(dataSessionStorage).toEqual(dataToCompareStr); + }); +}); diff --git a/platform/ui/src/hooks/useSessionStorage.tsx b/platform/ui/src/hooks/useSessionStorage.tsx new file mode 100644 index 00000000000..88790c35003 --- /dev/null +++ b/platform/ui/src/hooks/useSessionStorage.tsx @@ -0,0 +1,63 @@ +import { useState, useEffect, useCallback } from 'react'; + +/** + * A set of session storage keys that should be cleared out of session storage + * when the page unloads. + */ +const sessionKeysToClearOnUnload: Set = new Set(); + +const clearOnUnloadCallback = () => { + Array.from(sessionKeysToClearOnUnload.keys()).forEach(key => { + window.sessionStorage.removeItem(key); + }); + + // This is here for unit testing since there is no actual unload between tests + // and the set of keys need to be cleared between tests like it would in + // a browser between different sessions. + sessionKeysToClearOnUnload.clear(); +}; + +/** + * Technically there is no memory leak here because the listener needs to + * persist until the page unloads and once the page unloads it will be gone. + * + * ToDo: unload is not reliably fired on various browsers - particularly mobile. + * So on some systems the various session storage items will NOT be cleared + * when the page is refreshed or URL altered in some way. Alternatively, + * a 'visiblitychange' event could be used whereby on 'hidden' the session item + * is cleared but maintained in memory in case a 'visible' event is later fired. + */ +window.addEventListener('unload', clearOnUnloadCallback); + +type useSessionStorageProps = { + key: string; + defaultValue: unknown; + clearOnUnload: boolean; +}; + +const useSessionStorage = ({ + key, + defaultValue = {}, + clearOnUnload = false, +}: useSessionStorageProps) => { + const valueFromStorage = window.sessionStorage.getItem(key); + const storageValue = valueFromStorage ? JSON.parse(valueFromStorage) : defaultValue; + const [storeItem, setStoreItem] = useState({ ...storageValue }); + + const saveToSessionStorage = useCallback(value => { + setStoreItem({ ...value }); + window.sessionStorage.setItem(key, JSON.stringify(value)); + }, []); + + useEffect(() => { + saveToSessionStorage(storeItem); + }, []); + + if (clearOnUnload) { + sessionKeysToClearOnUnload.add(key); + } + + return [storeItem, saveToSessionStorage]; +}; + +export default useSessionStorage; diff --git a/platform/ui/src/index.js b/platform/ui/src/index.js index 1acacefd16d..69fc814fa53 100644 --- a/platform/ui/src/index.js +++ b/platform/ui/src/index.js @@ -119,6 +119,8 @@ export { ViewportOverlay, } from './components'; +export { useSessionStorage } from './hooks'; + /** These are mostly used in the docs */ export { getIcon, ICONS, addIcon } from './components/Icon/getIcon'; export { BackgroundColor } from './pages/Colors/BackgroundColor'; diff --git a/yarn.lock b/yarn.lock index 9b8ad2e7f4c..a52cf2fb73c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4860,6 +4860,14 @@ dependencies: defer-to-connect "^2.0.1" +"@testing-library/react-hooks@^3.2.1": + version "3.7.0" + resolved "https://registry.yarnpkg.com/@testing-library/react-hooks/-/react-hooks-3.7.0.tgz#6d75c5255ef49bce39b6465bf6b49e2dac84919e" + integrity sha512-TwfbY6BWtWIHitjT05sbllyLIProcysC0dF0q1bbDa7OHLC6A6rJOYJwZ13hzfz3O4RtOuInmprBozJRyyo7/g== + dependencies: + "@babel/runtime" "^7.12.5" + "@types/testing-library__react-hooks" "^3.4.0" + "@tootallnate/once@2": version "2.0.0" resolved "https://registry.yarnpkg.com/@tootallnate/once/-/once-2.0.0.tgz#f544a148d3ab35801c1f633a7441fd87c2e484bf" @@ -5322,6 +5330,13 @@ "@types/history" "^4.7.11" "@types/react" "*" +"@types/react-test-renderer@*": + version "18.0.5" + resolved "https://registry.yarnpkg.com/@types/react-test-renderer/-/react-test-renderer-18.0.5.tgz#b67a6ff37acd93d1b971ec4c838f69d52e772db0" + integrity sha512-PsnmF4Hpi61PTRX+dTxkjgDdtZ09kFFgPXczoF+yBfOVxn7xBLPvKP1BUrSasYHmerj33rhoJuvpIMsJuyRqHw== + dependencies: + "@types/react" "*" + "@types/react-transition-group@^4.4.0": version "4.4.6" resolved "https://registry.yarnpkg.com/@types/react-transition-group/-/react-transition-group-4.4.6.tgz#18187bcda5281f8e10dfc48f0943e2fdf4f75e2e" @@ -5437,6 +5452,13 @@ resolved "https://registry.yarnpkg.com/@types/tapable/-/tapable-1.0.8.tgz#b94a4391c85666c7b73299fd3ad79d4faa435310" integrity sha512-ipixuVrh2OdNmauvtT51o3d8z12p6LtFW9in7U79der/kwejjdNchQC5UMn5u/KxNoM7VHHOs/l8KS8uHxhODQ== +"@types/testing-library__react-hooks@^3.4.0": + version "3.4.1" + resolved "https://registry.yarnpkg.com/@types/testing-library__react-hooks/-/testing-library__react-hooks-3.4.1.tgz#b8d7311c6c1f7db3103e94095fe901f8fef6e433" + integrity sha512-G4JdzEcq61fUyV6wVW9ebHWEiLK2iQvaBuCHHn9eMSbZzVh4Z4wHnUGIvQOYCCYeu5DnUtFyNYuAAgbSaO/43Q== + dependencies: + "@types/react-test-renderer" "*" + "@types/tough-cookie@*": version "4.0.2" resolved "https://registry.yarnpkg.com/@types/tough-cookie/-/tough-cookie-4.0.2.tgz#6286b4c7228d58ab7866d19716f3696e03a09397" @@ -17331,7 +17353,7 @@ react-is@18.1.0: resolved "https://registry.yarnpkg.com/react-is/-/react-is-18.1.0.tgz#61aaed3096d30eacf2a2127118b5b41387d32a67" integrity sha512-Fl7FuabXsJnV5Q1qIOQwx/sagGF18kogb4gpfcG4gjLBWO0WDiiz1ko/ExayuxE7InyQkBLkxRFG5oxY6Uu3Kg== -react-is@^16.13.1, react-is@^16.6.0, react-is@^16.7.0, react-is@^16.8.4: +react-is@^16.13.1, react-is@^16.6.0, react-is@^16.7.0, react-is@^16.8.4, react-is@^16.8.6: version "16.13.1" resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.13.1.tgz#789729a4dc36de2999dc156dd6c1d9c18cea56a4" integrity sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ== @@ -17528,6 +17550,16 @@ react-style-singleton@^2.2.1: invariant "^2.2.4" tslib "^2.0.0" +react-test-renderer@^16.12.0: + version "16.14.0" + resolved "https://registry.yarnpkg.com/react-test-renderer/-/react-test-renderer-16.14.0.tgz#e98360087348e260c56d4fe2315e970480c228ae" + integrity sha512-L8yPjqPE5CZO6rKsKXRO/rVPiaCOy0tQQJbC+UjPNlobl5mad59lvPjwFsQHTvL03caVDIVr9x9/OSgDe6I5Eg== + dependencies: + object-assign "^4.1.1" + prop-types "^15.6.2" + react-is "^16.8.6" + scheduler "^0.19.1" + react-textarea-autosize@^8.3.2: version "8.5.2" resolved "https://registry.yarnpkg.com/react-textarea-autosize/-/react-textarea-autosize-8.5.2.tgz#6421df2b5b50b9ca8c5e96fd31be688ea7fa2f9d" @@ -18323,6 +18355,14 @@ saxes@^6.0.0: dependencies: xmlchars "^2.2.0" +scheduler@^0.19.1: + version "0.19.1" + resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.19.1.tgz#4f3e2ed2c1a7d65681f4c854fa8c5a1ccb40f196" + integrity sha512-n/zwRWRYSUj0/3g/otKDRPMh6qv2SYMWNq85IEa8iZyAv8od9zDYpGSnpBEjNgcMNq6Scbu5KfIPxNF72R/2EA== + dependencies: + loose-envify "^1.1.0" + object-assign "^4.1.1" + scheduler@^0.20.2: version "0.20.2" resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.20.2.tgz#4baee39436e34aa93b4874bddcbf0fe8b8b50e91" From a1ae4f9bc385188d0e64848634be70eca930a329 Mon Sep 17 00:00:00 2001 From: Joe Boccanfuso Date: Fri, 27 Oct 2023 09:23:54 -0400 Subject: [PATCH 2/5] Fix cypress tests - clear the session storage between tests for the study list. --- .../app/cypress/integration/study-list/OHIFStudyList.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js b/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js index fb134315869..462500e64fa 100644 --- a/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js +++ b/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js @@ -3,6 +3,7 @@ describe('OHIF Study List', function () { context('Desktop resolution', function () { beforeEach(function () { + window.sessionStorage.clear(); cy.openStudyList(); cy.viewport(1750, 720); From 820e7b868b1c87500e8f751b5ff3caf3663431e7 Mon Sep 17 00:00:00 2001 From: Joe Boccanfuso Date: Fri, 27 Oct 2023 09:53:03 -0400 Subject: [PATCH 3/5] Fix cypress tests. --- .../cypress/integration/study-list/OHIFStudyList.spec.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js b/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js index 462500e64fa..abfb9c77d74 100644 --- a/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js +++ b/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js @@ -3,7 +3,7 @@ describe('OHIF Study List', function () { context('Desktop resolution', function () { beforeEach(function () { - window.sessionStorage.clear(); + cy.window().then(win => win.sessionStorage.clear()); cy.openStudyList(); cy.viewport(1750, 720); @@ -15,6 +15,10 @@ describe('OHIF Study List', function () { cy.get('@StudyDescription').clear(); }); + afterEach(function () { + cy.window().then(win => win.sessionStorage.clear()); + }); + it('Displays several studies initially', function () { cy.waitStudyList(); cy.get('@searchResult2').should($list => { From fa54f19e5fc41a8aaca59a6456f807841c70a4ac Mon Sep 17 00:00:00 2001 From: Joe Boccanfuso Date: Fri, 27 Oct 2023 13:48:53 -0400 Subject: [PATCH 4/5] PR feedback. Swicthed to using a visibiliychange listener to better detect when the page gets unloaded. --- platform/app/src/routes/WorkList/WorkList.tsx | 4 +- .../ui/src/hooks/useSessionStorage.test.js | 27 -------- platform/ui/src/hooks/useSessionStorage.tsx | 64 +++++++++++-------- 3 files changed, 38 insertions(+), 57 deletions(-) diff --git a/platform/app/src/routes/WorkList/WorkList.tsx b/platform/app/src/routes/WorkList/WorkList.tsx index 503a2c87e4f..8e8af65f5ed 100644 --- a/platform/app/src/routes/WorkList/WorkList.tsx +++ b/platform/app/src/routes/WorkList/WorkList.tsx @@ -61,7 +61,7 @@ function WorkList({ const navigate = useNavigate(); const STUDIES_LIMIT = 101; const queryFilterValues = _getQueryFilterValues(searchParams); - const [sessionQueryFilterValues, saveSessionQueryFilterValues] = useSessionStorage({ + const [sessionQueryFilterValues, updateSessionQueryFilterValues] = useSessionStorage({ key: 'queryFilterValues', defaultValue: queryFilterValues, // ToDo: useSessionStorage currently uses an unload listener to clear the filters from session storage @@ -128,7 +128,7 @@ function WorkList({ val.pageNumber = 1; } _setFilterValues(val); - saveSessionQueryFilterValues(val); + updateSessionQueryFilterValues(val); setExpandedRows([]); }; diff --git a/platform/ui/src/hooks/useSessionStorage.test.js b/platform/ui/src/hooks/useSessionStorage.test.js index 99f895826ac..bd3645db249 100644 --- a/platform/ui/src/hooks/useSessionStorage.test.js +++ b/platform/ui/src/hooks/useSessionStorage.test.js @@ -99,31 +99,4 @@ describe('Hook Session Storage', () => { expect(dataSessionStorage).toEqual(dataToCompareStr); expect(hookStateToCompare).toStrictEqual(dataToCompare); }); - - it('session storage item should be cleared on unload when clearOnUnload is true', () => { - const data = { test: 10 }; - - renderHook(() => - useSessionStorage({ key: SESSION_STORAGE_KEY, defaultValue: data, clearOnUnload: true }) - ); - - window.dispatchEvent(new Event('unload')); - - const dataSessionStorage = window.sessionStorage.getItem(SESSION_STORAGE_KEY); - - expect(dataSessionStorage).toBeNull(); - }); - - it('session storage item should not be cleared on unload when clearOnUnload is defaulted false', () => { - const data = { test: 11 }; - - renderHook(() => useSessionStorage({ key: SESSION_STORAGE_KEY, defaultValue: data })); - - window.dispatchEvent(new Event('unload')); - - const dataToCompareStr = JSON.stringify(data); - const dataSessionStorage = window.sessionStorage.getItem(SESSION_STORAGE_KEY); - - expect(dataSessionStorage).toEqual(dataToCompareStr); - }); }); diff --git a/platform/ui/src/hooks/useSessionStorage.tsx b/platform/ui/src/hooks/useSessionStorage.tsx index 88790c35003..e1c16282b58 100644 --- a/platform/ui/src/hooks/useSessionStorage.tsx +++ b/platform/ui/src/hooks/useSessionStorage.tsx @@ -1,33 +1,36 @@ import { useState, useEffect, useCallback } from 'react'; /** - * A set of session storage keys that should be cleared out of session storage + * A map of session storage items that should be cleared out of session storage * when the page unloads. */ -const sessionKeysToClearOnUnload: Set = new Set(); +const sessionItemsToClearOnUnload: Map = new Map(); -const clearOnUnloadCallback = () => { - Array.from(sessionKeysToClearOnUnload.keys()).forEach(key => { - window.sessionStorage.removeItem(key); - }); - - // This is here for unit testing since there is no actual unload between tests - // and the set of keys need to be cleared between tests like it would in - // a browser between different sessions. - sessionKeysToClearOnUnload.clear(); +/** + * This callback simulates clearing the various session items when a page unloads. + * When the page is hidden the session storage items are removed but maintained + * in the map above in case the page becomes visible again. So those pages that + * are hidden because they are being unloaded have their session storage disposed + * of for ever. For those pages that are hidden, but later return to visible, + * this callback restores the session storage from the map above. + */ +const visibilityChangeCallback = () => { + if (document.visibilityState === 'hidden') { + Array.from(sessionItemsToClearOnUnload.keys()).forEach(key => { + window.sessionStorage.removeItem(key); + }); + } else { + Array.from(sessionItemsToClearOnUnload.keys()).forEach(key => { + window.sessionStorage.setItem(key, sessionItemsToClearOnUnload.get(key)); + }); + } }; /** * Technically there is no memory leak here because the listener needs to * persist until the page unloads and once the page unloads it will be gone. - * - * ToDo: unload is not reliably fired on various browsers - particularly mobile. - * So on some systems the various session storage items will NOT be cleared - * when the page is refreshed or URL altered in some way. Alternatively, - * a 'visiblitychange' event could be used whereby on 'hidden' the session item - * is cleared but maintained in memory in case a 'visible' event is later fired. */ -window.addEventListener('unload', clearOnUnloadCallback); +document.addEventListener('visibilitychange', visibilityChangeCallback); type useSessionStorageProps = { key: string; @@ -42,22 +45,27 @@ const useSessionStorage = ({ }: useSessionStorageProps) => { const valueFromStorage = window.sessionStorage.getItem(key); const storageValue = valueFromStorage ? JSON.parse(valueFromStorage) : defaultValue; - const [storeItem, setStoreItem] = useState({ ...storageValue }); + const [sessionItem, setSessionItem] = useState({ ...storageValue }); + + const updateSessionItem = useCallback(value => { + setSessionItem({ ...value }); - const saveToSessionStorage = useCallback(value => { - setStoreItem({ ...value }); - window.sessionStorage.setItem(key, JSON.stringify(value)); + const valueAsStr = JSON.stringify(value); + + if (!clearOnUnload || document.visibilityState === 'visible') { + window.sessionStorage.setItem(key, valueAsStr); + } + + if (clearOnUnload) { + sessionItemsToClearOnUnload.set(key, valueAsStr); + } }, []); useEffect(() => { - saveToSessionStorage(storeItem); + updateSessionItem(sessionItem); }, []); - if (clearOnUnload) { - sessionKeysToClearOnUnload.add(key); - } - - return [storeItem, saveToSessionStorage]; + return [sessionItem, updateSessionItem]; }; export default useSessionStorage; From dcfdf5f7ef5ddd5d91127e596310cad4453bd0a2 Mon Sep 17 00:00:00 2001 From: Joe Boccanfuso Date: Fri, 27 Oct 2023 14:42:58 -0400 Subject: [PATCH 5/5] Updated accession number for one of the cypress tests. --- .../app/cypress/integration/study-list/OHIFStudyList.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js b/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js index abfb9c77d74..d1c32e08811 100644 --- a/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js +++ b/platform/app/cypress/integration/study-list/OHIFStudyList.spec.js @@ -89,7 +89,7 @@ describe('OHIF Study List', function () { }); it('maintains Accession filter upon return from viewer', function () { - cy.get('@AccessionNumber').type('321'); + cy.get('@AccessionNumber').type('0000155811'); //Wait result list to be displayed cy.waitStudyList(); cy.get('[data-cy="studyRow-1.3.6.1.4.1.25403.345050719074.3824.20170125113417.1"]').click(); @@ -99,7 +99,7 @@ describe('OHIF Study List', function () { cy.get('[data-cy="return-to-work-list"]').click(); cy.get('@searchResult2').should($list => { expect($list.length).to.be.eq(1); - expect($list).to.contain('321'); + expect($list).to.contain('0000155811'); }); });