Skip to content

Commit

Permalink
Fix error messages when PDF is opened in multiple tabs. Fix #554
Browse files Browse the repository at this point in the history
* Resolves an issue where having the document open and scrolled in
  multiple places would result in a repeated error message about remote
  changes.
* If a remote update is detected (using a websocket), the web library
  will now fetch the latest page index for the currently viewed
  document.
* If sending the page index results in a conflict, local data is updated
  to match the server. The user's view of the document remains
  unaffected.
  • Loading branch information
tnajdek committed Jul 17, 2024
1 parent dd3f225 commit 8783c0b
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 28 deletions.
24 changes: 12 additions & 12 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
"redux-async-queue": "^1.0.0",
"redux-thunk": "^3.1.0",
"use-debounce": "^10.0.1",
"zotero-api-client": "^0.43.1"
"zotero-api-client": "^0.44.0"
},
"devDependencies": {
"@babel/core": "^7.24.7",
Expand Down
48 changes: 35 additions & 13 deletions src/js/actions/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const fetchLibrarySettings = (libraryKey, settingsKey) => {
}


const updateLibrarySettings = (settingsKey, value, libraryKey = null) => {
const updateLibrarySettings = (settingsKey, value, libraryKey = null, options) => {
return async (dispatch, getState) => {
libraryKey = libraryKey ?? getState().current.libraryKey;
const id = requestTracker.id++;
Expand All @@ -76,24 +76,25 @@ const updateLibrarySettings = (settingsKey, value, libraryKey = null) => {
});

dispatch(
queueUpdateLibrarySettings(settingsKey, value, libraryKey, { resolve, reject, id })
queueUpdateLibrarySettings(settingsKey, value, libraryKey, options, { resolve, reject, id })
);
});
};
}

const queueUpdateLibrarySettings = (settingsKey, value, libraryKey, { resolve, reject, id }) => {
const queueUpdateLibrarySettings = (settingsKey, value, libraryKey, options, { resolve, reject, id }) => {
return {
queue: `${libraryKey}:${settingsKey}`, // independent queue for each library/settingsKey pair, does not block libraryKey queue
callback: async (next, dispatch, getState) => {
const state = getState();
const config = state.config;
const oldValue = state.libraries?.[libraryKey].settings?.entries?.[settingsKey];
const version = oldValue?.version ?? 0;
const version = options?.version ?? oldValue?.version ?? 0;

if(oldValue?.value === value) {
dispatch({
type: CANCEL_UPDATE_LIBRARY_SETTINGS,
reason: 'no-change',
settingsKey,
value,
libraryKey,
Expand Down Expand Up @@ -130,15 +131,36 @@ const queueUpdateLibrarySettings = (settingsKey, value, libraryKey, { resolve, r
});
resolve();
} catch(error) {
dispatch({
type: ERROR_UPDATE_LIBRARY_SETTINGS,
settingsKey,
value,
libraryKey,
error
});
reject(error);
throw error;
if (error?.response?.status === 412 && (options.force || options.ignore)) {
// remote version is newer, but we've been asked to force or ignore conflict
let serverVersion = error.getVersion();
dispatch({
type: CANCEL_UPDATE_LIBRARY_SETTINGS,
reason: options.force ? 'conflict-force' : 'conflict-ignore',
localVersion: version,
serverVersion,
settingsKey,
value,
libraryKey,
id
});
if (options.ignore) {
dispatch(fetchLibrarySettings(libraryKey, settingsKey));
} else if (options.force) {
dispatch(updateLibrarySettings(settingsKey, value, libraryKey, { version: serverVersion }));
}
resolve();
} else {
dispatch({
type: ERROR_UPDATE_LIBRARY_SETTINGS,
settingsKey,
value,
libraryKey,
error
});
reject(error);
throw error;
}
} finally {
next();
}
Expand Down
5 changes: 5 additions & 0 deletions src/js/actions/remote.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { STREAMING_REMOTE_LIBRARY_UPDATE } from '../constants/actions';
import { get } from '../utils';
import { getLastPageIndexSettingKey } from '../common/item';
import { fetchDeletedContentSince, fetchAllCollectionsSince, fetchAllItemsSince, fetchLibrarySettings } from '.';

const remoteLibraryUpdate = (libraryKey, version) => {
Expand All @@ -12,6 +13,10 @@ const remoteLibraryUpdate = (libraryKey, version) => {
dispatch(fetchAllItemsSince(oldVersion, { includeTrashed: 1 }, { current: { libraryKey } }));
dispatch(fetchAllCollectionsSince(oldVersion, libraryKey));
dispatch(fetchDeletedContentSince(oldVersion, libraryKey));
if (state.current.view === 'reader' && state.current.attachmentKey) {
const pageIndexSettingKey = getLastPageIndexSettingKey(state.current.attachmentKey, state.current.libraryKey);
dispatch(fetchLibrarySettings(libraryKey, pageIndexSettingKey));
}
}
}
}
Expand Down
17 changes: 15 additions & 2 deletions src/js/component/reader.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ const readerReducer = (state, action) => {
return { ...state, isRouteConfirmed: true }
case 'COMPLETE_FETCH_SETTINGS':
return { ...state, isSettingsFetched: true }
case 'BEGIN_PUSH_SETTINGS':
return { ...state, isSettingPushRequired: true, newSettings: action.value }
case 'COMPLETE_PUSH_SETTINGS':
return { ...state, isSettingPushRequired: false, newSettings: null }
case 'BEGIN_FETCH_DATA':
return { ...state, dataState: FETCHING };
case 'COMPLETE_FETCH_DATA':
Expand Down Expand Up @@ -188,6 +192,8 @@ const Reader = () => {
isReady: false,
isRouteConfirmed: false,
isSettingsFetched: false,
isSettingPushRequired: false,
newSettings: null,
data: null,
dataState: UNFETCHED,
annotationsState: NOT_IMPORTED,
Expand Down Expand Up @@ -265,9 +271,9 @@ const Reader = () => {
const handleChangeViewState = useDebouncedCallback(useCallback((newViewState, isPrimary) => {
const pageIndexKey = PAGE_INDEX_KEY_LOOKUP[attachmentItem?.contentType];
if (isPrimary && userLibraryKey && pageIndexKey) {
dispatch(updateLibrarySettings(pageIndexSettingKey, newViewState[pageIndexKey], userLibraryKey));
dispatchState({ type: 'BEGIN_PUSH_SETTINGS', value: newViewState[pageIndexKey] });
}
}, [attachmentItem, dispatch, pageIndexSettingKey, userLibraryKey]), 1000);
}, [attachmentItem, userLibraryKey]), 1000);

// NOTE: handler can't be updated once it has been passed to Reader
const handleToggleSidebar = useDebouncedCallback(useCallback((isOpen) => {
Expand Down Expand Up @@ -496,6 +502,13 @@ const Reader = () => {
}
}, [rotatePages, state.action, state.data, state.isReady]);

useEffect(() => {
if (state.isSettingsFetched && state.isSettingPushRequired) {
dispatch(updateLibrarySettings(pageIndexSettingKey, state.newSettings, userLibraryKey, { ignore: true }));
dispatchState({ type: 'COMPLETE_PUSH_SETTINGS' });
}
}, [dispatch, pageIndexSettingKey, state.isSettingPushRequired, state.isSettingsFetched, state.newSettings, userLibraryKey]);

return (
<section className="reader-wrapper" onKeyDown={handleKeyDown} tabIndex="0">
{isReaderCompatibleBrowser() ? (
Expand Down

0 comments on commit 8783c0b

Please sign in to comment.