From 4c04bcffa8b5f792cd1cd4c33b2b83280b2f96b9 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Tue, 31 Jan 2023 15:42:58 +0800 Subject: [PATCH 01/14] Rename HighlightsModal to NotebookModal --- .../templates/article/HighlightsLayer.tsx | 14 ++++++++------ .../{HighlightsModal.tsx => NotebookModal.tsx} | 9 +++++---- .../templates/article/PdfArticleContainer.tsx | 14 ++++++++------ 3 files changed, 21 insertions(+), 16 deletions(-) rename packages/web/components/templates/article/{HighlightsModal.tsx => NotebookModal.tsx} (97%) diff --git a/packages/web/components/templates/article/HighlightsLayer.tsx b/packages/web/components/templates/article/HighlightsLayer.tsx index 38acad310..c29fec273 100644 --- a/packages/web/components/templates/article/HighlightsLayer.tsx +++ b/packages/web/components/templates/article/HighlightsLayer.tsx @@ -19,7 +19,7 @@ import { removeHighlights } from '../../../lib/highlights/deleteHighlight' import { createHighlight } from '../../../lib/highlights/createHighlight' import { HighlightNoteModal } from './HighlightNoteModal' import { ShareHighlightModal } from './ShareHighlightModal' -import { HighlightsModal } from './HighlightsModal' +import { NotebookModal } from './NotebookModal' import { useCanShareNative } from '../../../lib/hooks/useCanShareNative' import { showErrorToast } from '../../../lib/toastHelpers' import { ArticleMutations } from '../../../lib/articleActions' @@ -69,16 +69,18 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element { >([]) const focusedHighlightMousePos = useRef({ pageX: 0, pageY: 0 }) - const [focusedHighlight, setFocusedHighlight] = - useState(undefined) + const [focusedHighlight, setFocusedHighlight] = useState< + Highlight | undefined + >(undefined) const [selectionData, setSelectionData] = useSelection( highlightLocations, false //noteModal.open, ) - const [labelsTarget, setLabelsTarget] = - useState(undefined) + const [labelsTarget, setLabelsTarget] = useState( + undefined + ) const canShareNative = useCanShareNative() @@ -641,7 +643,7 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element { if (props.showHighlightsModal) { return ( - props.setShowHighlightsModal(false)} deleteHighlightAction={(highlightId: string) => { diff --git a/packages/web/components/templates/article/HighlightsModal.tsx b/packages/web/components/templates/article/NotebookModal.tsx similarity index 97% rename from packages/web/components/templates/article/HighlightsModal.tsx rename to packages/web/components/templates/article/NotebookModal.tsx index e507652ba..4215435c4 100644 --- a/packages/web/components/templates/article/HighlightsModal.tsx +++ b/packages/web/components/templates/article/NotebookModal.tsx @@ -28,7 +28,7 @@ import { setLabelsForHighlight } from '../../../lib/networking/mutations/setLabe import { updateHighlightMutation } from '../../../lib/networking/mutations/updateHighlightMutation' import { showErrorToast, showSuccessToast } from '../../../lib/toastHelpers' -type HighlightsModalProps = { +type NotebookModalProps = { highlights: Highlight[] scrollToHighlight?: (arg: string) => void updateHighlight: (highlight: Highlight) => void @@ -36,11 +36,12 @@ type HighlightsModalProps = { onOpenChange: (open: boolean) => void } -export function HighlightsModal(props: HighlightsModalProps): JSX.Element { +export function NotebookModal(props: NotebookModalProps): JSX.Element { const [showConfirmDeleteHighlightId, setShowConfirmDeleteHighlightId] = useState(undefined) - const [labelsTarget, setLabelsTarget] = - useState(undefined) + const [labelsTarget, setLabelsTarget] = useState( + undefined + ) const [, updateState] = useState({}) return ( diff --git a/packages/web/components/templates/article/PdfArticleContainer.tsx b/packages/web/components/templates/article/PdfArticleContainer.tsx index a222fcdae..e2bffea2a 100644 --- a/packages/web/components/templates/article/PdfArticleContainer.tsx +++ b/packages/web/components/templates/article/PdfArticleContainer.tsx @@ -15,7 +15,7 @@ import { ShareHighlightModal } from './ShareHighlightModal' import { useCanShareNative } from '../../../lib/hooks/useCanShareNative' import { webBaseURL } from '../../../lib/appConfig' import { pspdfKitKey } from '../../../lib/appConfig' -import { HighlightsModal } from './HighlightsModal' +import { NotebookModal } from './NotebookModal' import { HighlightNoteModal } from './HighlightNoteModal' export type PdfArticleContainerProps = { @@ -29,11 +29,13 @@ export default function PdfArticleContainer( props: PdfArticleContainerProps ): JSX.Element { const containerRef = useRef(null) - const [shareTarget, setShareTarget] = - useState(undefined) + const [shareTarget, setShareTarget] = useState( + undefined + ) const [noteTarget, setNoteTarget] = useState(undefined) - const [noteTargetPageIndex, setNoteTargetPageIndex] = - useState(undefined) + const [noteTargetPageIndex, setNoteTargetPageIndex] = useState< + number | undefined + >(undefined) const highlightsRef = useRef([]) const canShareNative = useCanShareNative() @@ -423,7 +425,7 @@ export default function PdfArticleContainer( /> )} {props.showHighlightsModal && ( - props.setShowHighlightsModal(false)} /* eslint-disable @typescript-eslint/no-empty-function */ From fe9ed7aa13dd0f2181edc76c47361260afe7e846 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 1 Feb 2023 16:42:36 +0800 Subject: [PATCH 02/14] Track the highlight position percent and anchor index when saving on web --- .../Views/WebReader/WebReaderContent.swift | 1 + .../Views/Article/OmnivoreWebView.swift | 21 +++--- .../templates/article/HighlightsLayer.tsx | 73 ++++++++++++++----- .../web/lib/highlights/createHighlight.ts | 14 +++- packages/web/lib/highlights/useSelection.tsx | 10 +-- .../networking/fragments/highlightFragment.ts | 4 + 6 files changed, 85 insertions(+), 38 deletions(-) diff --git a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContent.swift b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContent.swift index ab2cea59d..0df5750b3 100644 --- a/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContent.swift +++ b/apple/OmnivoreKit/Sources/App/Views/WebReader/WebReaderContent.swift @@ -88,6 +88,7 @@ struct WebReaderContent { window.localStorage.setItem("theme", "\(themeKey)") window.prefersHighContrastFont = \(prefersHighContrastText) window.enableHighlightBar = \(isMacApp) + window.enableSelectToHighlight = true diff --git a/apple/OmnivoreKit/Sources/Views/Article/OmnivoreWebView.swift b/apple/OmnivoreKit/Sources/Views/Article/OmnivoreWebView.swift index 7405e4777..a76c02c4b 100644 --- a/apple/OmnivoreKit/Sources/Views/Article/OmnivoreWebView.swift +++ b/apple/OmnivoreKit/Sources/Views/Article/OmnivoreWebView.swift @@ -196,15 +196,17 @@ public final class OmnivoreWebView: WKWebView { private func setDefaultMenu() { currentMenu = .defaultMenu - if #available(iOS 16.0, *) { - // on iOS16 we use menuBuilder to create these items - } else { - let annotate = UIMenuItem(title: "Annotate", action: #selector(annotateSelection)) - let highlight = UIMenuItem(title: LocalText.genericHighlight, action: #selector(highlightSelection)) - // let share = UIMenuItem(title: "Share", action: #selector(shareSelection)) + setHighlightMenu() - UIMenuController.shared.menuItems = [highlight, /* share, */ annotate] - } +// if #available(iOS 16.0, *) { +// // on iOS16 we use menuBuilder to create these items +// } else { +// let annotate = UIMenuItem(title: "Annotate", action: #selector(annotateSelection)) +// let highlight = UIMenuItem(title: LocalText.genericHighlight, action: #selector(highlightSelection)) +// // let share = UIMenuItem(title: "Share", action: #selector(shareSelection)) +// +// UIMenuController.shared.menuItems = [highlight, /* share, */ annotate] +// } } private func setHighlightMenu() { @@ -230,7 +232,8 @@ public final class OmnivoreWebView: WKWebView { } @objc func menuDidHide() { - setDefaultMenu() + // setDefaultMenu() + setHighlightMenu() } // swiftlint:disable:next line_length diff --git a/packages/web/components/templates/article/HighlightsLayer.tsx b/packages/web/components/templates/article/HighlightsLayer.tsx index c29fec273..4dda397db 100644 --- a/packages/web/components/templates/article/HighlightsLayer.tsx +++ b/packages/web/components/templates/article/HighlightsLayer.tsx @@ -73,10 +73,7 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element { Highlight | undefined >(undefined) - const [selectionData, setSelectionData] = useSelection( - highlightLocations, - false //noteModal.open, - ) + const [selectionData, setSelectionData] = useSelection(highlightLocations) const [labelsTarget, setLabelsTarget] = useState( undefined @@ -192,6 +189,41 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element { [props.highlightBarDisabled] ) + const selectionPercentPos = (selection: Selection): number | undefined => { + if ( + selection.rangeCount > 0 && + window && + window.document.scrollingElement + ) { + const percent = + (selection.getRangeAt(0).getBoundingClientRect().y + window.scrollY) / + window.document.scrollingElement.scrollHeight + return Math.min(Math.max(0, percent * 100), 100) + } + return undefined + } + + const selectionAnchorIndex = (selection: Selection): number | undefined => { + if (selection.rangeCount > 0) { + const containerElement = () => { + const node = selection.getRangeAt(0).startContainer + if (node.nodeType == Node.ELEMENT_NODE) { + return node as HTMLElement + } + return node.parentElement + } + let walk = containerElement() + while (walk) { + const idx = Number(walk.getAttribute('data-omnivore-anchor-idx')) + if (idx > 0) { + return idx + } + walk = walk.parentElement + } + } + return undefined + } + const createHighlightFromSelection = async ( selection: SelectionAttributes, note?: string @@ -203,6 +235,8 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element { existingHighlights: highlights, highlightStartEndOffsets: highlightLocations, annotation: note, + highlightPositionPercent: selectionPercentPos(selection.selection), + highlightPositionAnchorIndex: selectionAnchorIndex(selection.selection), }, props.articleMutations ) @@ -258,20 +292,6 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element { ] ) - const scrollToHighlight = (id: string) => { - const foundElement = document.querySelector( - `[omnivore-highlight-id="${id}"]` - ) - if (foundElement) { - foundElement.scrollIntoView({ - block: 'center', - behavior: 'smooth', - }) - window.location.hash = `#${id}` - props.setShowHighlightsModal(false) - } - } - // Detect mouseclick on a highlight -- call `setFocusedHighlight` when highlight detected const handleClickHighlight = useCallback( (event: MouseEvent) => { @@ -424,6 +444,23 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element { ] ) + useEffect(() => { + ;(async () => { + if ( + 'enableSelectToHighlight' in window && + window.enableSelectToHighlight && + selectionData + ) { + // console.log('auto highlight: ', selectionData.selection.toString()) + await createHighlightCallback('none', undefined) + selectionData.selection.collapseToStart() + // handleClickHighlight(selectionData.mouseEvent) + // setSelectionData(null) + // window.getSelection()?.removeAllRanges() + } + })() + }, [selectionData, setSelectionData]) + const dispatchHighlightError = (action: string, error: unknown) => { if (props.isAppleAppEmbed) { window?.webkit?.messageHandlers.highlightAction?.postMessage({ diff --git a/packages/web/lib/highlights/createHighlight.ts b/packages/web/lib/highlights/createHighlight.ts index 7262e3252..045f28819 100644 --- a/packages/web/lib/highlights/createHighlight.ts +++ b/packages/web/lib/highlights/createHighlight.ts @@ -18,6 +18,8 @@ type CreateHighlightInput = { annotation?: string existingHighlights: Highlight[] highlightStartEndOffsets: HighlightLocation[] + highlightPositionPercent?: number + highlightPositionAnchorIndex?: number } type CreateHighlightOutput = { @@ -30,7 +32,6 @@ export async function createHighlight( input: CreateHighlightInput, articleMutations: ArticleMutations ): Promise { - if (!input.selection.selection) { return {} } @@ -65,7 +66,10 @@ export async function createHighlight( annotations.push(annotation) } }) - removeHighlights(input.selection.overlapHighlights, input.highlightStartEndOffsets) + removeHighlights( + input.selection.overlapHighlights, + input.highlightStartEndOffsets + ) } const highlightAttributes = makeHighlightNodeAttributes( @@ -83,6 +87,8 @@ export async function createHighlight( patch, annotation: annotations.length > 0 ? annotations.join('\n') : undefined, articleId: input.articleId, + highlightPositionPercent: input.highlightPositionPercent, + highlightPositionAnchorIndex: input.highlightPositionAnchorIndex, } let highlight: Highlight | undefined @@ -98,7 +104,9 @@ export async function createHighlight( ($0) => !input.selection.overlapHighlights.includes($0.id) ) } else { - highlight = await articleMutations.createHighlightMutation(newHighlightAttributes) + highlight = await articleMutations.createHighlightMutation( + newHighlightAttributes + ) } if (highlight) { diff --git a/packages/web/lib/highlights/useSelection.tsx b/packages/web/lib/highlights/useSelection.tsx index 5070b182e..10fb104fc 100644 --- a/packages/web/lib/highlights/useSelection.tsx +++ b/packages/web/lib/highlights/useSelection.tsx @@ -7,10 +7,8 @@ import { import type { SelectionAttributes } from './highlightHelpers' export function useSelection( - highlightLocations: HighlightLocation[], - isDisabled: boolean + highlightLocations: HighlightLocation[] ): [SelectionAttributes | null, (x: SelectionAttributes | null) => void] { - const disabled = isDisabled const [selectionAttributes, setSelectionAttributes] = useState(null) @@ -142,10 +140,6 @@ export function useSelection( }, [selectionAttributes?.selection]) useEffect(() => { - if (disabled) { - return - } - document.addEventListener('mouseup', handleFinishTouch) document.addEventListener('touchend', handleFinishTouch) document.addEventListener('contextmenu', handleFinishTouch) @@ -157,7 +151,7 @@ export function useSelection( document.removeEventListener('contextmenu', handleFinishTouch) document.removeEventListener('copyTextSelection', copyTextSelection) } - }, [highlightLocations, handleFinishTouch, disabled, copyTextSelection]) + }, [highlightLocations, handleFinishTouch, copyTextSelection]) return [selectionAttributes, setSelectionAttributes] } diff --git a/packages/web/lib/networking/fragments/highlightFragment.ts b/packages/web/lib/networking/fragments/highlightFragment.ts index fc7bfabb2..d7ca16cf4 100644 --- a/packages/web/lib/networking/fragments/highlightFragment.ts +++ b/packages/web/lib/networking/fragments/highlightFragment.ts @@ -13,6 +13,8 @@ export const highlightFragment = gql` createdByMe updatedAt sharedAt + highlightPositionPercent + highlightPositionAnchorIndex labels { id name @@ -34,6 +36,8 @@ export type Highlight = { updatedAt: string sharedAt: string labels?: Label[] + highlightPositionPercent?: number + highlightPositionAnchorIndex?: number } export type User = { From a4ed72439faf97c6fc6f8087d1215244ab2cec73 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 1 Feb 2023 17:00:09 +0800 Subject: [PATCH 03/14] Sort highlights in notebooks --- .../templates/article/NotebookModal.tsx | 37 +++++++++++++++++-- .../networking/fragments/highlightFragment.ts | 2 + 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/packages/web/components/templates/article/NotebookModal.tsx b/packages/web/components/templates/article/NotebookModal.tsx index 4215435c4..a629f6fdf 100644 --- a/packages/web/components/templates/article/NotebookModal.tsx +++ b/packages/web/components/templates/article/NotebookModal.tsx @@ -17,7 +17,7 @@ import { TrashIcon } from '../../elements/images/TrashIcon' import { theme } from '../../tokens/stitches.config' import type { Highlight } from '../../../lib/networking/fragments/highlightFragment' import { HighlightView } from '../../patterns/HighlightView' -import { useCallback, useRef, useState } from 'react' +import { useCallback, useMemo, useRef, useState } from 'react' import { StyledTextArea } from '../../elements/StyledTextArea' import { ConfirmationModal } from '../../patterns/ConfirmationModal' import { DotsThree } from 'phosphor-react' @@ -27,6 +27,7 @@ import { Label } from '../../../lib/networking/fragments/labelFragment' import { setLabelsForHighlight } from '../../../lib/networking/mutations/setLabelsForHighlight' import { updateHighlightMutation } from '../../../lib/networking/mutations/updateHighlightMutation' import { showErrorToast, showSuccessToast } from '../../../lib/toastHelpers' +import { diff_match_patch } from 'diff-match-patch' type NotebookModalProps = { highlights: Highlight[] @@ -36,6 +37,12 @@ type NotebookModalProps = { onOpenChange: (open: boolean) => void } +export const getHighlightLocation = (patch: string): number | undefined => { + const dmp = new diff_match_patch() + const patches = dmp.patch_fromText(patch) + return patches[0].start1 || undefined +} + export function NotebookModal(props: NotebookModalProps): JSX.Element { const [showConfirmDeleteHighlightId, setShowConfirmDeleteHighlightId] = useState(undefined) @@ -44,6 +51,30 @@ export function NotebookModal(props: NotebookModalProps): JSX.Element { ) const [, updateState] = useState({}) + const sortedHighlights = useMemo(() => { + const sorted = (a: number, b: number) => { + if (a < b) { + return -1 + } + if (a > b) { + return 1 + } + return 0 + } + + return props.highlights.sort((a: Highlight, b: Highlight) => { + if (a.highlightPositionPercent && b.highlightPositionPercent) { + return sorted(a.highlightPositionPercent, b.highlightPositionPercent) + } + const aPos = getHighlightLocation(a.patch) + const bPos = getHighlightLocation(b.patch) + if (aPos && bPos) { + return sorted(aPos, bPos) + } + return a.createdAt.localeCompare(b.createdAt) + }) + }, [props.highlights]) + return ( @@ -57,7 +88,7 @@ export function NotebookModal(props: NotebookModalProps): JSX.Element { - {props.highlights.map((highlight) => ( + {sortedHighlights.map((highlight) => ( ))} - {props.highlights.length === 0 && ( + {sortedHighlights.length === 0 && ( You have not added any highlights or notes to this document diff --git a/packages/web/lib/networking/fragments/highlightFragment.ts b/packages/web/lib/networking/fragments/highlightFragment.ts index d7ca16cf4..47a938a6c 100644 --- a/packages/web/lib/networking/fragments/highlightFragment.ts +++ b/packages/web/lib/networking/fragments/highlightFragment.ts @@ -11,6 +11,7 @@ export const highlightFragment = gql` patch annotation createdByMe + createdAt updatedAt sharedAt highlightPositionPercent @@ -33,6 +34,7 @@ export type Highlight = { patch: string annotation?: string createdByMe: boolean + createdAt: string updatedAt: string sharedAt: string labels?: Label[] From f6622bc95938c2b6d6cb8e987180108ac8650b23 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 1 Feb 2023 17:04:14 +0800 Subject: [PATCH 04/14] Remove some debug code --- .../templates/article/HighlightsLayer.tsx | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/packages/web/components/templates/article/HighlightsLayer.tsx b/packages/web/components/templates/article/HighlightsLayer.tsx index 4dda397db..fc1fbaaff 100644 --- a/packages/web/components/templates/article/HighlightsLayer.tsx +++ b/packages/web/components/templates/article/HighlightsLayer.tsx @@ -444,23 +444,6 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element { ] ) - useEffect(() => { - ;(async () => { - if ( - 'enableSelectToHighlight' in window && - window.enableSelectToHighlight && - selectionData - ) { - // console.log('auto highlight: ', selectionData.selection.toString()) - await createHighlightCallback('none', undefined) - selectionData.selection.collapseToStart() - // handleClickHighlight(selectionData.mouseEvent) - // setSelectionData(null) - // window.getSelection()?.removeAllRanges() - } - })() - }, [selectionData, setSelectionData]) - const dispatchHighlightError = (action: string, error: unknown) => { if (props.isAppleAppEmbed) { window?.webkit?.messageHandlers.highlightAction?.postMessage({ From 707ec53cf6699a07a386fba81b894641e9709472 Mon Sep 17 00:00:00 2001 From: Jackson Harper Date: Wed, 1 Feb 2023 17:07:59 +0800 Subject: [PATCH 05/14] Remove unused test story --- .../stories/ShareHighlightModal.stories.tsx | 97 ------------------- 1 file changed, 97 deletions(-) delete mode 100644 packages/web/stories/ShareHighlightModal.stories.tsx diff --git a/packages/web/stories/ShareHighlightModal.stories.tsx b/packages/web/stories/ShareHighlightModal.stories.tsx deleted file mode 100644 index 593a2f55b..000000000 --- a/packages/web/stories/ShareHighlightModal.stories.tsx +++ /dev/null @@ -1,97 +0,0 @@ -import { ComponentStory, ComponentMeta } from '@storybook/react' -import {ShareHighlightModal} from '../components/templates/article/ShareHighlightModal'; -import { Highlight } from '../lib/networking/fragments/highlightFragment'; -import { updateThemeLocally } from '../lib/themeUpdater'; -import { ThemeId } from '../components/tokens/stitches.config'; - -export default { - title: 'Components/ShareHighlightModal', - parameters: { - previewTabs: { - 'storybook/docs/panel': { hidden: true } - }, - viewMode: 'canvas', - }, - component: ShareHighlightModal, - argTypes: { - author: {control: 'text'}, - title: {control: 'text'}, - }, -} as ComponentMeta - -const Template = (props: {highlight: Highlight, handleOpenChange: () => void, title: string, author: string}) => { - return ( - props.handleOpenChange()} - /> -)} - -const highlight: Highlight = { - id: "nnnnn", - shortId: "shortId", - quote: "children not only participate in herding work, but are also encouraged to act independently in most other areas of life. They have a say in deciding when to eat, when to sleep, and what to wear, even at temperatures of -30C (-22F).", - patch: "patchhhhhhy", - createdByMe: true, - updatedAt: '123', - sharedAt: '123', - prefix: "Among the Sami, an indigenous people spread across the northernmost regions of Norway, Sweden, Finland and Russia's Kola Peninsula,", - suffix: ' To outsiders, that independence can be surprising. Missionaries who visited the Arctic in the 18th Century and later, wrote in their diaries that it seemed like Sámi children could do whatever they liked, and that they lacked discipline altogether.', -} - -const highlightWithAnnotation: Highlight = { - ...highlight, - annotation: "Okay… this is wild! I love this independence. Wondering how I can reponsibly instill this type of indepence in my own kids…", -} - -export const LightShareHightlightModal: ComponentStory = (args: any) => { - updateThemeLocally(ThemeId.Light); - highlight.annotation = undefined; - return ( -