feat(annotations): unify highlights and annotations into one record (#3870, #4511) (#4647)

A highlight and its note are now a single BookNote. Adding a note attaches
it to the highlight at that CFI (or creates one with the current global
style) instead of creating a second record, and a unified record renders as
both a highlight overlay and a note bubble.

- onDrawAnnotation chooses the draw kind from the overlay value prefix
  (cfi -> highlight, NOTE_PREFIX -> bubble) instead of annotation.note, so a
  record with both a style and a note draws both. Fixes notes synced from
  KOReader losing their highlight (#4511).
- handleSaveNote updates the existing annotation at the CFI rather than
  pushing a new record (#3870); re-styling preserves the note.
- unifyAnnotations migration (book config schema v1 -> v2, run in
  deserializeConfig) collapses existing split highlight+note records into one
  survivor and tombstones the redundant record (deletedAt) so the merge syncs
  to the cloud and KOReader.
- Sidebar: a note's quoted highlight text uses the theme foreground so it
  stays legible on the highlight background.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Huang Xin
2026-06-18 21:37:46 +08:00
committed by GitHub
parent 38a6d3d9ba
commit ff96c6d3f7
10 changed files with 436 additions and 26 deletions
@@ -0,0 +1,82 @@
import { describe, expect, it } from 'vitest';
import {
decideAnnotationDraw,
findAnnotationAtCfi,
mergeRestyledAnnotation,
} from '@/app/reader/utils/annotatorUtil';
import { BookNote } from '@/types/book';
import { NOTE_PREFIX } from '@/types/view';
const makeNote = (over: Partial<BookNote>): BookNote => ({
id: 'id',
type: 'annotation',
cfi: 'epubcfi(/6/4!/4)',
note: '',
createdAt: 1,
updatedAt: 1,
...over,
});
describe('decideAnnotationDraw', () => {
it('returns bubble for a note-prefixed overlay value regardless of style', () => {
expect(decideAnnotationDraw(`${NOTE_PREFIX}epubcfi(/6/4!/4)`, 'highlight')).toBe('bubble');
expect(decideAnnotationDraw(`${NOTE_PREFIX}epubcfi(/6/4!/4)`, undefined)).toBe('bubble');
});
it('returns the style kind for a plain cfi overlay value', () => {
expect(decideAnnotationDraw('epubcfi(/6/4!/4)', 'highlight')).toBe('highlight');
expect(decideAnnotationDraw('epubcfi(/6/4!/4)', 'underline')).toBe('underline');
expect(decideAnnotationDraw('epubcfi(/6/4!/4)', 'squiggly')).toBe('squiggly');
});
it('returns none when there is no style and it is not a note overlay', () => {
expect(decideAnnotationDraw('epubcfi(/6/4!/4)', undefined)).toBe('none');
expect(decideAnnotationDraw(undefined, undefined)).toBe('none');
});
});
describe('findAnnotationAtCfi', () => {
it('finds the live annotation at the cfi', () => {
const notes = [makeNote({ id: 'a', cfi: 'X' }), makeNote({ id: 'b', cfi: 'Y' })];
expect(findAnnotationAtCfi(notes, 'Y')).toBe(1);
});
it('ignores deleted annotations and non-annotation types', () => {
const notes = [
makeNote({ id: 'a', cfi: 'X', deletedAt: 5 }),
makeNote({ id: 'b', cfi: 'X', type: 'bookmark' }),
];
expect(findAnnotationAtCfi(notes, 'X')).toBe(-1);
});
});
describe('mergeRestyledAnnotation', () => {
it('keeps the existing id, note, text, createdAt, and global while taking the new style/color', () => {
const existing = makeNote({
id: 'a',
style: 'highlight',
color: 'yellow',
note: 'hi',
text: 'word',
global: true,
createdAt: 100,
});
const restyled = makeNote({
id: 'tmp',
style: 'underline',
color: 'red',
note: '',
text: 'word',
createdAt: 200,
updatedAt: 200,
});
const merged = mergeRestyledAnnotation(existing, restyled);
expect(merged.id).toBe('a');
expect(merged.style).toBe('underline');
expect(merged.color).toBe('red');
expect(merged.note).toBe('hi');
expect(merged.global).toBe(true);
expect(merged.createdAt).toBe(100);
expect(merged.updatedAt).toBe(200);
});
});
@@ -0,0 +1,98 @@
import { describe, expect, it } from 'vitest';
import { unifyAnnotations } from '@/utils/booknoteMigration';
import { BookNote } from '@/types/book';
const note = (over: Partial<BookNote>): BookNote => ({
id: 'id',
type: 'annotation',
cfi: 'C',
note: '',
createdAt: 1,
updatedAt: 1,
...over,
});
describe('unifyAnnotations', () => {
it('merges a separate highlight and note at the same cfi into a survivor + a tombstone', () => {
const input = [
note({
id: 'h',
cfi: 'C1',
style: 'highlight',
color: 'yellow',
text: 'hello',
note: '',
createdAt: 10,
}),
note({ id: 'n', cfi: 'C1', text: 'hello', note: 'my note', createdAt: 20 }),
];
const out = unifyAnnotations(input);
const survivor = out.find((n) => n.id === 'h')!;
const tombstone = out.find((n) => n.id === 'n')!;
expect(survivor.style).toBe('highlight');
expect(survivor.note).toBe('my note');
expect(survivor.deletedAt).toBeFalsy();
expect(tombstone.deletedAt).toBeTruthy();
});
it('deterministically prefers the styled record as survivor', () => {
const input = [
note({ id: 'n', cfi: 'C', note: 'note', createdAt: 5 }),
note({ id: 'h', cfi: 'C', style: 'highlight', note: '', createdAt: 50 }),
];
const out = unifyAnnotations(input);
expect(out.find((n) => n.id === 'h')!.deletedAt).toBeFalsy();
expect(out.find((n) => n.id === 'n')!.deletedAt).toBeTruthy();
});
it('keeps the latest-updated non-empty note', () => {
const input = [
note({ id: 'h', cfi: 'C', style: 'highlight', note: 'old', updatedAt: 10, createdAt: 1 }),
note({ id: 'n', cfi: 'C', note: 'new', updatedAt: 99, createdAt: 2 }),
];
const out = unifyAnnotations(input);
expect(out.find((n) => n.id === 'h')!.note).toBe('new');
});
it('leaves a single annotation per cfi untouched (same reference)', () => {
const input = [note({ id: 'a', cfi: 'C', style: 'highlight', note: 'x' })];
expect(unifyAnnotations(input)).toBe(input);
});
it('is idempotent', () => {
const input = [
note({ id: 'h', cfi: 'C', style: 'highlight', note: '', createdAt: 10 }),
note({ id: 'n', cfi: 'C', note: 'note', createdAt: 20 }),
];
const once = unifyAnnotations(input);
const twice = unifyAnnotations(once);
expect(twice.filter((n) => !n.deletedAt).length).toBe(once.filter((n) => !n.deletedAt).length);
expect(twice.find((n) => n.id === 'h')!.note).toBe('note');
});
it('does not merge bookmarks, excerpts, or global highlights', () => {
const input = [
note({ id: 'b1', cfi: 'C', type: 'bookmark' }),
note({ id: 'b2', cfi: 'C', type: 'bookmark' }),
note({ id: 'e1', cfi: 'C', type: 'excerpt' }),
note({ id: 'e2', cfi: 'C', type: 'excerpt' }),
note({ id: 'g1', cfi: 'C2', style: 'highlight', global: true, note: '' }),
note({ id: 'g2', cfi: 'C2', style: 'highlight', global: true, note: '' }),
];
const out = unifyAnnotations(input);
expect(out.filter((n) => n.deletedAt).length).toBe(0);
});
it('collapses three records at one cfi to a single survivor + two tombstones', () => {
const input = [
note({ id: 'h', cfi: 'C', style: 'highlight', note: '', createdAt: 10 }),
note({ id: 'n1', cfi: 'C', note: 'first', updatedAt: 20, createdAt: 11 }),
note({ id: 'n2', cfi: 'C', note: 'second', updatedAt: 30, createdAt: 12 }),
];
const out = unifyAnnotations(input);
expect(out.filter((n) => !n.deletedAt).length).toBe(1);
const survivor = out.find((n) => !n.deletedAt)!;
expect(survivor.id).toBe('h');
expect(survivor.note).toBe('second');
});
});
@@ -70,4 +70,71 @@ describe('BookConfig serialization', () => {
expect(config.viewSettings?.zoomLevel).toBe(90);
expect(config.searchConfig?.query).toBe('rabbit');
});
it('migrates legacy split annotations on load when schemaVersion < 2', () => {
const config = deserializeConfig(
JSON.stringify({
updatedAt: 1,
booknotes: [
{
id: 'h',
type: 'annotation',
cfi: 'C1',
style: 'highlight',
color: 'yellow',
text: 't',
note: '',
createdAt: 10,
updatedAt: 10,
},
{
id: 'n',
type: 'annotation',
cfi: 'C1',
text: 't',
note: 'hello',
createdAt: 20,
updatedAt: 20,
},
],
}),
globalViewSettings,
defaultSearchConfig,
);
const survivor = config.booknotes!.find((n) => n.id === 'h')!;
const tombstone = config.booknotes!.find((n) => n.id === 'n')!;
expect(survivor.note).toBe('hello');
expect(survivor.deletedAt).toBeFalsy();
expect(tombstone.deletedAt).toBeTruthy();
});
it('does not migrate annotations when schemaVersion is already 2', () => {
const config = deserializeConfig(
JSON.stringify({
schemaVersion: 2,
booknotes: [
{
id: 'h',
type: 'annotation',
cfi: 'C1',
style: 'highlight',
note: '',
createdAt: 10,
updatedAt: 10,
},
{
id: 'n',
type: 'annotation',
cfi: 'C1',
note: 'hello',
createdAt: 20,
updatedAt: 20,
},
],
}),
globalViewSettings,
defaultSearchConfig,
);
expect(config.booknotes!.every((n) => !n.deletedAt)).toBe(true);
});
});
@@ -56,7 +56,9 @@ import { TransformContext } from '@/services/transformers/types';
import { transformContent } from '@/services/transformService';
import {
buildTTSSentenceHighlight,
decideAnnotationDraw,
getHighlightColorHex,
mergeRestyledAnnotation,
removeBookNoteOverlays,
} from '../../utils/annotatorUtil';
import { buildAnnotationIndex, selectLocationAnnotations } from '../../utils/annotationIndex';
@@ -495,21 +497,27 @@ const Annotator: React.FC<{ bookKey: string; contentInsets: Insets }> = ({
const detail = (event as CustomEvent).detail;
const { draw, annotation, doc, range } = detail;
const { style, color } = annotation as BookNote;
const value = (annotation as BookNote & { value?: string }).value;
const hexColor = getHighlightColorHex(settings, color);
const einkBgColor = isDarkMode ? '#000000' : '#ffffff';
const einkFgColor = isDarkMode ? '#ffffff' : '#000000';
if (annotation.note) {
// Choose what to draw from the overlay's `value` (cfi vs NOTE_PREFIX+cfi),
// not from `annotation.note`: a unified record (style + note) is added as
// two overlays and must draw a highlight for the cfi overlay AND a bubble
// for the note overlay. Keying off `note` drew only the bubble (#4511).
const kind = decideAnnotationDraw(value, style);
if (kind === 'bubble') {
const { defaultView } = doc;
const node = range.startContainer;
const el = node.nodeType === 1 ? node : node.parentElement;
const { writingMode } = defaultView.getComputedStyle(el);
draw(Overlayer.bubble, { writingMode });
} else if (style === 'highlight') {
} else if (kind === 'highlight') {
draw(Overlayer.highlight, {
color: isBwEink ? einkBgColor : hexColor,
vertical: viewSettings.vertical,
});
} else if (['underline', 'squiggly'].includes(style as string)) {
} else if (kind === 'underline' || kind === 'squiggly') {
const { defaultView } = doc;
const node = range.startContainer;
const el = node.nodeType === 1 ? node : node.parentElement;
@@ -522,7 +530,7 @@ const Annotator: React.FC<{ bookKey: string; contentInsets: Insets }> = ({
const padding = viewSettings.vertical
? (lineHeightValue - fontSizeValue) / 2 - strokeWidth + verticalCompensation
: (lineHeightValue - fontSizeValue) / 2 - strokeWidth + horizontalCompensation;
draw(Overlayer[style as keyof typeof Overlayer], {
draw(Overlayer[kind], {
writingMode,
color: isBwEink ? einkFgColor : hexColor,
padding,
@@ -1089,15 +1097,17 @@ const Annotator: React.FC<{ bookKey: string; contentInsets: Insets }> = ({
views.forEach((view) => removeGlobalAnnotationOverlays(view, existing));
}
if (update) {
annotation.id = existing.id;
// Carry the existing `global` flag forward — toggling color/style
// shouldn't silently demote a global highlight back to single-range.
if (existing.global) annotation.global = true;
annotations[existingIndex] = annotation;
views.forEach((view) => view?.addAnnotation(annotation));
if (annotation.global) {
// Preserve the note/text/createdAt and the `global` flag of the existing
// record so a restyle (color/style change) of a unified annotation
// doesn't wipe its note or silently demote a global highlight. The note
// bubble overlay (NOTE_PREFIX) isn't torn down above, so it persists; we
// only redraw the highlight overlay (value = cfi).
const merged = mergeRestyledAnnotation(existing, annotation);
annotations[existingIndex] = merged;
views.forEach((view) => view?.addAnnotation(merged));
if (merged.global) {
views.forEach((view) => {
if (view) expandAllRenderedSections(view, annotation);
if (view) expandAllRenderedSections(view, merged);
});
}
} else {
@@ -23,6 +23,7 @@ import { Overlay } from '@/components/Overlay';
import { saveSysSettings } from '@/helpers/settings';
import { NOTE_PREFIX } from '@/types/view';
import useShortcuts from '@/hooks/useShortcuts';
import { findAnnotationAtCfi } from '../../utils/annotatorUtil';
import BooknoteItem from '../sidebar/BooknoteItem';
import AIAssistant from './AIAssistant';
import NotebookHeader from './Header';
@@ -154,18 +155,42 @@ const Notebook: React.FC = ({}) => {
if (!cfi) return;
const { booknotes: annotations = [] } = config;
const annotation: BookNote = {
id: uniqueId(),
type: 'annotation',
cfi,
note,
page: selection.page,
text: selection.text,
createdAt: Date.now(),
updatedAt: Date.now(),
};
view?.addAnnotation({ ...annotation, value: `${NOTE_PREFIX}${annotation.cfi}` });
annotations.push(annotation);
const existingIndex = findAnnotationAtCfi(annotations, cfi);
if (existingIndex !== -1) {
// Attach the note to the existing highlight at this CFI instead of
// creating a second record. The highlight overlay (value = cfi) already
// exists; add the note bubble overlay (value = NOTE_PREFIX+cfi).
const existing = annotations[existingIndex]!;
const updated: BookNote = {
...existing,
note,
text: selection.text || existing.text,
updatedAt: Date.now(),
};
annotations[existingIndex] = updated;
view?.addAnnotation({ ...updated, value: `${NOTE_PREFIX}${updated.cfi}` });
} else {
// No highlight at this CFI yet (e.g. a note added without first
// highlighting): create one unified record with the current global style
// so the note still shows an underlying highlight, and draw both overlays.
const style = settings.globalReadSettings.highlightStyle;
const color = settings.globalReadSettings.highlightStyles[style];
const annotation: BookNote = {
id: uniqueId(),
type: 'annotation',
cfi,
style,
color,
note,
page: selection.page,
text: selection.text,
createdAt: Date.now(),
updatedAt: Date.now(),
};
view?.addAnnotation(annotation);
view?.addAnnotation({ ...annotation, value: `${NOTE_PREFIX}${annotation.cfi}` });
annotations.push(annotation);
}
const updatedConfig = updateBooknotes(sideBarBookKey, annotations);
if (updatedConfig) {
saveConfig(envConfig, sideBarBookKey, updatedConfig, settings);
@@ -199,7 +199,7 @@ const BooknoteItem: React.FC<BooknoteItemProps> = ({ bookKey, item, isNearest, o
<span
className={clsx(
'booknote-text inline leading-normal',
item.note && 'content font-size-xs text-gray-500',
item.note && 'content font-size-xs text-base-content',
(item.style === 'underline' || item.style === 'squiggly') &&
'underline decoration-2',
item.style === 'highlight' && 'rounded-[4px] px-[2px] py-[1px]',
@@ -229,3 +229,50 @@ export function buildTTSSentenceHighlight(
...params,
};
}
export type AnnotationDrawKind = 'bubble' | 'highlight' | 'underline' | 'squiggly' | 'none';
/**
* Decide what an overlay should draw for an annotation. The bubble vs.
* highlight choice keys off the overlay's `value` prefix — NOT `annotation.note`
* — so a single unified record draws BOTH its highlight overlay (value = cfi)
* and its note bubble (value = `${NOTE_PREFIX}${cfi}`).
*/
export function decideAnnotationDraw(
value: string | undefined,
style: HighlightStyle | undefined,
): AnnotationDrawKind {
if (value?.startsWith(NOTE_PREFIX)) return 'bubble';
if (style === 'highlight') return 'highlight';
if (style === 'underline' || style === 'squiggly') return style;
return 'none';
}
/**
* Index of the live (`!deletedAt`) annotation record at `cfi`, or -1. Used when
* adding a note so it attaches to the existing highlight instead of creating a
* second record at the same position.
*/
export function findAnnotationAtCfi(booknotes: BookNote[], cfi: string): number {
return booknotes.findIndex(
(note) => note.type === 'annotation' && note.cfi === cfi && !note.deletedAt,
);
}
/**
* Merge a freshly-built restyle (`restyled`, carrying the new style/color) onto
* an `existing` annotation, preserving the parts a restyle must not lose: the
* record id, its note text, the selected text, the original creation time, and
* the `global` flag. Without preserving `note`, recoloring a unified annotation
* would wipe the note.
*/
export function mergeRestyledAnnotation(existing: BookNote, restyled: BookNote): BookNote {
return {
...restyled,
id: existing.id,
createdAt: existing.createdAt,
note: existing.note,
text: existing.text ?? restyled.text,
global: existing.global || restyled.global,
};
}
+1 -1
View File
@@ -434,7 +434,7 @@ export interface BookSearchResult {
progress?: number;
}
export const BOOK_CONFIG_SCHEMA_VERSION = 1;
export const BOOK_CONFIG_SCHEMA_VERSION = 2;
export interface BookConfig {
schemaVersion?: number;
@@ -0,0 +1,75 @@
import { BookNote } from '@/types/book';
/**
* Schema v1 → v2 migration: collapse a highlight and its note that were stored
* as two separate `type:'annotation'` records at the same CFI into one unified
* record, tombstoning the redundant record(s) so the deletion propagates to the
* cloud and KOReader.
*
* The survivor is chosen deterministically (prefer a record with a `style`, then
* earliest `createdAt`, then smallest `id`) so independent devices converge under
* the server's last-writer-wins merge. The latest-updated non-empty note wins.
*
* Idempotent: a CFI that already has a single live record is left untouched, and
* a re-run after migration finds nothing to merge. Bookmarks, excerpts, and
* `global` highlights are never touched. Returns the same array reference when
* nothing changed.
*/
export function unifyAnnotations(booknotes: BookNote[]): BookNote[] {
const groups = new Map<string, BookNote[]>();
for (const note of booknotes) {
if (note.type !== 'annotation') continue;
if (note.deletedAt) continue;
if (note.global) continue;
if (!note.cfi) continue;
const bucket = groups.get(note.cfi);
if (bucket) bucket.push(note);
else groups.set(note.cfi, [note]);
}
const survivorById = new Map<string, BookNote>();
const tombstonedIds = new Set<string>();
let changed = false;
const now = Date.now();
for (const group of groups.values()) {
if (group.length < 2) continue;
changed = true;
const sorted = [...group].sort((a, b) => {
const aStyled = a.style ? 0 : 1;
const bStyled = b.style ? 0 : 1;
if (aStyled !== bStyled) return aStyled - bStyled;
const aCreated = a.createdAt ?? 0;
const bCreated = b.createdAt ?? 0;
if (aCreated !== bCreated) return aCreated - bCreated;
return a.id < b.id ? -1 : a.id > b.id ? 1 : 0;
});
const survivor = sorted[0]!;
let note = survivor.note ?? '';
let noteUpdatedAt = note.trim().length > 0 ? (survivor.updatedAt ?? 0) : -1;
for (const member of group) {
const memberNote = member.note ?? '';
if (memberNote.trim().length === 0) continue;
const memberUpdatedAt = member.updatedAt ?? 0;
if (memberUpdatedAt > noteUpdatedAt) {
note = memberNote;
noteUpdatedAt = memberUpdatedAt;
}
}
survivorById.set(survivor.id, { ...survivor, note, updatedAt: now });
for (const member of group) {
if (member.id !== survivor.id) tombstonedIds.add(member.id);
}
}
if (!changed) return booknotes;
return booknotes.map((note) => {
if (survivorById.has(note.id)) return survivorById.get(note.id)!;
if (tombstonedIds.has(note.id)) return { ...note, deletedAt: now, updatedAt: now };
return note;
});
}
+6
View File
@@ -4,6 +4,7 @@ import {
BookSearchConfig,
ViewSettings,
} from '@/types/book';
import { unifyAnnotations } from '@/utils/booknoteMigration';
export const stampBookConfigSchema = <T extends Partial<BookConfig>>(config: T): T => {
return { ...config, schemaVersion: BOOK_CONFIG_SCHEMA_VERSION };
@@ -64,6 +65,11 @@ export const deserializeConfig = (
const { viewSettings, searchConfig } = config;
config.viewSettings = { ...globalViewSettings, ...viewSettings };
config.searchConfig = { ...defaultSearchConfig, ...searchConfig };
// v1 -> v2: collapse split highlight+note records into one unified record so a
// note renders with its highlight and round-trips cleanly to KOReader.
if ((config.schemaVersion ?? 0) < 2 && config.booknotes?.length) {
config.booknotes = unifyAnnotations(config.booknotes);
}
config.schemaVersion ??= BOOK_CONFIG_SCHEMA_VERSION;
config.updatedAt ??= Date.now();
return config;