Skip to content

Commit

Permalink
Miscellaneous tweaks related to metadata storage and processing
Browse files Browse the repository at this point in the history
- Item type options are now stored, mapped, and sorted. Previously, this process was repeated in multiple components, wasting processing power and resulting in different orderings for entries in the "New Item" dropdown and the item type selector.
- Creator type options are also stored mapped to avoid repeated processing.
- Optimised the Creators component to avoid using `deepEqual`.
- Added tests to ensure the correct order of item type options.
- Fixed a missing a11y label for the New Item Type Picker dropdown.
  • Loading branch information
tnajdek committed Feb 1, 2025
1 parent 5a21daa commit c42c552
Show file tree
Hide file tree
Showing 27 changed files with 175 additions and 127 deletions.
7 changes: 0 additions & 7 deletions src/js/common/format.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,6 @@ const noteAsTitle = text => {
const creator = creator => 'lastName' in creator ?
[creator.lastName, creator.firstName].filter(s => s.trim().length).join(', ') : creator.name;

const itemTypeLocalized = (item, itemTypes) => {
const { itemType } = item;
if(itemType === 'note') { return "Note" }
if(itemType === 'attachment') { return "Attachment" }
return (itemTypes.find(it => it.itemType === itemType) || {}).localized || itemType;
}
const dateLocalized = date => (date instanceof Date && !isNaN(date)) ?
date.toLocaleString(navigator.language || navigator.userLanguage) : '';

Expand Down Expand Up @@ -211,7 +205,6 @@ export {
formatDate,
formatDateTime,
itemsSourceLabel,
itemTypeLocalized,
noteSummary,
noteAsTitle,
parseDescriptiveString,
Expand Down
5 changes: 2 additions & 3 deletions src/js/common/item.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { cleanDOI, cleanURL, get, containsEmoji, extractEmoji } from '../utils';
import { noteAsTitle, itemTypeLocalized, dateLocalized } from './format';
import { noteAsTitle, dateLocalized } from './format';
import { itemTypesWithIcons } from '../../../data/item-types-with-icons.json';


Expand Down Expand Up @@ -135,10 +135,9 @@ const getDerivedData = (mappings, item, itemTypes, tagColors) => {
const createdByUser = item[Symbol.for('meta')] && item[Symbol.for('meta')].createdByUser ?
item[Symbol.for('meta')].createdByUser.username :
'';
const itemTypeName = itemTypeLocalized(item, itemTypes);
const itemTypeName = (itemTypes.find(({ itemType }) => itemType === item.itemType) || { localized: item.itemType }).localized;
const iconName = (!item.itemType && item.name) ? 'folder' : item.itemType === 'attachment' ? getAttachmentIcon(item) : getItemTypeIcon(item.itemType);


// same logic as https://github.com/zotero/zotero/blob/6abfd3b5b03969564424dc03313d63ae1de86100/chrome/content/zotero/xpcom/itemTreeView.js#L1062
const year = date.substr(0, 4);

Expand Down
22 changes: 11 additions & 11 deletions src/js/component/form/creators.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ import { enumerateObjects, splice } from '../../utils';
import { useEditMode } from '../../hooks';

const Creators = props => {
const { creatorTypes, onSave, name, value = [], isForm, onDragStatusChange, isReadOnly } = props;

const { onSave, name, value = [], isForm, itemType, onDragStatusChange, isReadOnly } = props;
const creatorTypeOptions = useSelector(state => state.meta.itemTypeCreatorTypeOptions[itemType]);
const virtualCreators = useMemo(() => [{
id: 0,
creatorType: creatorTypes?.[0]?.value ?? '',
creatorType: creatorTypeOptions?.[0]?.value ?? '',
firstName: '',
lastName: '',
[Symbol.for('isVirtual')]: true
}], [creatorTypes]);
}], [creatorTypeOptions]);

const [creators, setCreators] = useState(
value.length ? enumerateObjects(value) : virtualCreators
Expand All @@ -34,7 +34,7 @@ const Creators = props => {
const animateAppearOnNextRender = useRef(null);
const prevValue = usePrevious(value);
const prevCreators = usePrevious(creators);
const prevCreatorTypes = usePrevious(creatorTypes);
const prevItemType = usePrevious(itemType);
const shouldUseEditMode = useSelector(state => state.device.shouldUseEditMode);
const [isEditing, ] = useEditMode();

Expand Down Expand Up @@ -150,14 +150,14 @@ const Creators = props => {
}, [value, virtualCreators, prevValue]);

useEffect(() => {
if(typeof(prevCreatorTypes) !== 'undefined' && !deepEqual(creatorTypes, prevCreatorTypes)) {
const validCreatorTypes = creatorTypes.map(ct => ct.value);
if (typeof (prevItemType) !== 'undefined' && prevItemType !== itemType) {
const validCreatorTypes = creatorTypeOptions.map(ct => ct.value);
setCreators(enumerateObjects(creators.map(creator => ({
...creator,
creatorType: validCreatorTypes.includes(creator.creatorType) ? creator.creatorType : validCreatorTypes[0]
}))));
}
}, [creators, creatorTypes, prevCreatorTypes]);
}, [creatorTypeOptions, creators, itemType, prevItemType]);

useEffect(() => {
if(creators && prevCreators && creators.length > prevCreators.length) {
Expand Down Expand Up @@ -187,7 +187,7 @@ const Creators = props => {
animateAppearOnNextRender.current = null;
}

if (!creatorTypes || !creatorTypes.length) {
if (!creatorTypeOptions || !creatorTypeOptions.length) {
return null;
}

Expand All @@ -210,7 +210,7 @@ const Creators = props => {
}) }
creator={ creator }
creatorsCount={ creators.length }
creatorTypes={ creatorTypes }
creatorTypes={ creatorTypeOptions }
index={ index }
isCreateAllowed={ !hasVirtual }
isDeleteAllowed={ !hasVirtual || creators.length > 1 }
Expand Down Expand Up @@ -250,9 +250,9 @@ const Creators = props => {
}

Creators.propTypes = {
creatorTypes: PropTypes.array.isRequired,
isForm: PropTypes.bool,
isReadOnly: PropTypes.bool,
itemType: PropTypes.string.isRequired,
name: PropTypes.string,
onDragStatusChange: PropTypes.func,
onSave: PropTypes.func,
Expand Down
18 changes: 3 additions & 15 deletions src/js/component/item-details/box.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,27 +46,15 @@ const ItemBox = ({ isReadOnly }) => {
const item = useSelector(state =>
get(state, ['libraries', state.current.libraryKey, 'items', state.current.itemKey])
);
const creatorTypes = useSelector(state=> state.meta.itemTypeCreatorTypes[item.itemType]);

const itemTypeFields = useSelector(state=> state.meta.itemTypeFields);
const itemTypes = useSelector(state => state.meta.itemTypes);
const itemTypeOptions = useSelector(state => state.meta.itemTypeOptions);
const mappings = useSelector(state => state.meta.mappings);
const pendingChanges = useSelector(state =>
get(state, ['libraries', state.current.libraryKey, 'updating', 'items', state.current.itemKey])
);

const isForm = !!(shouldUseEditMode && isEditing && item);

//@TODO: mapping should be handled by <Creators />
//@TODO: even better, we should store this mapped already
const creatorTypeOptions = useMemo(() => (creatorTypes || []).map(ct => ({
value: ct.creatorType,
label: ct.localized
})), [creatorTypes]);
const itemTypeOptions = useMemo(() => itemTypes.map(it => ({
value: it.itemType,
label: it.localized
})).filter(it => it.value !== 'note'), [itemTypes]);

const fields = useMemo(
() => makeFields(item, pendingChanges, itemTypeOptions, itemTypeFields, mappings, isReadOnly),
[item, pendingChanges, itemTypeOptions, itemTypeFields, mappings, isReadOnly]
Expand Down Expand Up @@ -138,7 +126,7 @@ const ItemBox = ({ isReadOnly }) => {
{ fields.map(field =>
field.key === 'creators' ? (
<Creators
creatorTypes = { creatorTypeOptions }
itemType = { item.itemType }
isForm = { isForm }
isReadOnly = { field.isReadOnly }
key = { field.key }
Expand Down
56 changes: 26 additions & 30 deletions src/js/component/item/actions/new-item.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,47 +4,44 @@ import { Dropdown, DropdownToggle, DropdownMenu, DropdownItem, Icon } from 'web-
import { useSelector } from 'react-redux';
import { noop } from 'web-common/utils';

import primaryItemTypes from '../../../constants/primary-item-types';
import { primaryItemTypes } from '../../../constants/item-types';
import { getPrevSibling } from '../../../utils';
import UploadAction from './upload';

const DropdownItemType = props => {
const { itemTypeSpec, onNewItemCreate } = props;
const { itemType, localized } = itemTypeSpec;
const { itemTypeOption, onNewItemCreate } = props;
const { value, label } = itemTypeOption;

const handleSelect = useCallback(() => {
onNewItemCreate(itemType);
}, [itemType, onNewItemCreate]);
onNewItemCreate(value);
}, [onNewItemCreate, value]);

return (
<DropdownItem onClick={ handleSelect }>
{ localized }
{ label }
</DropdownItem>
);
}

DropdownItemType.propTypes = {
itemTypeOption: PropTypes.object,
onNewItemCreate: PropTypes.func,
};

const NewItemSelector = props => {
const { disabled, onFocusNext = noop, onFocusPrev = noop, onNewItemCreate = noop, tabIndex } = props;
const [isOpen, setIsOpen] = useState(false);
const [isSecondaryVisible, setIsSecondaryVisible] = useState(false);
const itemTypes = useSelector(state => state.meta.itemTypes);
const itemTypeOptions = useSelector(state => state.meta.itemTypeOptions);
const isFileUploadAllowed = useSelector(state => (state.config.libraries.find(l => l.key === state.current.libraryKey) || {}).isFileUploadAllowed);

const primaryItemTypesDesc = useMemo(() => {
const primaryTypes = itemTypes.filter(
it => primaryItemTypes.includes(it.itemType)
);
primaryTypes.sort((a, b) => a.localized.localeCompare(b.localized));
return primaryTypes;

}, [itemTypes]);
const secondaryItemTypesDesc = useMemo(() => {
const secondaryTypes = itemTypes.filter(
it => it.itemType !== 'note' && !primaryItemTypes.includes(it.itemType)
);
secondaryTypes.sort((a, b) => a.localized.localeCompare(b.localized));
return secondaryTypes;
}, [itemTypes]);
const primaryItemTypesDesc = useMemo(
() => itemTypeOptions.filter(it => primaryItemTypes.includes(it.value)), [itemTypeOptions]
);

const secondaryItemTypesDesc = useMemo(
() => itemTypeOptions.filter(it => !primaryItemTypes.includes(it.value)), [itemTypeOptions]
);

const handleToggleDropdown = useCallback(() => {
if(disabled) {
Expand Down Expand Up @@ -93,12 +90,11 @@ const NewItemSelector = props => {
>
<Icon type={ '16/plus' } width="16" height="16" />
</DropdownToggle>
<DropdownMenu>

{ primaryItemTypesDesc.map(itemTypeSpec => (
<DropdownMenu aria-label="New Item Type Picker">
{ primaryItemTypesDesc.map(itemTypeOption => (
<DropdownItemType
itemTypeSpec={ itemTypeSpec }
key={ itemTypeSpec.itemType }
itemTypeOption={ itemTypeOption }
key={ itemTypeOption.value }
onNewItemCreate={ onNewItemCreate }
/>
)) }
Expand All @@ -110,10 +106,10 @@ const NewItemSelector = props => {
)}
<DropdownItem divider />
{ isSecondaryVisible ?
secondaryItemTypesDesc.map(itemTypeSpec => (
secondaryItemTypesDesc.map(itemTypeOption => (
<DropdownItemType
itemTypeSpec={ itemTypeSpec }
key={ itemTypeSpec.itemType }
itemTypeOption={ itemTypeOption }
key={ itemTypeOption.value }
onNewItemCreate={ onNewItemCreate }
/>
)) : (
Expand Down
39 changes: 15 additions & 24 deletions src/js/component/modal/new-item.jsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { memo, useCallback, useMemo, useRef, useState } from 'react';
import { memo, useCallback, useRef, useState } from 'react';
import { useDispatch, useSelector, shallowEqual } from 'react-redux';
import { Button } from 'web-common/components';

import Modal from '../ui/modal';
import Select from '../form/select';
import { get } from '../../utils';
import { getUniqueId } from '../../utils';
import { NEW_ITEM } from '../../constants/modals';
import { toggleModal, createItem, fetchItemTemplate, navigate, triggerEditingItem } from '../../actions';
Expand All @@ -17,7 +16,7 @@ const NewItemModal = () => {
const collection = useSelector(
state => state.libraries[state.current.libraryKey]?.dataObjects[state.modal.collectionKey], shallowEqual
);
const itemTypes = useSelector(state => state.meta.itemTypes, shallowEqual);
const itemTypeOptions = useSelector(state => state.meta.itemTypeOptions);
const inputId = useRef(getUniqueId());
const [isBusy, setIsBusy] = useState(false);
const [itemType, setItemType] = useState('book');
Expand All @@ -37,19 +36,11 @@ const NewItemModal = () => {
setIsBusy(false);
}, [collection, dispatch, itemsSource, itemType, libraryKey]);

const itemTypeOptions = useMemo(() => {
const options = itemTypes.map(({ itemType, localized }) => (
{ value: itemType, label: localized }
));
options.sort((a, b) => a.label.localeCompare(b.label));
return options;
}, [itemTypes]);

const handleChange = useCallback(() => true, []);
const handleCancel = useCallback(() => dispatch(toggleModal(NEW_ITEM, false)), [dispatch]);

const handleSelect = useCallback((newItemType, hasChanged) => {
if(hasChanged) {
if (hasChanged) {
setItemType(newItemType);
}
}, []);
Expand All @@ -58,16 +49,16 @@ const NewItemModal = () => {
<Modal
className="modal-touch"
contentLabel="Create a New Item"
isBusy={ isBusy }
isOpen={ isOpen }
onRequestClose={ handleCancel }
isBusy={isBusy}
isOpen={isOpen}
onRequestClose={handleCancel}
overlayClassName="modal-centered modal-slide"
>
<div className="modal-header">
<div className="modal-header-left">
<Button
className="btn-link"
onClick={ handleCancel }
onClick={handleCancel}
>
Cancel
</Button>
Expand All @@ -84,7 +75,7 @@ const NewItemModal = () => {
<div className="modal-header-right">
<Button
className="btn-link"
onClick={ handleNewItemCreate }
onClick={handleNewItemCreate}
>
Create
</Button>
Expand All @@ -93,17 +84,17 @@ const NewItemModal = () => {
<div className="modal-body">
<div className="form">
<div className="form-group">
<label htmlFor={ inputId.current }>
<label htmlFor={inputId.current}>
Item Type
</label>
<Select
id={ inputId.current }
id={inputId.current}
className="form-control form-control-sm"
onChange={ handleChange }
onCommit={ handleSelect }
options={ itemTypeOptions }
value={ itemType }
searchable={ true }
onChange={handleChange}
onCommit={handleSelect}
options={itemTypeOptions}
value={itemType}
searchable={true}
/>
</div>
</div>
Expand Down
15 changes: 15 additions & 0 deletions src/js/constants/item-types.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';

// when creating a new item, types not on this list are hidden under "More"
const primaryItemTypes = [
'book',
'bookSection',
'case',
'hearing',
'journalArticle',
];

// item types that cannot be created manually or changed to from another type
const ignoredItemTypes = ['note', 'attachment', 'annotation'];

export { primaryItemTypes, ignoredItemTypes };
9 changes: 0 additions & 9 deletions src/js/constants/primary-item-types.js

This file was deleted.

Loading

0 comments on commit c42c552

Please sign in to comment.