Skip to content

Commit

Permalink
Improve file importing UX and fix edge cases #450
Browse files Browse the repository at this point in the history
  • Loading branch information
@tnajdek
tnajdek committed Dec 3, 2024
1 parent 4efe041 commit ed6d0cb
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 45 deletions.
7 changes: 5 additions & 2 deletions src/js/actions/identifier.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ const searchIdentifier = (identifier, { shouldImport = false } = {}) => {
if (response.status === 501 || response.status === 400) {
const message = 'Zotero could not find any identifiers in your input. Please verify your input and try again.';
dispatch({ type: RECEIVE_ADD_BY_IDENTIFIER, identifier, identifierIsUrl, result: EMPTY, message });
} else if (response.status === 413) {
const message = 'Selected file is too large.';
dispatch({ type: RECEIVE_ADD_BY_IDENTIFIER, identifier, identifierIsUrl, result: EMPTY, message });
} else if (response.status === 300) {
const data = await response.json();
const items = 'items' in data && 'session' in data ? data.items : data;
Expand Down Expand Up @@ -269,9 +272,9 @@ const searchIdentifierMore = () => {
}
}

const reportIdentifierNoResults = () => ({
const reportIdentifierNoResults = (message = 'Zotero could not find any identifiers in your input. Please verify your input and try again.') => ({
type: ERROR_IDENTIFIER_NO_RESULT,
error: 'Zotero could not find any identifiers in your input. Please verify your input and try again.',
error: message,
errorType: 'info',
});

Expand Down
5 changes: 3 additions & 2 deletions src/js/component/item/actions/add-by-identifier.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const AddByIdentifier = props => {
const result = useSelector(state => state.identifier.result);
const item = useSelector(state => state.identifier.item);
const items = useSelector(state => state.identifier.items);
const message = useSelector(state => state.identifier.message);
const prevItem = usePrevious(item);
const prevItems = usePrevious(items);
const wasSearching = usePrevious(isSearching);
Expand Down Expand Up @@ -113,13 +114,13 @@ const AddByIdentifier = props => {
if(!isSearching && wasSearching) {
setIdentifier('');
if(result === EMPTY) {
dispatch(reportIdentifierNoResults());
dispatch(reportIdentifierNoResults(message));
} else {
ref.current.focus();
setIsOpen(false);
}
}
}, [dispatch, isSearching, wasSearching, result]);
}, [dispatch, isSearching, wasSearching, result, message]);

useLayoutEffect(() => {
if (isOpen && !wasOpen) {
Expand Down
5 changes: 3 additions & 2 deletions src/js/component/modal/add-by-identifier.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const AddByIdentifierModal = () => {
const result = useSelector(state => state.identifier.result);
const item = useSelector(state => state.identifier.item);
const items = useSelector(state => state.identifier.items);
const message = useSelector(state => state.identifier.message);
const prevItem = usePrevious(item);
const prevItems = usePrevious(items);
const wasSearching = usePrevious(isSearching);
Expand Down Expand Up @@ -81,7 +82,7 @@ const AddByIdentifierModal = () => {
if(isOpen && wasSearching && !isSearching) {
setIdentifier('');
if(result === EMPTY) {
dispatch(reportIdentifierNoResults());
dispatch(reportIdentifierNoResults(message));
} else {
dispatch(toggleModal(ADD_BY_IDENTIFIER, false));
if(items && prevItems === null && [CHOICE, CHOICE_EXHAUSTED, MULTIPLE].includes(result)) {
Expand All @@ -90,7 +91,7 @@ const AddByIdentifierModal = () => {
}

}
}, [dispatch, isOpen, isSearching, items, prevItems, result, wasSearching]);
}, [dispatch, isOpen, isSearching, items, message, prevItems, result, wasSearching]);

return (
<Modal
Expand Down
112 changes: 77 additions & 35 deletions src/js/component/modal/identifier-picker.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import { usePrevious } from 'web-common/hooks';

import Modal from '../ui/modal';
import { IDENTIFIER_PICKER } from '../../constants/modals';
import { currentAddMultipleTranslatedItems, searchIdentifierMore, toggleModal } from '../../actions';
import { currentAddMultipleTranslatedItems, searchIdentifierMore, reportIdentifierNoResults, toggleModal } from '../../actions';
import { useBufferGate } from '../../hooks';
import { getUniqueId, processIdentifierMultipleItems } from '../../utils';
import { getBaseMappedValue } from '../../common/item';
import { CHOICE } from '../../constants/identifier-result-types';
import { CHOICE, EMPTY, MULTIPLE } from '../../constants/identifier-result-types';
import { pluralize } from '../../common/format';

const Item = memo(({ onChange, identifierIsUrl, isPicked, item, mappings }) => {
Expand Down Expand Up @@ -97,6 +97,7 @@ const IdentifierPicker = () => {
const isSearchingMultiple = useSelector(state => state.identifier.isSearchingMultiple);
const identifierIsUrl = useSelector(state => state.identifier.identifierIsUrl);
const identifierResult = useSelector(state => state.identifier.result);
const identifierMessage = useSelector(state => state.identifier.message);
const mappings = useSelector(state => state.meta.mappings);
const isSearching = useSelector(state => state.identifier.isSearching);
const isTouchOrSmall = useSelector(state => state.device.isTouchOrSmall);
Expand Down Expand Up @@ -127,7 +128,9 @@ const IdentifierPicker = () => {
}, [dispatch]);

const handleSelectAll = useCallback(() => {
setSelectedKeys(processedItems.map(i => i.key));
if (Array.isArray(processedItems)) {
setSelectedKeys(processedItems.map(i => i.key));
}
}, [processedItems]);

const handleClearSelection = useCallback(() => {
Expand All @@ -142,35 +145,72 @@ const IdentifierPicker = () => {

useEffect(() => {
if(!wasReady && isReady) {
handleSelectAll();
if (identifierResult === EMPTY) {
dispatch(toggleModal(IDENTIFIER_PICKER, false));
if (identifierMessage) {
dispatch(reportIdentifierNoResults(identifierMessage));
}
} else if(isImport || identifierResult === MULTIPLE) {
// Select all if either importing or add by identifier result is multiple (but not choice)
handleSelectAll();
}
}
}, [handleSelectAll, isReady, wasReady]);

const className = cx({
'identifier-picker-modal modal-scrollable modal-lg': true,
'modal-touch': isTouchOrSmall
});
}, [dispatch, handleSelectAll, identifierMessage, identifierResult, isImport, isReady, wasReady]);

return (
<Modal
className={ className }
className="identifier-picker-modal modal-scrollable modal-lg modal-touch"
contentLabel="Add By Identifier"
isOpen={ isOpen }
isBusy={ isBusy }
onRequestClose={ handleCancel }
overlayClassName="modal-slide modal-centered"
>
<div className="modal-header">
<h4 className="modal-title truncate">
Select Items
</h4>
<Button
icon
className="close"
onClick={handleCancel}
title="Close Dialog"
>
<Icon type={'16/close'} width="16" height="16" />
</Button>
{
isTouchOrSmall ? (
<>
<div className="modal-header-left">
<Button
className="btn-link"
onClick={handleCancel}
>
Cancel
</Button>
</div>
<div className="modal-header-center">
<h4 className="modal-title truncate">
Select Items
</h4>
</div>
<div className="modal-header-right">
<Button
disabled={selectedKeys.length === 0}
className="btn-link"
onClick={handleAddSelected}
>
{
selectedKeys.length > 0 ? `Add ${selectedKeys.length} ${pluralize('Item', selectedKeys.length)}` : 'Add Selected'
}
</Button>
</div>
</>
) : (
<>
<h4 className="modal-title truncate">
Select Items
</h4>
<Button
icon
className="close"
onClick={handleCancel}
title="Close Dialog"
>
<Icon type={'16/close'} width="16" height="16" />
</Button>
</>
)
}
</div>
<div className="modal-body">
<ul className="results">
Expand Down Expand Up @@ -199,26 +239,28 @@ const IdentifierPicker = () => {
<div className="modal-footer-center">
{ isSearching ? <Spinner /> : (
<Button
className="btn btn btn-lg btn-secondary more-button"
className={cx("btn more-button", { 'btn-link': isTouchOrSmall, 'btn-lg btn-secondary': !isTouchOrSmall }) }
onClick={ handleSearchMore }
>
More
</Button>
) }
</div>
) }
<div className="modal-footer-right">
<Button
disabled={selectedKeys.length === 0}
className="btn-outline-secondary btn-min-width"
onClick={ handleAddSelected }
tabIndex={-2}
>
{
selectedKeys.length > 0 ? `Add ${selectedKeys.length} ${pluralize('Item', selectedKeys.length)}` : 'Add Selected'
}
</Button>
</div>
{ !isTouchOrSmall && (
<div className="modal-footer-right">
<Button
disabled={selectedKeys.length === 0}
className="btn-outline-secondary btn-min-width"
onClick={ handleAddSelected }
tabIndex={-2}
>
{
selectedKeys.length > 0 ? `Add ${selectedKeys.length} ${pluralize('Item', selectedKeys.length)}` : 'Add Selected'
}
</Button>
</div>
) }
</div>
</Modal>
);
Expand Down
16 changes: 12 additions & 4 deletions src/js/component/ui/modal.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import cx from 'classnames';
import PropTypes from 'prop-types';
import { forwardRef, memo, useCallback, useEffect, useImperativeHandle, useRef } from 'react';
import ReactModal from 'react-modal';
import { createPortal} from 'react-dom';

import { useSelector } from 'react-redux';
import { usePrevious } from 'web-common/hooks';
import { getScrollbarWidth, pick } from 'web-common/utils';
Expand Down Expand Up @@ -51,20 +53,26 @@ const Modal = forwardRef((props, ref) => {
}
}, [isOpen, wasOpen]);

return (
return isBusy ? createPortal(
<div className="ReactModalPortal">
<div className={cx('ReactModal__Overlay modal-backdrop loading') }>
<Spinner className="large" />
</div>
</div>, document.querySelector(`.${containterClassName}`)
) : (
<ReactModal
role="dialog"
onAfterOpen={ handleModalAfterOpen }
contentRef= { setContentRef }
appElement= { document.querySelector('.library-container') }
parentSelector={ () => document.querySelector(`.${containterClassName}`) }
className= { cx('modal modal-content', className) }
overlayClassName={ cx({ 'loading': isBusy }, 'modal-backdrop', overlayClassName) }
className={cx('modal modal-content', className) }
overlayClassName={cx('modal-backdrop', overlayClassName) }
isOpen={ isOpen }
closeTimeoutMS={ isTouchOrSmall ? 200 : null }
{ ...pick(rest, ['contentLabel', 'onRequestClose', 'shouldFocusAfterRender', 'shouldCloseOnOverlayClick', 'shouldCloseOnEsc', 'shouldReturnFocusAfterClose']) }
>
{ isBusy ? <Spinner className="large" /> : children }
{ children }
</ReactModal>
);
});
Expand Down
2 changes: 2 additions & 0 deletions src/js/reducers/identifier.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const defaultState = {
identifierIsUrl: null,
import: false,
next: null,
message: null
}

const identifier = (state = defaultState, action) => {
Expand Down Expand Up @@ -48,6 +49,7 @@ const identifier = (state = defaultState, action) => {
items: action.items || null,
import: action.import,
next: action.next,
message: action.message,
};
case ERROR_ADD_BY_IDENTIFIER:
return {
Expand Down

0 comments on commit ed6d0cb

Please sign in to comment.