From a497e55c7189260d2d9ac4a5d9aeb78373a14cb9 Mon Sep 17 00:00:00 2001 From: Tom Najdek Date: Tue, 11 Feb 2025 14:48:18 +0100 Subject: [PATCH] Fix reader location not persisted if tab closed quickly #599 This change needs to be persisted remotely so that the value can be synced to other devices running Zotero. To achieve this, we use a fetch request with the `keepalive` option, ensuring the request is executed even after the tab is closed. --- package-lock.json | 34 ++++++++++++------------ package.json | 2 +- src/js/component/reader.jsx | 12 +++++---- src/js/hooks/index.js | 1 + src/js/hooks/use-tracked-settings-key.js | 26 ++++++++++++++++++ 5 files changed, 52 insertions(+), 23 deletions(-) create mode 100644 src/js/hooks/use-tracked-settings-key.js diff --git a/package-lock.json b/package-lock.json index a879f6c3..8f20ab85 100644 --- a/package-lock.json +++ b/package-lock.json @@ -42,7 +42,7 @@ "redux-async-queue": "^1.0.0", "redux-thunk": "^3.1.0", "use-debounce": "^10.0.4", - "zotero-api-client": "^0.46.0" + "zotero-api-client": "^0.47.0" }, "devDependencies": { "@babel/core": "^7.26.0", @@ -1872,9 +1872,9 @@ } }, "node_modules/@babel/runtime": { - "version": "7.26.0", - "resolved": "https://registry.npmjs.org/@babel/runtime/-/runtime-7.26.0.tgz", - "integrity": "sha512-FDSOghenHTiToteC/QRlv2q3DhPZ/oOXTBoirfWNx1Cx3TMVcGWQtMMmQcSvb/JjpNeGzx8Pq/b4fKEJuWm1sw==", + "version": "7.26.7", + "resolved": "https://registry.npmjs.org/@babel/runtime/-/runtime-7.26.7.tgz", + "integrity": "sha512-AOPI3D+a8dXnja+iwsUqGRjr1BbZIe771sXdapOtYI531gSqpi92vXivKcq2asu/DFpdl1ceFAKZyRzK2PCVcQ==", "dependencies": { "regenerator-runtime": "^0.14.0" }, @@ -1883,9 +1883,9 @@ } }, "node_modules/@babel/runtime-corejs3": { - "version": "7.26.0", - "resolved": "https://registry.npmjs.org/@babel/runtime-corejs3/-/runtime-corejs3-7.26.0.tgz", - "integrity": "sha512-YXHu5lN8kJCb1LOb9PgV6pvak43X2h4HvRApcN5SdWeaItQOzfn1hgP6jasD6KWQyJDBxrVmA9o9OivlnNJK/w==", + "version": "7.26.7", + "resolved": "https://registry.npmjs.org/@babel/runtime-corejs3/-/runtime-corejs3-7.26.7.tgz", + "integrity": "sha512-55gRV8vGrCIYZnaQHQrD92Lo/hYE3Sj5tmbuf0hhHR7sj2CWhEhHU89hbq+UVDXvFG1zUVXJhUkEq1eAfqXtFw==", "dependencies": { "core-js-pure": "^3.30.2", "regenerator-runtime": "^0.14.0" @@ -6194,11 +6194,11 @@ } }, "node_modules/cross-fetch": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/cross-fetch/-/cross-fetch-4.0.0.tgz", - "integrity": "sha512-e4a5N8lVvuLgAWgnCrLr2PP0YyDOTHa9H/Rj54dirp61qXnNq46m82bRhNqIA5VccJtWBvPTFRV3TtvHUKPB1g==", + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/cross-fetch/-/cross-fetch-4.1.0.tgz", + "integrity": "sha512-uKm5PU+MHTootlWEY+mZ4vvXoCn4fLQxT9dSc1sXVMSFkINTJVN8cAQROpwcKm8bJ/c7rgZVIBWzH5T78sNZZw==", "dependencies": { - "node-fetch": "^2.6.12" + "node-fetch": "^2.7.0" } }, "node_modules/cross-spawn": { @@ -17663,13 +17663,13 @@ } }, "node_modules/zotero-api-client": { - "version": "0.46.0", - "resolved": "https://registry.npmjs.org/zotero-api-client/-/zotero-api-client-0.46.0.tgz", - "integrity": "sha512-X6ojpKO6GCW6mX+w0WP11YXYFq30ahT6y+dPSl0hL/TOWepN8i3vWjsZ00S7k8XweAZ8GUkrlc4CTlbDgDYW2w==", + "version": "0.47.0", + "resolved": "https://registry.npmjs.org/zotero-api-client/-/zotero-api-client-0.47.0.tgz", + "integrity": "sha512-SAvdbPo6GLjWsHGt/WqfL32iphSD38i7+AI6fb4pd6MYNGildnd8ygXOd66xNHeA0ThgRhNvk0dtC+mgP/E+lg==", "dependencies": { - "@babel/runtime": "^7.26.0", - "@babel/runtime-corejs3": "^7.26.0", - "cross-fetch": "^4.0.0", + "@babel/runtime": "^7.26.7", + "@babel/runtime-corejs3": "^7.26.7", + "cross-fetch": "^4.1.0", "spark-md5": "^3.0.2" }, "engines": { diff --git a/package.json b/package.json index 293f4275..f0fe7dc4 100644 --- a/package.json +++ b/package.json @@ -102,7 +102,7 @@ "redux-async-queue": "^1.0.0", "redux-thunk": "^3.1.0", "use-debounce": "^10.0.4", - "zotero-api-client": "^0.46.0" + "zotero-api-client": "^0.47.0" }, "devDependencies": { "@babel/core": "^7.26.0", diff --git a/src/js/component/reader.jsx b/src/js/component/reader.jsx index 4e3ae522..9425a0f7 100644 --- a/src/js/component/reader.jsx +++ b/src/js/component/reader.jsx @@ -18,7 +18,7 @@ import { } from '../actions'; import { PDFWorker } from '../common/pdf-worker'; import { getItemViewState, updateItemViewState } from '../common/viewstate' -import { useFetchingState } from '../hooks'; +import { useFetchingState, useTrackedSettingsKey } from '../hooks'; import TagPicker from './item-details/tag-picker'; import { READER_CONTENT_TYPES } from '../constants/reader'; import Portal from './portal'; @@ -166,7 +166,7 @@ const Reader = () => { }); const location = useSelector(state => state.current.location); const pageIndexSettingKey = getLastPageIndexSettingKey(attachmentKey, libraryKey); - const locationValue = useSelector(state => state.libraries[userLibraryKey]?.settings?.entries?.[pageIndexSettingKey]?.value ?? null); + const { value: locationValue, update: updateLocationValueSync } = useTrackedSettingsKey(pageIndexSettingKey, userLibraryKey); const attachmentItem = useSelector(state => state.libraries[libraryKey]?.items[attachmentKey]); const isFetchingUrl = useSelector(state => state.libraries[libraryKey]?.attachmentsUrl[attachmentKey]?.isFetching ?? false); const url = useSelector(state => state.libraries[libraryKey]?.attachmentsUrl[attachmentKey]?.url); @@ -380,11 +380,13 @@ const Reader = () => { state.importedAnnotations, state.viewState ]); - const handleOnBeforeUnload = useCallback(async () => { + const handleOnBeforeUnload = useCallback(() => { if (pendingViewState.current) { - await updateItemViewState(attachmentItem.key, libraryKey, pendingViewState.current); + const pageIndexKey = PAGE_INDEX_KEY_LOOKUP[attachmentItem.contentType]; + updateItemViewState(attachmentItem.key, libraryKey, pendingViewState.current); + updateLocationValueSync(pendingViewState.current[pageIndexKey]); } - }, [attachmentItem, libraryKey]); + }, [attachmentItem, libraryKey, updateLocationValueSync]); useEffect(() => { // pdf js stores last page in localStorage but we want to use one from user library settings instead diff --git a/src/js/hooks/index.js b/src/js/hooks/index.js index 7e5bd8d2..cdf76da7 100644 --- a/src/js/hooks/index.js +++ b/src/js/hooks/index.js @@ -4,3 +4,4 @@ export * from './use-navigation-state'; export * from './state'; export * from './use-item-action-handlers'; export * from './use-items-count'; +export * from './use-tracked-settings-key'; diff --git a/src/js/hooks/use-tracked-settings-key.js b/src/js/hooks/use-tracked-settings-key.js new file mode 100644 index 00000000..fa224ca9 --- /dev/null +++ b/src/js/hooks/use-tracked-settings-key.js @@ -0,0 +1,26 @@ +import api from 'zotero-api-client'; + +import { useSelector } from 'react-redux'; + +// Normally components don't need to track version number of the setting (to avoid re-renders when +// the version changes) and updates are dispatched asynchonously (and might be queued, e.g., to +// avoid out-of-order updates). However, to support updating the value on tab/window close, we need +// a synchroneus version with keepalive fetch call. This hook tracks version of the setting and +// provides a method to update the setting synchronously, but using this hook might cause re-renders. +export const useTrackedSettingsKey = (settingsKey, libraryKey) => { + const config = useSelector(state => state.config); + const value = useSelector(state => state.libraries[libraryKey]?.settings?.entries?.[settingsKey]?.value ?? null); + const version = useSelector(state => state.libraries[libraryKey]?.settings?.entries?.[settingsKey]?.version ?? null); + + const update = (newValue) => { + if(newValue !== value) { + api(config.apiKey, config.apiConfig) + .library(libraryKey) + .version(version) + .settings(settingsKey) + .put({ value: newValue }, { keepalive: true }); + } + } + + return { value, update }; +};