Fix "Go to Page" not working in the note editor

In order to make this work, two changes had to be implemented:

* Reader now redirects to the best attachment if URL is pointing at a non-attachment item
* Reader now accepts location via URL

src/js/actions/attachments.js

@@ -1,19 +1,11 @@
 import { fetchAllChildItems, fetchChildItems, getAttachmentUrl } from '.';
-import { cleanDOI, cleanURL, get, getDOIURL, openDelayedURL } from '../utils';
+import { cleanDOI, cleanURL, get, getDOIURL, getItemFromApiUrl, openDelayedURL } from '../utils';
 import { makePath } from '../common/navigation';
 import { PDFWorker } from '../common/pdf-worker.js';
 import { REQUEST_EXPORT_PDF, RECEIVE_EXPORT_PDF, ERROR_EXPORT_PDF  } from '../constants/actions';
 import { saveAs } from 'file-saver';
 import { READER_CONTENT_TYPES } from '../constants/reader';
 
-const extractItemKey = url => {
-	const matchResult = url.match(/\/items\/([A-Z0-9]{8})/);
-	if(matchResult) {
-		return matchResult[1];
-	}
-	return null;
-}
-
 const tryGetFirstLink = itemKey => {
 	return async (dispatch, getState) => {
 		const state = getState();
@@ -83,7 +75,7 @@ const pickBestItemAction = itemKey => {
 		const current = state.current;
 		const attachment = get(item, [Symbol.for('links'), 'attachment'], null);
 		if(attachment) {
-			const attachmentItemKey = extractItemKey(attachment.href);
+			const attachmentItemKey = getItemFromApiUrl(attachment.href);
 			if (Object.keys(READER_CONTENT_TYPES).includes(attachment.attachmentType)) {
 				// "best" attachment is PDF, open in reader
 				const readerPath = makePath(state.config, {

src/js/actions/navigate.js

@@ -17,6 +17,21 @@ const pushEmbedded = (path) => ({
 	}
 });
 
+const currentToPath = current => ({
+	attachmentKey: current.attachmentKey,
+	collection: current.collectionKey,
+	items: current.itemKeys,
+	library: current.libraryKey,
+	noteKey: current.noteKey,
+	publications: current.isMyPublications,
+	qmode: current.qmode,
+	search: current.search,
+	tags: current.tags,
+	trash: current.isTrash,
+	view: current.view,
+	location: current.location,
+});
+
 const navigate = (path, isAbsolute = false) => {
 	return async (dispatch, getState) => {
 		const { config, current } = getState();
@@ -27,17 +42,7 @@ const navigate = (path, isAbsolute = false) => {
 			dispatch(pushFn(configuredPath));
 		} else {
 			const updatedPath = {
-				attachmentKey: current.attachmentKey,
-				collection: current.collectionKey,
-				items: current.itemKeys,
-				library: current.libraryKey,
-				noteKey: current.noteKey,
-				publications: current.isMyPublications,
-				qmode: current.qmode,
-				search: current.search,
-				tags: current.tags,
-				trash: current.isTrash,
-				view: current.view,
+				...currentToPath(current),
 				...path
 			};
 			const configuredPath = makePath(config, updatedPath);
@@ -46,6 +51,18 @@ const navigate = (path, isAbsolute = false) => {
 	}
 };
 
+const openInReader = (path) => {
+	return async (dispatch, getState) => {
+		const state = getState();
+		const readerPath = makePath(state.config, {
+			...path,
+			view: 'reader',
+		});
+
+		return window.open(readerPath);
+	}
+};
+
 const selectItemsKeyboard = (direction, magnitude, isMultiSelect) => {
 	return async (dispatch, getState) => {
 		const state = getState();
@@ -235,4 +252,4 @@ const redirectIfCollectionNotFound = () => {
 	}
 }
 
-export { navigate, navigateExitSearch, redirectIfCollectionNotFound, selectFirstItem, selectItemsKeyboard, selectItemsMouse, selectLastItem };
+export { navigate, navigateExitSearch, openInReader, redirectIfCollectionNotFound, selectFirstItem, selectItemsKeyboard, selectItemsMouse, selectLastItem };

src/js/common/navigation.js

@@ -13,7 +13,10 @@ const tagsToUrlPart = tags => tags.map(t => encodeURIComponent(t)).join(',');
 
 const makePath = (config, { library = null, collection = null,
 	items = null, trash = false, publications = false, tags = null,
-	search = null, qmode = null, view = null, noteKey = null, attachmentKey = null } = {}) => {
+	search = null, qmode = null, view = null, noteKey = null, attachmentKey = null,
+	location = null } = {}
+	) => {
+
 	const path = [];
 
 	if(library !== null) {
@@ -67,6 +70,11 @@ const makePath = (config, { library = null, collection = null,
 		path.push(view);
 	}
 
+	if (location && ['pageNumber', 'annotationID', 'position', 'href'].includes(Object.keys(location)[0])) {
+		path.push(Object.keys(location)[0])
+		path.push(location[Object.keys(location)[0]])
+	}
+
 	return '/' + path.join('/');
 }
 

src/js/component/reader.jsx

@@ -13,15 +13,15 @@ import { annotationItemToJSON } from '../common/annotations.js';
 import { ERROR_PROCESSING_ANNOTATIONS } from '../constants/actions';
 import {
 	deleteItems, fetchChildItems, fetchItemDetails, fetchLibrarySettings, navigate, tryGetAttachmentURL,
-	patchAttachment, postAnnotationsFromReader, uploadAttachment, updateLibrarySettings,
-	preferenceChange
+	patchAttachment, postAnnotationsFromReader,  uploadAttachment, updateLibrarySettings, preferenceChange
 } from '../actions';
 import { PDFWorker } from '../common/pdf-worker.js';
 import { useFetchingState } from '../hooks';
 import { strings } from '../constants/strings.js';
 import TagPicker from './item-details/tag-picker.jsx';
 import { READER_CONTENT_TYPES } from '../constants/reader.js';
 import Portal from './portal';
+import { getItemFromApiUrl } from '../utils';
 
 const PAGE_SIZE = 100;
 
@@ -102,6 +102,10 @@ PopupPortal.propTypes = {
 
 const readerReducer = (state, action) => {
 	switch (action.type) {
+		case 'ROUTE_CONFIRMED':
+			return { ...state, isRouteConfirmed: true }
+		case 'COMPLETE_FETCH_SETTINGS':
+			return { ...state, isSettingsFetched: true }
 		case 'BEGIN_FETCH_DATA':
 			return { ...state, dataState: FETCHING };
 		case 'COMPLETE_FETCH_DATA':
@@ -146,6 +150,7 @@ const Reader = () => {
 			return null;
 		}
 	});
+	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 attachmentItem = useSelector(state => state.libraries[libraryKey]?.items[attachmentKey]);
@@ -171,12 +176,15 @@ const Reader = () => {
 	});
 	const isReaderSidebarOpen = useSelector(state => state.preferences?.isReaderSidebarOpen);
 	const readerSidebarWidth = useSelector(state => state.preferences?.readerSidebarWidth);
+	// TODO: this should be entry-sepcific, but works for now because there is only one entry being fetched by the reader
 	const isFetchingUserLibrarySettings = useSelector(state => state.libraries[userLibraryKey]?.settings?.isFetching);
 	const pdfWorker = useMemo(() => new PDFWorker({ pdfWorkerURL, pdfReaderCMapsRoot }), [pdfReaderCMapsRoot, pdfWorkerURL]);
 
 	const [state, dispatchState] = useReducer(readerReducer, {
 		action: null,
 		isReady: false,
+		isRouteConfirmed: false,
+		isSettingsFetched: false,
 		data: null,
 		dataState: UNFETCHED,
 		annotationsState: NOT_IMPORTED,
@@ -252,8 +260,8 @@ const Reader = () => {
 
 	// NOTE: handler can't be updated once it has been passed to Reader
 	const handleChangeViewState = useDebouncedCallback(useCallback((newViewState, isPrimary) => {
-		const pageIndexKey = PAGE_INDEX_KEY_LOOKUP[attachmentItem.contentType];
-		if (isPrimary && userLibraryKey) {
+		const pageIndexKey = PAGE_INDEX_KEY_LOOKUP[attachmentItem?.contentType];
+		if (isPrimary && userLibraryKey && pageIndexKey) {
 			dispatch(updateLibrarySettings(pageIndexSettingKey, newViewState[pageIndexKey], userLibraryKey));
 		}
 	}, [attachmentItem, dispatch, pageIndexSettingKey, userLibraryKey]), 1000);
@@ -285,7 +293,7 @@ const Reader = () => {
 			annotations: [...processedAnnotations, ...state.importedAnnotations],
 			primaryViewState: readerState,
 			secondaryViewState: null,
-			location: null,
+			location,
 			readOnly: isReadOnly,
 			authorName: isGroup ? currentUserSlug : '',
 			showItemPaneToggle: false,
@@ -329,37 +337,39 @@ const Reader = () => {
 			onSetDataTransferAnnotations: noop, // n/a in web library, noop prevents errors printed on console from reader
 			// onDeletePages: handleDeletePages
 		});
-	}, [annotations, attachmentItem, attachmentKey, currentUserSlug, dispatch, getProcessedAnnotations, handleChangeViewState, handleResizeSidebar, handleToggleSidebar, isGroup, isReadOnly, isReaderSidebarOpen, locationValue, readerSidebarWidth, state.data, state.importedAnnotations])
+	}, [annotations, attachmentItem, attachmentKey, currentUserSlug, dispatch, getProcessedAnnotations, handleChangeViewState, handleResizeSidebar, handleToggleSidebar, isGroup, isReadOnly, isReaderSidebarOpen, location, locationValue, readerSidebarWidth, state.data, state.importedAnnotations])
 
-	// On first render, fetch attachment item details or redirect if invalid URL
+	useEffect(() => {
+		// pdf js stores last page in localStorage but we want to use one from user library settings instead
+		localStorage.removeItem('pdfjs.history');
+	}, []);
+
+	// fetch attachment item details or redirect if invalid URL
 	useEffect(() => {
 		if(!attachmentKey) {
 			dispatch(navigate({ items: null, attachmentKey: null, noteKey: null, view: 'item-list' }));
 		}
 		if (attachmentKey && !attachmentItem) {
 			dispatch(fetchItemDetails(attachmentKey));
 		}
-		// pdf js stores last page in localStorage but we want to use one from user library settings instead
-		localStorage.removeItem('pdfjs.history');
-		// we also need user library settings for last page read syncing
-		dispatch(fetchLibrarySettings(userLibraryKey, pageIndexSettingKey));
-	}, []);// eslint-disable-line react-hooks/exhaustive-deps
+
+	}, [attachmentItem, attachmentKey, dispatch]);
 
 	// Fetch all child items (annotations). This effect will execute multiple times for each page of annotations
 	useEffect(() => {
-		if (!isFetching && !isFetched) {
+		if (state.isRouteConfirmed && !isFetching && !isFetched) {
 			const start = pointer || 0;
 			const limit = PAGE_SIZE;
 			dispatch(fetchChildItems(attachmentKey, { start, limit }));
 		}
-	}, [dispatch, attachmentKey, isFetching, isFetched, pointer]);
+	}, [attachmentKey, dispatch, isFetched, isFetching, pointer, state.isRouteConfirmed]);
 
 	// Fetch attachment URL
 	useEffect(() => {
-		if (!urlIsFresh && !isFetchingUrl) {
+		if (state.isRouteConfirmed && !urlIsFresh && !isFetchingUrl) {
 			dispatch(tryGetAttachmentURL(attachmentKey));
 		}
-	}, [attachmentKey, attachmentItem, dispatch, isFetchingUrl, prevAttachmentItem, urlIsFresh]);
+	}, [attachmentKey, attachmentItem, dispatch, isFetchingUrl, prevAttachmentItem, urlIsFresh, state.isRouteConfirmed]);
 
 	// Fetch attachment binary data
 	useEffect(() => {
@@ -400,18 +410,34 @@ const Reader = () => {
 	}, [attachmentItem, pdfWorker, state.annotationsState, state.data, state.dataState]);
 
 	useEffect(() => {
-		if (!state.isReady && isFetched && state.data && state.annotationsState == IMPORTED && !isFetchingUserLibrarySettings) {
+		if (!state.isReady && isFetched && state.data && state.annotationsState == IMPORTED && state.isSettingsFetched && !isFetchingUserLibrarySettings) {
 			dispatchState({ type: 'READY' });
 		}
-	}, [isFetched, isFetchingUserLibrarySettings, state.annotationsState, state.data, state.isReady]);
+	}, [isFetched, isFetchingUserLibrarySettings, state.annotationsState, state.data, state.isReady, state.isSettingsFetched]);
+
+	useEffect(() => {
+		if (state.isRouteConfirmed && !state.isSettingsFetched && !isFetchingUserLibrarySettings) {
+			dispatch(fetchLibrarySettings(userLibraryKey, pageIndexSettingKey));
+			dispatchState({ type: 'COMPLETE_FETCH_SETTINGS' });
+		}
+	}, [dispatch, isFetchingUserLibrarySettings, pageIndexSettingKey, state.isRouteConfirmed, state.isSettingsFetched, userLibraryKey]);
 
 	useEffect(() => {
-		if (attachmentItem && !prevAttachmentItem
-			&& (attachmentItem.itemType !== 'attachment' || !Object.keys(READER_CONTENT_TYPES).includes(attachmentItem.contentType))
-		) {
-			dispatch(navigate({ view: 'item-details' }));
+		const isCompatible = (attachmentItem?.itemType === 'attachment' && Object.keys(READER_CONTENT_TYPES).includes(attachmentItem?.contentType));
+		if (attachmentItem && !prevAttachmentItem && !isCompatible) {
+			// item the URL points to is not an attachment or is of an unsupported type. We can try to navigate to the best attachment
+			const bestAttachment = attachmentItem?.[Symbol.for('links')]?.attachment;
+			const bestAttachmentKey = bestAttachment ? getItemFromApiUrl(bestAttachment.href) : null;
+			if (bestAttachmentKey) {
+				dispatch(navigate({ items: attachmentItem.key, attachmentKey: bestAttachmentKey }));
+			} else {
+				// best attachment not found, redirect to item details
+				dispatch(navigate({ view: 'item-details', location: null }));
+			}
+		} else if (!state.isRouteConfirmed && attachmentItem !== !prevAttachmentItem && isCompatible) {
+			dispatchState({ type: 'ROUTE_CONFIRMED' });
 		}
-	}, [dispatch, attachmentItem, prevAttachmentItem]);
+	}, [dispatch, attachmentItem, prevAttachmentItem, state.isRouteConfirmed]);
 
 	useEffect(() => {
 		if (lastFetchItemDetailsNoResults) {

src/js/component/rich-editor.jsx

@@ -5,7 +5,7 @@ import { useDispatch, useSelector } from 'react-redux';
 import { usePrevious } from 'web-common/hooks';
 import { noop } from 'web-common/utils';
 
-import { createAttachments, getAttachmentUrl, navigate } from '../actions';
+import { createAttachments, getAttachmentUrl, navigate, openInReader } from '../actions';
 import { getItemFromCanonicalUrl, parseBase64File } from '../utils';
 import { extensionLookup  } from '../common/mime';
 
@@ -109,10 +109,20 @@ const RichEditor = memo(forwardRef((props, ref) => {
 			case 'showCitationItem':
 				handleShowCitationItem(event.data?.message?.citation?.citationItems);
 				break;
+			case 'openCitationPage': {
+				const { uris, locator } = event.data?.message?.citation?.citationItems?.[0] || {};
+				if(uris?.length === 1) {
+					const { libraryKey, itemKey } = getItemFromCanonicalUrl(uris[0]) ?? {};
+					if(libraryKey && itemKey) {
+						dispatch(openInReader({ items: itemKey, library: libraryKey, location: { pageNumber: locator } }));
+					}
+				}
+				break;
+			}
 			default:
 				break;
 		}
-	}, [isReadOnly, handleSubscription, handleImportImages, handleShowCitationItem, onEdit, changeIfDifferent, focusEditor]);
+	}, [isReadOnly, handleSubscription, handleImportImages, handleShowCitationItem, onEdit, changeIfDifferent, focusEditor, dispatch]);
 
 	// handles iframe loaded once, installed on mount, never updated, doesn't need useCallback deps
 	const handleIframeLoaded = useCallback(() => {

src/js/reducers/current.js

@@ -36,6 +36,7 @@ const stateDefault = {
 	userLibraryKey: null,
 	useTransitions: false,
 	view: 'library',
+	location: null,
 };
 
 const getLibraryKey = (params, config) => {
@@ -84,6 +85,9 @@ const current = (state = stateDefault, action, { config = {}, device = {} } = {}
 			var itemsSource;
 			var searchState = state.searchState;
 			var itemKey = itemKeys && itemKeys.length === 1 ? itemKeys[0] : null
+			var location = params.locationType && params.locationValue ? {
+				[params.locationType]: params.locationValue
+			} : null;
 
 			if(tags.length || search.length) {
 				itemsSource = 'query';
@@ -97,6 +101,7 @@ const current = (state = stateDefault, action, { config = {}, device = {} } = {}
 				itemsSource = 'top';
 			}
 
+
 			if(!view) {
 				//@TODO: Refactor
 				view = itemKeys.length ?
@@ -148,6 +153,7 @@ const current = (state = stateDefault, action, { config = {}, device = {} } = {}
 				tags: shallowEqual(tags, state.tags) ? state.tags : tags,
 				useTransitions: state.useTransitions,
 				view,
+				location
 			}
 		case TRIGGER_EDITING_ITEM:
 			return {

src/js/routes.js

@@ -7,20 +7,20 @@ const multipleIdRe = '[A-Z0-9,]+';
 const getVariants = prefix => {
 	return [
 		`${prefix}/tags/:tags/search/:search/:qmode/items/:items(${multipleIdRe})/note/:note/:view(library|collection|item-list|item-details)?`,
-		`${prefix}/tags/:tags/search/:search/:qmode/items/:items(${multipleIdRe})/attachment/:attachment(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?`,
-		`${prefix}/tags/:tags/search/:search/:qmode/items/:items(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?`,
+		`${prefix}/tags/:tags/search/:search/:qmode/items/:items(${multipleIdRe})/attachment/:attachment(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?/:locationType(pageNumber|annotationID|position|href)?/:locationValue?`,
+		`${prefix}/tags/:tags/search/:search/:qmode/items/:items(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?/:locationType(pageNumber|annotationID|position|href)?/:locationValue?`,
 		`${prefix}/tags/:tags/search/:search/:qmode/items/:items(${multipleIdRe})/:view(library|collection|item-list|item-details)?`,
 		`${prefix}/tags/:tags/items/:items(${multipleIdRe})/note/:note/:view(library|collection|item-list|item-details)?`,
-		`${prefix}/tags/:tags/items/:items(${multipleIdRe})/attachment/:attachment(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?`,
-		`${prefix}/tags/:tags/items/:items(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?`,
+		`${prefix}/tags/:tags/items/:items(${multipleIdRe})/attachment/:attachment(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?/:locationType(pageNumber|annotationID|position|href)?/:locationValue?`,
+		`${prefix}/tags/:tags/items/:items(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?/:locationType(pageNumber|annotationID|position|href)?/:locationValue?`,
 		`${prefix}/tags/:tags/items/:items(${multipleIdRe})/:view(library|collection|item-list|item-details)?`,
 		`${prefix}/search/:search/:qmode/items/:items(${multipleIdRe})/note/:note/:view(library|collection|item-list|item-details)?`,
-		`${prefix}/search/:search/:qmode/items/:items(${multipleIdRe})/attachment/:attachment(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?`,
-		`${prefix}/search/:search/:qmode/items/:items(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?`,
+		`${prefix}/search/:search/:qmode/items/:items(${multipleIdRe})/attachment/:attachment(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?/:locationType(pageNumber|annotationID|position|href)?/:locationValue?`,
+		`${prefix}/search/:search/:qmode/items/:items(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?/:locationType(pageNumber|annotationID|position|href)?/:locationValue?`,
 		`${prefix}/search/:search/:qmode/items/:items(${multipleIdRe})/:view(library|collection|item-list|item-details)?`,
 		`${prefix}/items/:items(${multipleIdRe})/note/:note/:view(library|collection|item-list|item-details)?`,
-		`${prefix}/items/:items(${multipleIdRe})/attachment/:attachment(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?`,
-		`${prefix}/items/:items(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?`,
+		`${prefix}/items/:items(${multipleIdRe})/attachment/:attachment(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?/:locationType(pageNumber|annotationID|position|href)?/:locationValue?`,
+		`${prefix}/items/:items(${singleIdRe})/:view(library|collection|item-list|item-details|reader)?/:locationType(pageNumber|annotationID|position|href)?/:locationValue?`,
 		`${prefix}/items/:items(${multipleIdRe})/:view(library|collection|item-list|item-details)?`,
 		`${prefix}/tags/:tags/search/:search/:qmode/:view(library|collection|item-list|item-details)?`,
 		`${prefix}/tags/:tags/:view(library|collection|item-list|item-details)?`,