mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(reader): update existing annotation instead of duplicating
Prevent duplicate annotations from being created when a user modifies the style or color of an existing highlight. Route the popover actions through a PATCH request on the existing annotation ID instead of invoking the creation flow. Closes #543
This commit is contained in:
@@ -13,6 +13,7 @@ import { useVisibility } from './shared/composables/useVisibility'
|
||||
import { useWakeLock } from './shared/composables/useWakeLock'
|
||||
import { useBookmarks } from './epub/composables/useBookmarks'
|
||||
import { useAnnotations } from './epub/composables/useAnnotations'
|
||||
import { useReaderAnnotationActions } from './epub/composables/useReaderAnnotationActions'
|
||||
import { useToc } from './epub/composables/useToc'
|
||||
import { useSearch, type FoliateView } from './epub/composables/useSearch'
|
||||
import { useReaderSelection } from './epub/composables/useReaderSelection'
|
||||
@@ -35,7 +36,7 @@ import AudiobookReaderView from './audiobook/AudiobookReaderView.vue'
|
||||
import type { ReaderState } from './epub/composables/useReaderState'
|
||||
import type { FoliateLocationContext, FoliateRenderer } from './epub/composables/useFoliate'
|
||||
import type { EpubReaderSettings } from '@bookorbit/types'
|
||||
import { cfiRangesOverlap } from './epub/utils'
|
||||
import { findMatchingCfiRange } from './epub/utils'
|
||||
import { getFormatGroup } from '@bookorbit/types'
|
||||
|
||||
const route = useRoute()
|
||||
@@ -241,6 +242,7 @@ const {
|
||||
addAnnotation,
|
||||
addAnnotations,
|
||||
deleteAnnotation,
|
||||
redrawAnnotation,
|
||||
setTextSelectedHandler,
|
||||
setAnnotationClickHandler,
|
||||
view: foliateView,
|
||||
@@ -248,9 +250,18 @@ const {
|
||||
isFixedLayout,
|
||||
} = useFoliate(() => containerRef.value, onRelocateHandler, onApplyStylesHandler, onMiddleTapHandler)
|
||||
|
||||
const { handleHighlight, handleOpenNoteDialog, handleSaveNote } = useReaderAnnotationActions({
|
||||
bookId,
|
||||
fileId,
|
||||
chapterTitle,
|
||||
annotations,
|
||||
selection,
|
||||
addAnnotation,
|
||||
redrawAnnotation,
|
||||
})
|
||||
|
||||
function handleTextSelected(detail: SelectionDetail) {
|
||||
const selCfi = detail.cfi
|
||||
const match = selCfi ? (annotations.annotations.value.find((a) => a.cfi != null && cfiRangesOverlap(selCfi, a.cfi)) ?? null) : null
|
||||
const match = findMatchingCfiRange(annotations.annotations.value, detail.cfi)
|
||||
selection.show(detail, match?.id ?? null)
|
||||
}
|
||||
|
||||
@@ -406,30 +417,6 @@ watch(
|
||||
},
|
||||
)
|
||||
|
||||
async function handleHighlight(color: string, style: string, note?: string) {
|
||||
const annotationCfi = selection.cfi.value
|
||||
if (!selection.text.value || !annotationCfi) return
|
||||
const created = await annotations.create(bookId, {
|
||||
cfi: annotationCfi,
|
||||
bookFileId: fileId,
|
||||
text: selection.text.value,
|
||||
color,
|
||||
style,
|
||||
note: note ?? null,
|
||||
chapterTitle: chapterTitle.value || null,
|
||||
})
|
||||
if (created?.cfi) {
|
||||
addAnnotation(created.cfi, created.color, created.style)
|
||||
}
|
||||
selection.dismiss()
|
||||
}
|
||||
|
||||
async function handleSaveNote(note: string) {
|
||||
await handleHighlight('#FACC15', 'highlight', note)
|
||||
selection.showNoteDialog.value = false
|
||||
selection.noteText.value = ''
|
||||
}
|
||||
|
||||
function handleDeleteAnnotation(id: number) {
|
||||
const ann = annotations.annotations.value.find((a) => a.id === id)
|
||||
if (ann) {
|
||||
@@ -703,7 +690,7 @@ watch(
|
||||
@search="() => openSearchWithText(selection.text.value)"
|
||||
@translate="handleTranslate"
|
||||
@define="handleDefine"
|
||||
@note="selection.openNoteDialog()"
|
||||
@note="handleOpenNoteDialog"
|
||||
@deleteAnnotation="handleDeleteAnnotation"
|
||||
@dismiss="selection.dismiss()"
|
||||
/>
|
||||
|
||||
@@ -1,5 +1,14 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { cfiRangesOverlap, findNearestCfi, formatCfiLocation, formatDate, getCfiSortKey, stripFragment } from '../utils'
|
||||
import {
|
||||
cfiRangesMatch,
|
||||
cfiRangesOverlap,
|
||||
findMatchingCfiRange,
|
||||
findNearestCfi,
|
||||
formatCfiLocation,
|
||||
formatDate,
|
||||
getCfiSortKey,
|
||||
stripFragment,
|
||||
} from '../utils'
|
||||
|
||||
describe('epub utils', () => {
|
||||
it('removes hash fragments from href values', () => {
|
||||
@@ -101,3 +110,52 @@ describe('cfiRangesOverlap', () => {
|
||||
expect(cfiRangesOverlap(ann, 'not-a-cfi')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('cfiRangesMatch', () => {
|
||||
const ann = 'epubcfi(/6/4!/4/2,/2/1:0,/2/1:20)'
|
||||
|
||||
it('detects exact range matches', () => {
|
||||
expect(cfiRangesMatch(ann, ann)).toBe(true)
|
||||
})
|
||||
|
||||
it('does not treat subset selections as the existing annotation range', () => {
|
||||
const subset = 'epubcfi(/6/4!/4/2,/2/1:5,/2/1:10)'
|
||||
expect(cfiRangesOverlap(subset, ann)).toBe(true)
|
||||
expect(cfiRangesMatch(subset, ann)).toBe(false)
|
||||
})
|
||||
|
||||
it('does not treat superset or partial overlaps as the existing annotation range', () => {
|
||||
const superset = 'epubcfi(/6/4!/4/2,/2/1:0,/2/1:50)'
|
||||
const partial = 'epubcfi(/6/4!/4/2,/2/1:10,/2/1:30)'
|
||||
|
||||
expect(cfiRangesOverlap(superset, ann)).toBe(true)
|
||||
expect(cfiRangesOverlap(partial, ann)).toBe(true)
|
||||
expect(cfiRangesMatch(superset, ann)).toBe(false)
|
||||
expect(cfiRangesMatch(partial, ann)).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false for different spine items and invalid CFI strings', () => {
|
||||
expect(cfiRangesMatch('epubcfi(/6/6!/4/2,/2/1:0,/2/1:20)', ann)).toBe(false)
|
||||
expect(cfiRangesMatch('not-a-cfi', ann)).toBe(false)
|
||||
expect(cfiRangesMatch(ann, 'not-a-cfi')).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('findMatchingCfiRange', () => {
|
||||
const ann = { id: 1, cfi: 'epubcfi(/6/4!/4/2,/2/1:0,/2/1:20)' }
|
||||
|
||||
it('returns an annotation only when the selected range exactly matches it', () => {
|
||||
expect(findMatchingCfiRange([ann], ann.cfi)?.id).toBe(1)
|
||||
})
|
||||
|
||||
it('does not return a parent annotation when the selection is only a subset', () => {
|
||||
const subset = 'epubcfi(/6/4!/4/2,/2/1:5,/2/1:10)'
|
||||
|
||||
expect(cfiRangesOverlap(subset, ann.cfi)).toBe(true)
|
||||
expect(findMatchingCfiRange([ann], subset)).toBeNull()
|
||||
})
|
||||
|
||||
it('returns null when there is no selected CFI', () => {
|
||||
expect(findMatchingCfiRange([ann], null)).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -120,9 +120,44 @@ describe('useAnnotations', () => {
|
||||
|
||||
await store.updateNote(9, 2, 'New note')
|
||||
|
||||
const [url, req] = apiMock.mock.calls[0] as [string, RequestInit]
|
||||
expect(url).toBe('/api/v1/books/9/annotations/2')
|
||||
expect(req.method).toBe('PATCH')
|
||||
expect(JSON.parse(String(req.body))).toEqual({ note: 'New note' })
|
||||
expect(store.annotations.value).toEqual([existing[0], updated])
|
||||
})
|
||||
|
||||
it('patches color and style without appending a duplicate annotation', async () => {
|
||||
const existing = [makeAnnotation(1), makeAnnotation(2)]
|
||||
const updated = { ...existing[0]!, color: '#38BDF8', style: 'underline' }
|
||||
apiMock.mockResolvedValueOnce(response(true, updated))
|
||||
|
||||
const store = useAnnotations()
|
||||
store.annotations.value = existing
|
||||
|
||||
const result = await store.update(9, 1, { color: '#38BDF8', style: 'underline' })
|
||||
|
||||
const [url, req] = apiMock.mock.calls[0] as [string, RequestInit]
|
||||
expect(url).toBe('/api/v1/books/9/annotations/1')
|
||||
expect(req.method).toBe('PATCH')
|
||||
expect(JSON.parse(String(req.body))).toEqual({ color: '#38BDF8', style: 'underline' })
|
||||
expect(result).toEqual(updated)
|
||||
expect(store.annotations.value).toEqual([updated, existing[1]])
|
||||
})
|
||||
|
||||
it('returns null and leaves local annotations unchanged when update fails', async () => {
|
||||
const existing = [makeAnnotation(1), makeAnnotation(2)]
|
||||
apiMock.mockResolvedValueOnce(response(false))
|
||||
|
||||
const store = useAnnotations()
|
||||
store.annotations.value = existing
|
||||
|
||||
const result = await store.update(9, 1, { color: '#38BDF8' })
|
||||
|
||||
expect(result).toBeNull()
|
||||
expect(store.annotations.value).toEqual(existing)
|
||||
})
|
||||
|
||||
it('removes annotation when delete succeeds', async () => {
|
||||
apiMock.mockResolvedValueOnce(response(true))
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ vi.mock('../useFoliateAnnotations', () => ({
|
||||
addAnnotation: vi.fn<() => void>(),
|
||||
addAnnotations: vi.fn<() => void>(),
|
||||
deleteAnnotation: vi.fn<() => void>(),
|
||||
redrawAnnotation: vi.fn<() => void>(),
|
||||
reAddAll: vi.fn<() => void>(),
|
||||
handleDrawAnnotationEvent: vi.fn<() => void>(),
|
||||
}),
|
||||
@@ -23,11 +24,14 @@ vi.mock('../useFoliateSelection', () => ({
|
||||
}),
|
||||
}))
|
||||
|
||||
const inputMock = vi.hoisted(() => ({
|
||||
cleanup: vi.fn<() => void>(),
|
||||
attachIframeClicks: vi.fn<() => void>(),
|
||||
suppressNextTapNavigation: vi.fn<() => void>(),
|
||||
}))
|
||||
|
||||
vi.mock('../useFoliateInput', () => ({
|
||||
useFoliateInput: () => ({
|
||||
cleanup: vi.fn<() => void>(),
|
||||
attachIframeClicks: vi.fn<() => void>(),
|
||||
}),
|
||||
useFoliateInput: () => inputMock,
|
||||
}))
|
||||
|
||||
import { api } from '@/lib/api'
|
||||
@@ -39,8 +43,13 @@ describe('useFoliate.open', () => {
|
||||
let mockGoTo: ReturnType<typeof vi.fn>
|
||||
let mockGoToFraction: ReturnType<typeof vi.fn>
|
||||
let includeGoToFraction: boolean
|
||||
let viewEl: HTMLElement | null
|
||||
|
||||
beforeEach(() => {
|
||||
inputMock.cleanup.mockReset()
|
||||
inputMock.attachIframeClicks.mockReset()
|
||||
inputMock.suppressNextTapNavigation.mockReset()
|
||||
|
||||
container = document.createElement('div')
|
||||
document.body.appendChild(container)
|
||||
|
||||
@@ -48,11 +57,13 @@ describe('useFoliate.open', () => {
|
||||
mockGoTo = vi.fn<(target?: unknown) => Promise<unknown>>().mockResolvedValue({ index: 0 })
|
||||
mockGoToFraction = vi.fn<(fraction?: unknown) => Promise<void>>().mockResolvedValue(undefined)
|
||||
includeGoToFraction = true
|
||||
viewEl = null
|
||||
|
||||
const originalCreateElement = document.createElement.bind(document)
|
||||
vi.spyOn(document, 'createElement').mockImplementation((tag: string) => {
|
||||
if (tag === 'foliate-view') {
|
||||
const el = originalCreateElement('div')
|
||||
viewEl = el
|
||||
const view = {
|
||||
open: mockOpen,
|
||||
goTo: mockGoTo,
|
||||
@@ -239,4 +250,17 @@ describe('useFoliate.open', () => {
|
||||
expect(mockGoTo).toHaveBeenCalledWith('epubcfi(/6/4!)')
|
||||
expect(mockGoToFraction).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('suppresses tap navigation when an annotation is shown', async () => {
|
||||
const foliate = useFoliate(() => container)
|
||||
const onAnnotationClick = vi.fn<(cfi: string, popupPosition: { x: number; y: number; showBelow: boolean }) => void>()
|
||||
foliate.setAnnotationClickHandler(onAnnotationClick)
|
||||
|
||||
await foliate.open(1, 1, 'epub', null, undefined)
|
||||
viewEl?.dispatchEvent(new CustomEvent('show-annotation', { detail: { value: 'epubcfi(/6/4)' } }))
|
||||
|
||||
expect(inputMock.suppressNextTapNavigation).toHaveBeenCalledTimes(1)
|
||||
expect(onAnnotationClick).toHaveBeenCalledWith('epubcfi(/6/4)', expect.objectContaining({ x: expect.any(Number), y: expect.any(Number) }))
|
||||
expect(inputMock.suppressNextTapNavigation.mock.invocationCallOrder[0]!).toBeLessThan(onAnnotationClick.mock.invocationCallOrder[0]!)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -31,6 +31,24 @@ describe('useFoliateAnnotations', () => {
|
||||
expect(anns.annotationStyleMap.get('epubcfi(/6/6)')).toEqual({ color: '#FB923C', style: 'strikethrough' })
|
||||
})
|
||||
|
||||
it('redraws an existing annotation by deleting then re-adding it with updated style metadata', () => {
|
||||
const view = {
|
||||
addAnnotation: vi.fn<(arg: { value: string }) => void>(),
|
||||
deleteAnnotation: vi.fn<(arg: { value: string }) => void>(),
|
||||
}
|
||||
const anns = useFoliateAnnotations()
|
||||
|
||||
anns.addAnnotation(view, 'epubcfi(/6/4)', '#FACC15', 'highlight')
|
||||
view.addAnnotation.mockClear()
|
||||
|
||||
anns.redrawAnnotation(view, 'epubcfi(/6/4)', '#38BDF8', 'underline')
|
||||
|
||||
expect(anns.annotationStyleMap.get('epubcfi(/6/4)')).toEqual({ color: '#38BDF8', style: 'underline' })
|
||||
expect(view.deleteAnnotation).toHaveBeenCalledWith({ value: 'epubcfi(/6/4)' })
|
||||
expect(view.addAnnotation).toHaveBeenCalledWith({ value: 'epubcfi(/6/4)' })
|
||||
expect(view.deleteAnnotation.mock.invocationCallOrder[0]!).toBeLessThan(view.addAnnotation.mock.invocationCallOrder[0]!)
|
||||
})
|
||||
|
||||
it('re-adds all known annotations after renderer reload', () => {
|
||||
vi.useFakeTimers()
|
||||
const view = { addAnnotation: vi.fn<(arg: { value: string }) => void>() }
|
||||
|
||||
@@ -236,6 +236,88 @@ describe('useFoliateInput', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('ignores a page-turn click message that arrives after annotation tap suppression starts', () => {
|
||||
vi.useFakeTimers()
|
||||
|
||||
const next = vi.fn<() => void>()
|
||||
const onMiddleTap = vi.fn<() => void>()
|
||||
const view: ViewLike = {
|
||||
prev: vi.fn<() => void>(),
|
||||
next,
|
||||
getBoundingClientRect: () => ({ left: 0, width: 100 }) as DOMRect,
|
||||
}
|
||||
|
||||
const input = useFoliateInput(() => view, onMiddleTap, vi.fn<() => void>(), vi.fn<() => void>())
|
||||
const doc = makeDocTarget()
|
||||
input.attachIframeClicks(doc)
|
||||
|
||||
const originalMaxTouchPoints = Object.getOwnPropertyDescriptor(navigator, 'maxTouchPoints')
|
||||
const originalOntouchstart = Object.getOwnPropertyDescriptor(window, 'ontouchstart')
|
||||
Object.defineProperty(navigator, 'maxTouchPoints', {
|
||||
configurable: true,
|
||||
get: () => 0,
|
||||
})
|
||||
Reflect.deleteProperty(window as unknown as Record<string, unknown>, 'ontouchstart')
|
||||
|
||||
doc.dispatchEvent(new MouseEvent('mousedown', { bubbles: true }))
|
||||
input.suppressNextTapNavigation()
|
||||
window.dispatchEvent(new MessageEvent('message', { data: { type: 'foliate-click', clientX: 95 }, origin: window.location.origin }))
|
||||
vi.advanceTimersByTime(500)
|
||||
|
||||
expect(next).not.toHaveBeenCalled()
|
||||
expect(onMiddleTap).not.toHaveBeenCalled()
|
||||
|
||||
input.cleanup()
|
||||
|
||||
if (originalMaxTouchPoints) {
|
||||
Object.defineProperty(navigator, 'maxTouchPoints', originalMaxTouchPoints)
|
||||
}
|
||||
if (originalOntouchstart) {
|
||||
Object.defineProperty(window, 'ontouchstart', originalOntouchstart)
|
||||
}
|
||||
})
|
||||
|
||||
it('cancels a queued page turn when annotation tap suppression starts before the delay completes', () => {
|
||||
vi.useFakeTimers()
|
||||
|
||||
const next = vi.fn<() => void>()
|
||||
const onMiddleTap = vi.fn<() => void>()
|
||||
const view: ViewLike = {
|
||||
prev: vi.fn<() => void>(),
|
||||
next,
|
||||
getBoundingClientRect: () => ({ left: 0, width: 100 }) as DOMRect,
|
||||
}
|
||||
|
||||
const input = useFoliateInput(() => view, onMiddleTap, vi.fn<() => void>(), vi.fn<() => void>())
|
||||
const doc = makeDocTarget()
|
||||
input.attachIframeClicks(doc)
|
||||
|
||||
const originalMaxTouchPoints = Object.getOwnPropertyDescriptor(navigator, 'maxTouchPoints')
|
||||
const originalOntouchstart = Object.getOwnPropertyDescriptor(window, 'ontouchstart')
|
||||
Object.defineProperty(navigator, 'maxTouchPoints', {
|
||||
configurable: true,
|
||||
get: () => 0,
|
||||
})
|
||||
Reflect.deleteProperty(window as unknown as Record<string, unknown>, 'ontouchstart')
|
||||
|
||||
doc.dispatchEvent(new MouseEvent('mousedown', { bubbles: true }))
|
||||
window.dispatchEvent(new MessageEvent('message', { data: { type: 'foliate-click', clientX: 95 }, origin: window.location.origin }))
|
||||
input.suppressNextTapNavigation()
|
||||
vi.advanceTimersByTime(500)
|
||||
|
||||
expect(next).not.toHaveBeenCalled()
|
||||
expect(onMiddleTap).not.toHaveBeenCalled()
|
||||
|
||||
input.cleanup()
|
||||
|
||||
if (originalMaxTouchPoints) {
|
||||
Object.defineProperty(navigator, 'maxTouchPoints', originalMaxTouchPoints)
|
||||
}
|
||||
if (originalOntouchstart) {
|
||||
Object.defineProperty(window, 'ontouchstart', originalOntouchstart)
|
||||
}
|
||||
})
|
||||
|
||||
it('stops responding to document keydown after cleanup', () => {
|
||||
const next = vi.fn<() => void>()
|
||||
const view: ViewLike = {
|
||||
|
||||
+148
@@ -0,0 +1,148 @@
|
||||
import { ref } from 'vue'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import type { Annotation } from '../useAnnotations'
|
||||
import { useReaderAnnotationActions } from '../useReaderAnnotationActions'
|
||||
|
||||
function makeAnnotation(overrides: Partial<Annotation> = {}): Annotation {
|
||||
return {
|
||||
id: 42,
|
||||
bookId: 9,
|
||||
cfi: 'epubcfi(/6/4!/4/2,/1:0,/1:10)',
|
||||
jumpFileId: 33,
|
||||
pageno: null,
|
||||
text: 'Selected text',
|
||||
color: '#FACC15',
|
||||
style: 'highlight',
|
||||
note: null,
|
||||
chapterTitle: 'Intro',
|
||||
origin: 'web',
|
||||
positionStatus: 'exact',
|
||||
chapterIndex: null,
|
||||
createdAt: '2026-01-01T00:00:00.000Z',
|
||||
...overrides,
|
||||
}
|
||||
}
|
||||
|
||||
function makeHarness(initialAnnotations: Annotation[] = []) {
|
||||
const annotationItems = ref<Annotation[]>(initialAnnotations)
|
||||
const create = vi.fn<() => Promise<Annotation | null>>()
|
||||
const update = vi.fn<() => Promise<Annotation | null>>()
|
||||
const addAnnotation = vi.fn<(cfi: string, color: string, style: string) => void>()
|
||||
const redrawAnnotation = vi.fn<(cfi: string, color: string, style: string) => void>()
|
||||
|
||||
const selection = {
|
||||
text: ref('Selected text'),
|
||||
cfi: ref<string | null>('epubcfi(/6/4!/4/2,/1:0,/1:10)'),
|
||||
overlappingAnnotationId: ref<number | null>(null),
|
||||
showNoteDialog: ref(false),
|
||||
noteText: ref(''),
|
||||
dismiss: vi.fn<() => void>(),
|
||||
openNoteDialog: vi.fn<(initialNote?: string) => void>((initialNote = '') => {
|
||||
selection.noteText.value = initialNote
|
||||
selection.showNoteDialog.value = true
|
||||
selection.dismiss()
|
||||
}),
|
||||
}
|
||||
|
||||
const actions = useReaderAnnotationActions({
|
||||
bookId: 9,
|
||||
fileId: 33,
|
||||
chapterTitle: ref('Intro'),
|
||||
annotations: {
|
||||
annotations: annotationItems,
|
||||
create,
|
||||
update,
|
||||
},
|
||||
selection,
|
||||
addAnnotation,
|
||||
redrawAnnotation,
|
||||
})
|
||||
|
||||
return { actions, annotationItems, create, update, addAnnotation, redrawAnnotation, selection }
|
||||
}
|
||||
|
||||
describe('useReaderAnnotationActions', () => {
|
||||
it('creates and draws a new highlight when the selection does not overlap an existing annotation', async () => {
|
||||
const created = makeAnnotation({ id: 99, color: '#38BDF8', style: 'underline' })
|
||||
const { actions, create, update, addAnnotation, redrawAnnotation, selection } = makeHarness()
|
||||
create.mockResolvedValueOnce(created)
|
||||
|
||||
await actions.handleHighlight('#38BDF8', 'underline')
|
||||
|
||||
expect(create).toHaveBeenCalledWith(9, {
|
||||
cfi: 'epubcfi(/6/4!/4/2,/1:0,/1:10)',
|
||||
bookFileId: 33,
|
||||
text: 'Selected text',
|
||||
color: '#38BDF8',
|
||||
style: 'underline',
|
||||
note: null,
|
||||
chapterTitle: 'Intro',
|
||||
})
|
||||
expect(update).not.toHaveBeenCalled()
|
||||
expect(addAnnotation).toHaveBeenCalledWith(created.cfi, '#38BDF8', 'underline')
|
||||
expect(redrawAnnotation).not.toHaveBeenCalled()
|
||||
expect(selection.dismiss).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('updates and redraws an existing annotation instead of creating a duplicate', async () => {
|
||||
const existing = makeAnnotation({ id: 42, color: '#FACC15', style: 'highlight' })
|
||||
const updated = makeAnnotation({ id: 42, color: '#F472B6', style: 'squiggly' })
|
||||
const { actions, create, update, addAnnotation, redrawAnnotation, selection } = makeHarness([existing])
|
||||
selection.overlappingAnnotationId.value = 42
|
||||
update.mockResolvedValueOnce(updated)
|
||||
|
||||
await actions.handleHighlight('#F472B6', 'squiggly')
|
||||
|
||||
expect(update).toHaveBeenCalledWith(9, 42, { color: '#F472B6', style: 'squiggly' })
|
||||
expect(create).not.toHaveBeenCalled()
|
||||
expect(addAnnotation).not.toHaveBeenCalled()
|
||||
expect(redrawAnnotation).toHaveBeenCalledWith(updated.cfi, '#F472B6', 'squiggly')
|
||||
expect(selection.dismiss).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('saves a note onto an existing annotation without changing its drawn style', async () => {
|
||||
const existing = makeAnnotation({ id: 42, note: 'old note' })
|
||||
const updated = makeAnnotation({ id: 42, note: 'edited note' })
|
||||
const { actions, create, update, redrawAnnotation, selection } = makeHarness([existing])
|
||||
selection.overlappingAnnotationId.value = 42
|
||||
selection.showNoteDialog.value = true
|
||||
selection.noteText.value = 'draft'
|
||||
update.mockResolvedValueOnce(updated)
|
||||
|
||||
await actions.handleSaveNote('edited note')
|
||||
|
||||
expect(update).toHaveBeenCalledWith(9, 42, { note: 'edited note' })
|
||||
expect(create).not.toHaveBeenCalled()
|
||||
expect(redrawAnnotation).not.toHaveBeenCalled()
|
||||
expect(selection.showNoteDialog.value).toBe(false)
|
||||
expect(selection.noteText.value).toBe('')
|
||||
})
|
||||
|
||||
it('creates a default highlight with the saved note when annotating new text', async () => {
|
||||
const created = makeAnnotation({ id: 99, note: 'new note' })
|
||||
const { actions, create, update, addAnnotation, selection } = makeHarness()
|
||||
selection.showNoteDialog.value = true
|
||||
selection.noteText.value = 'draft'
|
||||
create.mockResolvedValueOnce(created)
|
||||
|
||||
await actions.handleSaveNote('new note')
|
||||
|
||||
expect(create).toHaveBeenCalledWith(9, expect.objectContaining({ color: '#FACC15', style: 'highlight', note: 'new note' }))
|
||||
expect(update).not.toHaveBeenCalled()
|
||||
expect(addAnnotation).toHaveBeenCalledWith(created.cfi, '#FACC15', 'highlight')
|
||||
expect(selection.showNoteDialog.value).toBe(false)
|
||||
expect(selection.noteText.value).toBe('')
|
||||
})
|
||||
|
||||
it('opens the note dialog with the existing annotation note when available', () => {
|
||||
const existing = makeAnnotation({ id: 42, note: 'old note' })
|
||||
const { actions, selection } = makeHarness([existing])
|
||||
selection.overlappingAnnotationId.value = 42
|
||||
|
||||
actions.handleOpenNoteDialog()
|
||||
|
||||
expect(selection.openNoteDialog).toHaveBeenCalledWith('old note')
|
||||
expect(selection.noteText.value).toBe('old note')
|
||||
expect(selection.showNoteDialog.value).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -36,15 +36,15 @@ describe('useReaderSelection', () => {
|
||||
expect(selection.overlappingAnnotationId.value).toBe(42)
|
||||
})
|
||||
|
||||
it('dismisses popup and opens note dialog with clean note body', () => {
|
||||
it('dismisses popup and opens note dialog with provided note body', () => {
|
||||
const selection = useReaderSelection()
|
||||
selection.visible.value = true
|
||||
selection.noteText.value = 'old note'
|
||||
|
||||
selection.openNoteDialog()
|
||||
selection.openNoteDialog('existing note')
|
||||
|
||||
expect(selection.showNoteDialog.value).toBe(true)
|
||||
expect(selection.noteText.value).toBe('')
|
||||
expect(selection.noteText.value).toBe('existing note')
|
||||
expect(selection.visible.value).toBe(false)
|
||||
|
||||
selection.dismiss()
|
||||
|
||||
@@ -4,6 +4,12 @@ import { api } from '@/lib/api'
|
||||
|
||||
export type Annotation = AnnotationItem
|
||||
|
||||
export interface AnnotationPatch {
|
||||
note?: string | null
|
||||
color?: string
|
||||
style?: string
|
||||
}
|
||||
|
||||
export function useAnnotations() {
|
||||
const annotations = ref<Annotation[]>([])
|
||||
const loadError = ref<string | null>(null)
|
||||
@@ -33,16 +39,21 @@ export function useAnnotations() {
|
||||
return created
|
||||
}
|
||||
|
||||
async function updateNote(bookId: number, id: number, note: string | null) {
|
||||
async function update(bookId: number, id: number, data: AnnotationPatch): Promise<Annotation | null> {
|
||||
const res = await api(`/api/v1/books/${bookId}/annotations/${id}`, {
|
||||
method: 'PATCH',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ note }),
|
||||
body: JSON.stringify(data),
|
||||
})
|
||||
if (res.ok) {
|
||||
const updated: Annotation = await res.json()
|
||||
annotations.value = annotations.value.map((a) => (a.id === id ? updated : a))
|
||||
}
|
||||
if (!res.ok) return null
|
||||
|
||||
const updated: Annotation = await res.json()
|
||||
annotations.value = annotations.value.map((a) => (a.id === id ? updated : a))
|
||||
return updated
|
||||
}
|
||||
|
||||
function updateNote(bookId: number, id: number, note: string | null): Promise<Annotation | null> {
|
||||
return update(bookId, id, { note })
|
||||
}
|
||||
|
||||
async function remove(bookId: number, id: number) {
|
||||
@@ -54,5 +65,5 @@ export function useAnnotations() {
|
||||
}
|
||||
}
|
||||
|
||||
return { annotations, loadError, load, create, updateNote, remove }
|
||||
return { annotations, loadError, load, create, update, updateNote, remove }
|
||||
}
|
||||
|
||||
@@ -160,6 +160,8 @@ export function useFoliate(
|
||||
const detail = (e as CustomEvent).detail
|
||||
if (!detail?.value || !onAnnotationClick) return
|
||||
|
||||
input.suppressNextTapNavigation()
|
||||
|
||||
const range = detail.range as Range | undefined
|
||||
let x = window.innerWidth / 2
|
||||
let selectionTop = 0
|
||||
@@ -364,6 +366,7 @@ export function useFoliate(
|
||||
addAnnotation: (cfi: string, color = '#FACC15', style = 'highlight') => annotations.addAnnotation(viewRef.value, cfi, color, style),
|
||||
addAnnotations: (anns: { cfi: string; color: string; style: string }[]) => annotations.addAnnotations(viewRef.value, anns),
|
||||
deleteAnnotation: (cfi: string) => annotations.deleteAnnotation(viewRef.value, cfi),
|
||||
redrawAnnotation: (cfi: string, color: string, style: string) => annotations.redrawAnnotation(viewRef.value, cfi, color, style),
|
||||
setTextSelectedHandler: selection.setHandler,
|
||||
setAnnotationClickHandler,
|
||||
}
|
||||
|
||||
@@ -111,6 +111,14 @@ export function useFoliateAnnotations() {
|
||||
;(view as any)?.deleteAnnotation?.({ value: cfi })
|
||||
}
|
||||
|
||||
function redrawAnnotation(view: unknown, cfi: string, color: string, style: string) {
|
||||
annotationStyleMap.set(cfi, { color, style })
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
;(view as any)?.deleteAnnotation?.({ value: cfi })
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
;(view as any)?.addAnnotation?.({ value: cfi })
|
||||
}
|
||||
|
||||
function reAddAll(view: unknown) {
|
||||
setTimeout(() => {
|
||||
for (const [cfi] of annotationStyleMap) {
|
||||
@@ -128,5 +136,14 @@ export function useFoliateAnnotations() {
|
||||
draw(getDrawFunction(stored.style), { color: stored.color })
|
||||
}
|
||||
|
||||
return { annotationStyleMap, getDrawFunction, addAnnotation, addAnnotations, deleteAnnotation, reAddAll, handleDrawAnnotationEvent }
|
||||
return {
|
||||
annotationStyleMap,
|
||||
getDrawFunction,
|
||||
addAnnotation,
|
||||
addAnnotations,
|
||||
deleteAnnotation,
|
||||
redrawAnnotation,
|
||||
reAddAll,
|
||||
handleDrawAnnotationEvent,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
const LEFT_ZONE = 0.3
|
||||
const RIGHT_ZONE = 0.7
|
||||
const DOUBLE_CLICK_MS = 300
|
||||
const ANNOTATION_CLICK_SUPPRESSION_MS = DOUBLE_CLICK_MS + 100
|
||||
const SWIPE_THRESHOLD = 50
|
||||
|
||||
export function useFoliateInput(
|
||||
@@ -14,6 +15,7 @@ export function useFoliateInput(
|
||||
let lastClickTime = 0
|
||||
let lastClickZone: 'left' | 'middle' | 'right' | null = null
|
||||
let isNavigating = false
|
||||
let suppressClickNavigationUntil = 0
|
||||
let touchStartX = 0
|
||||
let touchStartY = 0
|
||||
let touchStartTime = 0
|
||||
@@ -51,6 +53,16 @@ export function useFoliateInput(
|
||||
getViewEl()?.next?.()
|
||||
}
|
||||
|
||||
function suppressNextTapNavigation() {
|
||||
suppressClickNavigationUntil = Date.now() + ANNOTATION_CLICK_SUPPRESSION_MS
|
||||
lastClickTime = 0
|
||||
lastClickZone = null
|
||||
}
|
||||
|
||||
function isTapNavigationSuppressed() {
|
||||
return Date.now() < suppressClickNavigationUntil
|
||||
}
|
||||
|
||||
function handleTouchStart(e: TouchEvent) {
|
||||
if (e.touches.length !== 1) return
|
||||
const touch = e.touches[0]!
|
||||
@@ -183,6 +195,7 @@ export function useFoliateInput(
|
||||
function handleWindowMessage(e: MessageEvent) {
|
||||
if (e.origin !== window.location.origin) return
|
||||
if (e.data?.type !== 'foliate-click') return
|
||||
if (isTapNavigationSuppressed()) return
|
||||
const view = getViewEl()
|
||||
if (!view) return
|
||||
|
||||
@@ -213,6 +226,7 @@ export function useFoliateInput(
|
||||
lastClickZone = currentZone
|
||||
|
||||
setTimeout(() => {
|
||||
if (isTapNavigationSuppressed()) return
|
||||
if (Date.now() - lastClickTime < DOUBLE_CLICK_MS) return
|
||||
if (!longHoldTimeout) return
|
||||
if (isNavigating) return
|
||||
@@ -267,5 +281,5 @@ export function useFoliateInput(
|
||||
document.removeEventListener('keydown', handleKeydown)
|
||||
}
|
||||
|
||||
return { attachIframeClicks, cleanup }
|
||||
return { attachIframeClicks, suppressNextTapNavigation, cleanup }
|
||||
}
|
||||
|
||||
@@ -0,0 +1,104 @@
|
||||
import type { Ref } from 'vue'
|
||||
import type { Annotation, AnnotationPatch } from './useAnnotations'
|
||||
|
||||
interface ReaderAnnotations {
|
||||
annotations: Ref<Annotation[]>
|
||||
create: (
|
||||
bookId: number,
|
||||
data: { cfi: string; bookFileId?: number; text: string; color: string; style: string; note?: string | null; chapterTitle?: string | null },
|
||||
) => Promise<Annotation | null>
|
||||
update: (bookId: number, id: number, data: AnnotationPatch) => Promise<Annotation | null>
|
||||
}
|
||||
|
||||
interface ReaderSelection {
|
||||
text: Ref<string>
|
||||
cfi: Ref<string | null>
|
||||
overlappingAnnotationId: Ref<number | null>
|
||||
showNoteDialog: Ref<boolean>
|
||||
noteText: Ref<string>
|
||||
dismiss: () => void
|
||||
openNoteDialog: (initialNote?: string) => void
|
||||
}
|
||||
|
||||
interface ReaderAnnotationActionsOptions {
|
||||
bookId: number
|
||||
fileId: number
|
||||
chapterTitle: Ref<string | null | undefined>
|
||||
annotations: ReaderAnnotations
|
||||
selection: ReaderSelection
|
||||
addAnnotation: (cfi: string, color: string, style: string) => void
|
||||
redrawAnnotation: (cfi: string, color: string, style: string) => void
|
||||
}
|
||||
|
||||
const DEFAULT_NOTE_HIGHLIGHT_COLOR = '#FACC15'
|
||||
const DEFAULT_NOTE_HIGHLIGHT_STYLE = 'highlight'
|
||||
|
||||
export function useReaderAnnotationActions({
|
||||
bookId,
|
||||
fileId,
|
||||
chapterTitle,
|
||||
annotations,
|
||||
selection,
|
||||
addAnnotation,
|
||||
redrawAnnotation,
|
||||
}: ReaderAnnotationActionsOptions) {
|
||||
function selectedAnnotation(): Annotation | null {
|
||||
const id = selection.overlappingAnnotationId.value
|
||||
if (id === null) return null
|
||||
return annotations.annotations.value.find((annotation) => annotation.id === id) ?? null
|
||||
}
|
||||
|
||||
async function updateSelectedAnnotation(data: AnnotationPatch, redraw: boolean) {
|
||||
const id = selection.overlappingAnnotationId.value
|
||||
if (id === null) return null
|
||||
|
||||
const updated = await annotations.update(bookId, id, data)
|
||||
if (redraw && updated?.cfi) {
|
||||
redrawAnnotation(updated.cfi, updated.color, updated.style)
|
||||
}
|
||||
return updated
|
||||
}
|
||||
|
||||
async function handleHighlight(color: string, style: string, note?: string) {
|
||||
if (selection.overlappingAnnotationId.value !== null) {
|
||||
const patch: AnnotationPatch = { color, style }
|
||||
if (note !== undefined) patch.note = note
|
||||
await updateSelectedAnnotation(patch, true)
|
||||
selection.dismiss()
|
||||
return
|
||||
}
|
||||
|
||||
const annotationCfi = selection.cfi.value
|
||||
if (!selection.text.value || !annotationCfi) return
|
||||
|
||||
const created = await annotations.create(bookId, {
|
||||
cfi: annotationCfi,
|
||||
bookFileId: fileId,
|
||||
text: selection.text.value,
|
||||
color,
|
||||
style,
|
||||
note: note ?? null,
|
||||
chapterTitle: chapterTitle.value || null,
|
||||
})
|
||||
if (created?.cfi) {
|
||||
addAnnotation(created.cfi, created.color, created.style)
|
||||
}
|
||||
selection.dismiss()
|
||||
}
|
||||
|
||||
function handleOpenNoteDialog() {
|
||||
selection.openNoteDialog(selectedAnnotation()?.note ?? '')
|
||||
}
|
||||
|
||||
async function handleSaveNote(note: string) {
|
||||
if (selection.overlappingAnnotationId.value !== null) {
|
||||
await updateSelectedAnnotation({ note }, false)
|
||||
} else {
|
||||
await handleHighlight(DEFAULT_NOTE_HIGHLIGHT_COLOR, DEFAULT_NOTE_HIGHLIGHT_STYLE, note)
|
||||
}
|
||||
selection.showNoteDialog.value = false
|
||||
selection.noteText.value = ''
|
||||
}
|
||||
|
||||
return { handleHighlight, handleOpenNoteDialog, handleSaveNote }
|
||||
}
|
||||
@@ -24,8 +24,8 @@ export function useReaderSelection() {
|
||||
visible.value = false
|
||||
}
|
||||
|
||||
function openNoteDialog() {
|
||||
noteText.value = ''
|
||||
function openNoteDialog(initialNote = '') {
|
||||
noteText.value = initialNote
|
||||
showNoteDialog.value = true
|
||||
dismiss()
|
||||
}
|
||||
|
||||
@@ -126,3 +126,15 @@ export function cfiRangesOverlap(a: string, b: string): boolean {
|
||||
if (ra.spineStep !== rb.spineStep) return false
|
||||
return ra.startKey <= rb.endKey && ra.endKey >= rb.startKey
|
||||
}
|
||||
|
||||
export function cfiRangesMatch(a: string, b: string): boolean {
|
||||
const ra = parseCfiRange(a)
|
||||
const rb = parseCfiRange(b)
|
||||
if (!ra || !rb) return false
|
||||
return ra.spineStep === rb.spineStep && ra.startKey === rb.startKey && ra.endKey === rb.endKey
|
||||
}
|
||||
|
||||
export function findMatchingCfiRange<T extends { cfi: string | null | undefined }>(items: T[], selectedCfi: string | null | undefined): T | null {
|
||||
if (!selectedCfi) return null
|
||||
return items.find((item) => item.cfi != null && cfiRangesMatch(selectedCfi, item.cfi)) ?? null
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user