Commit a079e683 authored by Robert Knight's avatar Robert Knight

Avoid duplicated segment headings in VitalSource PDF books

In PDF-based VitalSource books, each page has a different CFI, but many pages
may belong to the same logical book chapter and thus share the same chapter
title. In the sidebar we only want to show one heading per chapter. Therefore
add logic in ThreadList to avoid showing the same heading multiple times in
succession.

Fixes https://github.com/hypothesis/client/issues/5064
parent 3251192d
......@@ -170,15 +170,29 @@ export default function ThreadList({ threads }: ThreadListProps) {
// ones, so the association doesn't change while scrolling.
const headings = useMemo(() => {
let prevHeadingKey: string | null = null;
let prevHeading: string | null = null;
const headingForKey = headingMap(threads);
const headings = new Map();
for (const thread of threads) {
const key = headingKey(thread);
if (key && prevHeadingKey !== key) {
prevHeadingKey = key;
headings.set(thread, headingForKey.get(key) ?? 'Untitled chapter');
if (!key || prevHeadingKey === key) {
continue;
}
prevHeadingKey = key;
const heading = headingForKey.get(key) ?? 'Untitled chapter';
if (prevHeading === heading) {
// In some books (eg. VitalSource PDF-based books) a single logical
// chapter gets split into many content documents and the heading is
// repeated for each of them. We only want to render distinct headings.
continue;
}
prevHeading = heading;
headings.set(thread, heading);
}
return headings;
}, [threads]);
......
......@@ -384,6 +384,20 @@ describe('ThreadList', () => {
assert.equal(getHeading(containers.at(2)), null);
assert.equal(getHeading(containers.at(3)), 'Untitled chapter');
});
it('does not render heading if same as previous chapter', () => {
// Add two annotations from different chapters/document segments, but
// with the same heading.
addThreadInChapter('/4', 'Chapter One');
addThreadInChapter('/6', 'Chapter One');
const wrapper = createComponent();
const containers = wrapper.find('[data-testid="thread-card-container"]');
assert.equal(containers.length, fakeTopThread.children.length);
assert.equal(getHeading(containers.at(0)), 'Chapter One');
assert.equal(getHeading(containers.at(1)), null);
});
});
it('does not error if thread heights cannot be measured', () => {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment