Merge pull request #862 from omnivore-app/fix/highlight-locations

Fix issues with merging highlights
This commit is contained in:
Jackson Harper
2022-06-23 16:28:43 -07:00
committed by GitHub
3 changed files with 57 additions and 50 deletions

View File

@ -21,7 +21,6 @@ import { ArticleMutations } from '../../../lib/articleActions'
export type ArticleProps = {
articleId: string
content: string
highlightReady: boolean
initialAnchorIndex: number
initialReadingProgress?: number
highlightHref: MutableRefObject<string | null>
@ -128,14 +127,17 @@ export function Article(props: ArticleProps): JSX.Element {
if (typeof window === 'undefined') {
return
}
if (props.highlightReady) {
if (!shouldScrollToInitialPosition) {
return
}
setShouldScrollToInitialPosition(false)
// If we are scrolling to a highlight, dont scroll to read position
if (props.highlightHref.current) {
return
}
if (props.initialReadingProgress && props.initialReadingProgress >= 98) {
return
}
@ -165,9 +167,7 @@ export function Article(props: ArticleProps): JSX.Element {
const calculatedOffset = calculateOffset(anchorElement)
window.document.documentElement.scroll(0, calculatedOffset - 100)
}
}
}, [
props.highlightReady,
props.initialAnchorIndex,
props.initialReadingProgress,
shouldScrollToInitialPosition,

View File

@ -56,10 +56,6 @@ export function ArticleContainer(props: ArticleContainerProps): JSX.Element {
const highlightHref = useRef(
window.location.hash ? window.location.hash.split('#')[1] : null
)
const [highlightReady, setHighlightReady] = useState(false)
const [highlightLocations, setHighlightLocations] = useState<
HighlightLocation[]
>([])
const updateFontSize = async (newFontSize: number) => {
if (fontSize !== newFontSize) {
@ -72,21 +68,6 @@ export function ArticleContainer(props: ArticleContainerProps): JSX.Element {
updateFontSize(props.fontSize ?? 20)
}, [props.fontSize])
// Load the highlights
useEffect(() => {
const res: HighlightLocation[] = []
props.article.highlights.forEach((highlight) => {
try {
const offset = makeHighlightStartEndOffset(highlight)
res.push(offset)
} catch (err) {
console.error(err)
}
})
setHighlightLocations(res)
setHighlightReady(true)
}, [props.article.highlights, setHighlightLocations])
// Listen for preference change events sent from host apps (ios, macos...)
useEffect(() => {
interface UpdateLineHeightEvent extends Event {
@ -275,10 +256,9 @@ export function ArticleContainer(props: ArticleContainerProps): JSX.Element {
) : null}
</VStack>
<Article
highlightReady={highlightReady}
highlightHref={highlightHref}
articleId={props.article.id}
content={props.article.content}
highlightHref={highlightHref}
initialAnchorIndex={props.article.readingProgressAnchorIndex}
articleMutations={props.articleMutations}
/>
@ -300,7 +280,7 @@ export function ArticleContainer(props: ArticleContainerProps): JSX.Element {
<Box css={{ height: '100px' }} />
</Box>
<HighlightsLayer
highlightLocations={highlightLocations}
scrollToHighlight={highlightHref}
highlights={props.article.highlights}
articleTitle={props.article.title}
articleAuthor={props.article.author ?? ''}

View File

@ -1,4 +1,4 @@
import { useEffect, useRef, useCallback, useState } from 'react'
import { useEffect, useRef, useCallback, useState, MutableRefObject } from 'react'
import { makeHighlightStartEndOffset } from '../../../lib/highlights/highlightGenerator'
import type { HighlightLocation } from '../../../lib/highlights/highlightGenerator'
import { useSelection } from '../../../lib/highlights/useSelection'
@ -27,9 +27,10 @@ type HighlightsLayerProps = {
highlightBarDisabled: boolean
showHighlightsModal: boolean
highlightsBaseURL: string
scrollToHighlight: MutableRefObject<string | null>
setShowHighlightsModal: React.Dispatch<React.SetStateAction<boolean>>
articleMutations: ArticleMutations
highlightLocations: HighlightLocation[]
}
type HighlightModalAction = 'none' | 'addComment' | 'share'
@ -50,6 +51,9 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element {
const [highlightModalAction, setHighlightModalAction] =
useState<HighlightActionProps>({ highlightModalAction: 'none' })
const [highlightLocations, setHighlightLocations] = useState<
HighlightLocation[]
>([])
const focusedHighlightMousePos = useRef({ pageX: 0, pageY: 0 })
const [focusedHighlight, setFocusedHighlight] = useState<
@ -57,12 +61,36 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element {
>(undefined)
const [selectionData, setSelectionData] = useSelection(
props.highlightLocations,
highlightLocations,
false //noteModal.open,
)
const canShareNative = useCanShareNative()
// Load the highlights
useEffect(() => {
const res: HighlightLocation[] = []
highlights.forEach((highlight) => {
try {
const offset = makeHighlightStartEndOffset(highlight)
res.push(offset)
} catch (err) {
console.error(err)
}
})
setHighlightLocations(res)
// If we were given an initial highlight to scroll to we do
// that now that all the content has been injected into the
// page.
if (props.scrollToHighlight.current) {
const anchorElement = document.querySelector(`[omnivore-highlight-id="${props.scrollToHighlight.current}"]`)
if (anchorElement) {
anchorElement.scrollIntoView({ behavior: 'auto' })
}
}
}, [highlights, setHighlightLocations])
const removeHighlightCallback = useCallback(
async (id?: string) => {
const highlightId = id || focusedHighlight?.id
@ -73,7 +101,7 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element {
if (didDeleteHighlight) {
removeHighlights(
highlights.map(($0) => $0.id),
props.highlightLocations
highlightLocations
)
setHighlights(highlights.filter(($0) => $0.id !== highlightId))
setFocusedHighlight(undefined)
@ -81,16 +109,16 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element {
console.error('Failed to delete highlight')
}
},
[focusedHighlight, highlights, props.highlightLocations]
[focusedHighlight, highlights, highlightLocations]
)
const updateHighlightsCallback = useCallback(
(highlight: Highlight) => {
removeHighlights([highlight.id], props.highlightLocations)
removeHighlights([highlight.id], highlightLocations)
const keptHighlights = highlights.filter(($0) => $0.id !== highlight.id)
setHighlights([...keptHighlights, highlight])
},
[highlights, props.highlightLocations]
[highlights, highlightLocations]
)
const handleNativeShare = useCallback(
@ -143,7 +171,7 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element {
selection: selection,
articleId: props.articleId,
existingHighlights: highlights,
highlightStartEndOffsets: props.highlightLocations,
highlightStartEndOffsets: highlightLocations,
annotation: note,
}, props.articleMutations)
@ -198,21 +226,21 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element {
selectionData,
setSelectionData,
canShareNative,
props.highlightLocations,
highlightLocations,
]
)
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)
}
}
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(
@ -257,7 +285,7 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element {
})
} else setFocusedHighlight(undefined)
},
[highlights, props.highlightLocations]
[highlights, highlightLocations]
)
useEffect(() => {
@ -465,10 +493,9 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element {
if (props.showHighlightsModal) {
return (
<HighlightsModal
highlights={highlights}
onOpenChange={() => props.setShowHighlightsModal(false)}
scrollToHighlight={scrollToHighlight}
deleteHighlightAction={(highlightId: string) => {
highlights={highlights}
onOpenChange={() => props.setShowHighlightsModal(false)}
deleteHighlightAction={(highlightId: string) => {
removeHighlightCallback(highlightId)
}}
/>
@ -476,4 +503,4 @@ export function HighlightsLayer(props: HighlightsLayerProps): JSX.Element {
}
return <></>
}
}