From 81532f2f20b9a75d87adf8a5df4789e38f43d5f5 Mon Sep 17 00:00:00 2001 From: Aman Harwara Date: Wed, 12 Oct 2022 21:57:51 +0530 Subject: [PATCH] refactor: item linking (#1781) --- packages/mobile/src/Hooks/useFiles.ts | 2 +- .../src/Screens/SideMenu/NoteSideMenu.tsx | 4 +- .../src/Domain/Item/ItemsClientInterface.ts | 4 +- .../lib/Services/Items/ItemManager.spec.ts | 34 +++- .../snjs/lib/Services/Items/ItemManager.ts | 19 +- .../ContentListView/NoteListItem.tsx | 2 +- .../src/javascripts/Components/Icon/Icon.tsx | 1 + .../LinkedItems/LinkedItemBubble.tsx | 42 ++-- .../LinkedItemBubblesContainer.tsx | 56 ++++-- .../LinkedItems/LinkedItemsPanel.tsx | 80 +++++--- .../Controllers/FilesController.ts | 2 +- .../Controllers/LinkingController.tsx | 180 +++++++++++++----- 12 files changed, 292 insertions(+), 134 deletions(-) diff --git a/packages/mobile/src/Hooks/useFiles.ts b/packages/mobile/src/Hooks/useFiles.ts index f57416833..004ca2b07 100644 --- a/packages/mobile/src/Hooks/useFiles.ts +++ b/packages/mobile/src/Hooks/useFiles.ts @@ -68,7 +68,7 @@ export const useFiles = ({ note }: Props) => { const filesService = application.getFilesService() const reloadAttachedFiles = useCallback(() => { - setAttachedFiles(application.items.getSortedFilesForItem(note).sort(filesService.sortByName)) + setAttachedFiles(application.items.getSortedFilesLinkingToItem(note).sort(filesService.sortByName)) }, [application.items, filesService.sortByName, note]) const reloadAllFiles = useCallback(() => { diff --git a/packages/mobile/src/Screens/SideMenu/NoteSideMenu.tsx b/packages/mobile/src/Screens/SideMenu/NoteSideMenu.tsx index 55224c3e3..dd2ec975e 100644 --- a/packages/mobile/src/Screens/SideMenu/NoteSideMenu.tsx +++ b/packages/mobile/src/Screens/SideMenu/NoteSideMenu.tsx @@ -139,7 +139,7 @@ export const NoteSideMenu = React.memo((props: Props) => { setAttachedFilesLength(0) return } - setAttachedFilesLength(application.items.getSortedFilesForItem(note).length) + setAttachedFilesLength(application.items.getSortedFilesLinkingToItem(note).length) }, [application, note]) useEffect(() => { @@ -147,7 +147,7 @@ export const NoteSideMenu = React.memo((props: Props) => { return } const removeFilesObserver = application.streamItems(ContentType.File, () => { - setAttachedFilesLength(application.items.getSortedFilesForItem(note).length) + setAttachedFilesLength(application.items.getSortedFilesLinkingToItem(note).length) }) return () => { removeFilesObserver() diff --git a/packages/services/src/Domain/Item/ItemsClientInterface.ts b/packages/services/src/Domain/Item/ItemsClientInterface.ts index 141b50e0b..7e3d22f96 100644 --- a/packages/services/src/Domain/Item/ItemsClientInterface.ts +++ b/packages/services/src/Domain/Item/ItemsClientInterface.ts @@ -114,7 +114,9 @@ export interface ItemsClientInterface { * @returns Array containing tags associated with an item */ getSortedTagsForItem(item: DecryptedItemInterface): SNTag[] - getSortedFilesForItem(item: DecryptedItemInterface): FileItem[] + + getSortedLinkedFilesForItem(item: DecryptedItemInterface): FileItem[] + getSortedFilesLinkingToItem(item: DecryptedItemInterface): FileItem[] getSortedLinkedNotesForItem(item: DecryptedItemInterface): SNNote[] getSortedNotesLinkingToItem(item: DecryptedItemInterface): SNNote[] diff --git a/packages/snjs/lib/Services/Items/ItemManager.spec.ts b/packages/snjs/lib/Services/Items/ItemManager.spec.ts index 2b2414d42..b4a413a61 100644 --- a/packages/snjs/lib/Services/Items/ItemManager.spec.ts +++ b/packages/snjs/lib/Services/Items/ItemManager.spec.ts @@ -805,7 +805,7 @@ describe('itemManager', () => { await itemManager.associateFileWithNote(file, note) - const filesAssociatedWithNote = itemManager.getSortedFilesForItem(note) + const filesAssociatedWithNote = itemManager.getSortedFilesLinkingToItem(note) expect(filesAssociatedWithNote).toHaveLength(1) expect(filesAssociatedWithNote[0].uuid).toBe(file.uuid) @@ -873,20 +873,38 @@ describe('itemManager', () => { it('should get all linked files for item', async () => { itemManager = createService() - const note = createNote('note') const file = createFile('A1') const file2 = createFile('B2') + const file3 = createFile('C3') - await itemManager.insertItems([note, file, file2]) + await itemManager.insertItems([file, file2, file3]) - await itemManager.associateFileWithNote(file2, note) - await itemManager.associateFileWithNote(file, note) + await itemManager.linkFileToFile(file, file3) + await itemManager.linkFileToFile(file, file2) - const sortedFilesForItem = itemManager.getSortedFilesForItem(note) + const sortedFilesForItem = itemManager.getSortedLinkedFilesForItem(file) expect(sortedFilesForItem).toHaveLength(2) - expect(sortedFilesForItem[0].uuid).toEqual(file.uuid) - expect(sortedFilesForItem[1].uuid).toEqual(file2.uuid) + expect(sortedFilesForItem[0].uuid).toEqual(file2.uuid) + expect(sortedFilesForItem[1].uuid).toEqual(file3.uuid) + }) + + it('should get all files linking to item', async () => { + itemManager = createService() + const baseFile = createFile('file') + const fileToLink1 = createFile('A1') + const fileToLink2 = createFile('B2') + + await itemManager.insertItems([baseFile, fileToLink1, fileToLink2]) + + await itemManager.linkFileToFile(fileToLink2, baseFile) + await itemManager.linkFileToFile(fileToLink1, baseFile) + + const sortedFilesForItem = itemManager.getSortedFilesLinkingToItem(baseFile) + + expect(sortedFilesForItem).toHaveLength(2) + expect(sortedFilesForItem[0].uuid).toEqual(fileToLink1.uuid) + expect(sortedFilesForItem[1].uuid).toEqual(fileToLink2.uuid) }) it('should get all linked notes for item', async () => { diff --git a/packages/snjs/lib/Services/Items/ItemManager.ts b/packages/snjs/lib/Services/Items/ItemManager.ts index 495e90b64..9fe97c05d 100644 --- a/packages/snjs/lib/Services/Items/ItemManager.ts +++ b/packages/snjs/lib/Services/Items/ItemManager.ts @@ -1192,7 +1192,19 @@ export class ItemManager ) } - public getSortedFilesForItem(item: DecryptedItemInterface): Models.FileItem[] { + public getSortedLinkedFilesForItem(item: DecryptedItemInterface): Models.FileItem[] { + if (this.isTemplateItem(item)) { + return [] + } + + const filesReferencedByItem = this.referencesForItem(item).filter( + (ref) => ref.content_type === ContentType.File, + ) as Models.FileItem[] + + return naturalSort(filesReferencedByItem, 'title') + } + + public getSortedFilesLinkingToItem(item: DecryptedItemInterface): Models.FileItem[] { if (this.isTemplateItem(item)) { return [] } @@ -1200,11 +1212,8 @@ export class ItemManager const filesReferencingItem = this.itemsReferencingItem(item).filter( (ref) => ref.content_type === ContentType.File, ) as Models.FileItem[] - const filesReferencedByItem = this.referencesForItem(item).filter( - (ref) => ref.content_type === ContentType.File, - ) as Models.FileItem[] - return naturalSort(filesReferencingItem.concat(filesReferencedByItem), 'title') + return naturalSort(filesReferencingItem, 'title') } public getSortedLinkedNotesForItem(item: DecryptedItemInterface): Models.SNNote[] { diff --git a/packages/web/src/javascripts/Components/ContentListView/NoteListItem.tsx b/packages/web/src/javascripts/Components/ContentListView/NoteListItem.tsx index e4fa38211..a289bd1b5 100644 --- a/packages/web/src/javascripts/Components/ContentListView/NoteListItem.tsx +++ b/packages/web/src/javascripts/Components/ContentListView/NoteListItem.tsx @@ -32,7 +32,7 @@ const NoteListItem: FunctionComponent = ({ const editorForNote = application.componentManager.editorForNote(item as SNNote) const editorName = editorForNote?.name ?? PLAIN_EDITOR_NAME const [icon, tint] = application.iconsController.getIconAndTintForNoteType(editorForNote?.package_info.note_type) - const hasFiles = application.items.getSortedFilesForItem(item).length > 0 + const hasFiles = application.items.getSortedFilesLinkingToItem(item).length > 0 const openNoteContextMenu = (posX: number, posY: number) => { notesController.setContextMenuOpen(false) diff --git a/packages/web/src/javascripts/Components/Icon/Icon.tsx b/packages/web/src/javascripts/Components/Icon/Icon.tsx index f602e0943..48bc7e557 100644 --- a/packages/web/src/javascripts/Components/Icon/Icon.tsx +++ b/packages/web/src/javascripts/Components/Icon/Icon.tsx @@ -5,6 +5,7 @@ import * as icons from '@standardnotes/icons' export const ICONS = { 'account-circle': icons.AccountCircleIcon, 'arrow-left': icons.ArrowLeftIcon, + 'arrow-right': icons.ArrowRightIcon, 'arrows-sort-down': icons.ArrowsSortDownIcon, 'arrows-sort-up': icons.ArrowsSortUpIcon, 'attachment-file': icons.AttachmentFileIcon, diff --git a/packages/web/src/javascripts/Components/LinkedItems/LinkedItemBubble.tsx b/packages/web/src/javascripts/Components/LinkedItems/LinkedItemBubble.tsx index 0613d2922..34671f94c 100644 --- a/packages/web/src/javascripts/Components/LinkedItems/LinkedItemBubble.tsx +++ b/packages/web/src/javascripts/Components/LinkedItems/LinkedItemBubble.tsx @@ -1,24 +1,26 @@ -import { LinkableItem, LinkingController } from '@/Controllers/LinkingController' +import { ItemLink, LinkableItem, LinkingController } from '@/Controllers/LinkingController' import { classNames } from '@/Utils/ConcatenateClassNames' import { KeyboardKey } from '@standardnotes/ui-services' import { observer } from 'mobx-react-lite' import { KeyboardEventHandler, MouseEventHandler, useEffect, useRef, useState } from 'react' +import { ContentType } from '@standardnotes/snjs' import Icon from '../Icon/Icon' type Props = { - item: LinkableItem + link: ItemLink getItemIcon: LinkingController['getLinkedItemIcon'] getTitleForLinkedTag: LinkingController['getTitleForLinkedTag'] activateItem: (item: LinkableItem) => Promise - unlinkItem: (item: LinkableItem) => void + unlinkItem: LinkingController['unlinkItemFromSelectedItem'] focusPreviousItem: () => void focusNextItem: () => void focusedId: string | undefined setFocusedId: (id: string) => void + isBidirectional: boolean } const LinkedItemBubble = ({ - item, + link, getItemIcon, getTitleForLinkedTag, activateItem, @@ -27,6 +29,7 @@ const LinkedItemBubble = ({ focusNextItem, focusedId, setFocusedId, + isBidirectional, }: Props) => { const ref = useRef(null) @@ -36,8 +39,8 @@ const LinkedItemBubble = ({ const [wasClicked, setWasClicked] = useState(false) const handleFocus = () => { - if (focusedId !== item.uuid) { - setFocusedId(item.uuid) + if (focusedId !== link.id) { + setFocusedId(link.id) } setShowUnlinkButton(true) } @@ -50,7 +53,7 @@ const LinkedItemBubble = ({ const onClick: MouseEventHandler = (event) => { if (wasClicked && event.target !== unlinkButtonRef.current) { setWasClicked(false) - void activateItem(item) + void activateItem(link.item) } else { setWasClicked(true) } @@ -58,14 +61,14 @@ const LinkedItemBubble = ({ const onUnlinkClick: MouseEventHandler = (event) => { event.stopPropagation() - unlinkItem(item) + unlinkItem(link) } const onKeyDown: KeyboardEventHandler = (event) => { switch (event.key) { case KeyboardKey.Backspace: { focusPreviousItem() - unlinkItem(item) + unlinkItem(link) break } case KeyboardKey.Left: @@ -77,29 +80,34 @@ const LinkedItemBubble = ({ } } - const [icon, iconClassName] = getItemIcon(item) - const tagTitle = getTitleForLinkedTag(item) + const [icon, iconClassName] = getItemIcon(link.item) + const tagTitle = getTitleForLinkedTag(link.item) useEffect(() => { - if (item.uuid === focusedId) { + if (link.id === focusedId) { ref.current?.focus() } - }, [focusedId, item.uuid]) + }, [focusedId, link.id]) return (