Benature/omnivore
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix issues with merging highlights

Browse Source 
highlightLocations can't be a prop or they wont be updated
properly when highlights are created and merged.

This fixes issues with merging highlights. It changes how we
scroll to an initial highlight in the case of deep links too.
This commit is contained in:
Jackson Harper
2022-06-23 16:09:25 -07:00
parent 5fdc03d87f
commit da6103b444
3 changed files with 57 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
packages/web/components/templates/article/Article.tsx
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,

24
packages/web/components/templates/article/ArticleContainer.tsx
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 ?? ''}

73
packages/web/components/templates/article/HighlightsLayer.tsx
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 <></>
}
}