fix: clear highlight overlay when deleting annotation from sidebar, closes #3756 (#3758)

The sidebar delete handler always removed annotations through the note
bubble overlay key (`NOTE_PREFIX + cfi`), but highlights are keyed by
the raw CFI. Deleting a highlight from the sidebar therefore left its
overlay drawn until the book was reopened, while the popup path — which
lets `wrappedFoliateView` default the value to the CFI — worked.

Introduce `removeBookNoteOverlays` that mirrors the draw filters in
`Annotator.tsx` and clears every overlay a BookNote can own (highlight
and/or note bubble), and route the sidebar delete through it.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Huang Xin
2026-04-05 22:25:58 +08:00
committed by GitHub
parent 7e62516b5d
commit a2d17e6a79
3 changed files with 113 additions and 6 deletions
@@ -2,11 +2,13 @@ import { describe, it, expect, vi } from 'vitest';
import {
getExternalDragHandle,
getHighlightColorLabel,
removeBookNoteOverlays,
toParentViewportPoint,
} from '@/app/reader/utils/annotatorUtil';
import { Point } from '@/utils/sel';
import { UserHighlightColor } from '@/types/book';
import { BookNote, UserHighlightColor } from '@/types/book';
import { SystemSettings } from '@/types/settings';
import { FoliateView, NOTE_PREFIX } from '@/types/view';
describe('getExternalDragHandle', () => {
const currentStart: Point = { x: 100, y: 200 };
@@ -140,3 +142,86 @@ describe('getHighlightColorLabel', () => {
expect(getHighlightColorLabel(settings, 'red')).toBeUndefined();
});
});
describe('removeBookNoteOverlays', () => {
const makeView = () => {
const addAnnotation = vi.fn();
const view = { addAnnotation } as unknown as FoliateView;
return { view, addAnnotation };
};
const baseNote = (overrides: Partial<BookNote> = {}): BookNote => ({
id: 'id-1',
type: 'annotation',
cfi: 'epubcfi(/6/4!/4/2)',
note: '',
createdAt: 1,
updatedAt: 1,
...overrides,
});
it('removes only the highlight overlay for a highlight-only annotation', () => {
const { view, addAnnotation } = makeView();
const note = baseNote({ style: 'highlight', color: 'yellow' });
removeBookNoteOverlays(view, note);
expect(addAnnotation).toHaveBeenCalledTimes(1);
expect(addAnnotation).toHaveBeenCalledWith(expect.objectContaining({ value: note.cfi }), true);
const passed = addAnnotation.mock.calls[0]![0] as BookNote & { value: string };
expect(passed.value.startsWith(NOTE_PREFIX)).toBe(false);
});
it('removes only the note overlay for a note-only annotation', () => {
const { view, addAnnotation } = makeView();
const note = baseNote({ note: 'my comment' });
removeBookNoteOverlays(view, note);
expect(addAnnotation).toHaveBeenCalledTimes(1);
expect(addAnnotation).toHaveBeenCalledWith(
expect.objectContaining({ value: `${NOTE_PREFIX}${note.cfi}` }),
true,
);
});
it('removes both overlays when the annotation has a highlight and a note', () => {
const { view, addAnnotation } = makeView();
const note = baseNote({ style: 'underline', color: 'red', note: 'my comment' });
removeBookNoteOverlays(view, note);
expect(addAnnotation).toHaveBeenCalledTimes(2);
const values = addAnnotation.mock.calls.map(
(call) => (call[0] as BookNote & { value: string }).value,
);
expect(values).toContain(note.cfi);
expect(values).toContain(`${NOTE_PREFIX}${note.cfi}`);
for (const call of addAnnotation.mock.calls) {
expect(call[1]).toBe(true);
}
});
it('does nothing for a bookmark (no highlight, no note text)', () => {
const { view, addAnnotation } = makeView();
const bookmark = baseNote({ type: 'bookmark' });
removeBookNoteOverlays(view, bookmark);
expect(addAnnotation).not.toHaveBeenCalled();
});
it('treats whitespace-only note text as empty and skips the note overlay', () => {
const { view, addAnnotation } = makeView();
const note = baseNote({ style: 'highlight', note: ' \n ' });
removeBookNoteOverlays(view, note);
expect(addAnnotation).toHaveBeenCalledTimes(1);
expect(addAnnotation).toHaveBeenCalledWith(expect.objectContaining({ value: note.cfi }), true);
});
it('is a no-op when view is null', () => {
expect(() => removeBookNoteOverlays(null, baseNote({ style: 'highlight' }))).not.toThrow();
});
});
@@ -13,7 +13,7 @@ import { useBookDataStore } from '@/store/bookDataStore';
import { useTranslation } from '@/hooks/useTranslation';
import { useResponsiveSize } from '@/hooks/useResponsiveSize';
import { eventDispatcher } from '@/utils/event';
import { NOTE_PREFIX } from '@/types/view';
import { removeBookNoteOverlays } from '../../utils/annotatorUtil';
import useScrollToItem from '../../hooks/useScrollToItem';
import TextButton from '@/components/TextButton';
import TextEditor, { TextEditorRef } from '@/components/TextEditor';
@@ -66,9 +66,7 @@ const BooknoteItem: React.FC<BooknoteItemProps> = ({ bookKey, item, isNearest, o
if (item.id === note.id) {
item.deletedAt = Date.now();
const views = getViewsById(bookKey.split('-')[0]!);
views.forEach((view) =>
view?.addAnnotation({ ...item, value: `${NOTE_PREFIX}${item.cfi}` }, true),
);
views.forEach((view) => removeBookNoteOverlays(view, item));
}
});
const updatedConfig = updateBooknotes(bookKey, booknotes);
@@ -1,6 +1,7 @@
import { HIGHLIGHT_COLOR_HEX } from '@/services/constants';
import { DEFAULT_HIGHLIGHT_COLORS, HighlightColor } from '@/types/book';
import { BookNote, DEFAULT_HIGHLIGHT_COLORS, HighlightColor } from '@/types/book';
import { SystemSettings } from '@/types/settings';
import { FoliateView, NOTE_PREFIX } from '@/types/view';
import { Point } from '@/utils/sel';
export const isDefaultHighlightColor = (
@@ -62,3 +63,26 @@ export function toParentViewportPoint(doc: Document, x: number, y: number): Poin
const frameRect = frameElement?.getBoundingClientRect() ?? { top: 0, left: 0 };
return { x: x + frameRect.left, y: y + frameRect.top };
}
/**
* Remove any overlays drawn for a BookNote from the given view.
*
* A single BookNote can have up to two overlays attached:
* - a highlight/underline/squiggly overlay (keyed by the raw CFI)
* - a note bubble overlay (keyed by `${NOTE_PREFIX}${cfi}`)
*
* The set of overlays drawn is defined by the progress-sync effect in
* Annotator.tsx, and this helper mirrors those filters so that deleting
* an annotation from the sidebar clears every overlay that was drawn
* for it, not just the note bubble.
*/
export function removeBookNoteOverlays(view: FoliateView | null, note: BookNote): void {
if (!view) return;
if (note.type !== 'annotation') return;
if (note.style) {
view.addAnnotation({ ...note, value: note.cfi }, true);
}
if (note.note && note.note.trim().length > 0) {
view.addAnnotation({ ...note, value: `${NOTE_PREFIX}${note.cfi}` }, true);
}
}