Skip to content

Switch "Copy Link to Highlight" to use custom highlight rendering + beginnings of annotations support#1548

Merged
cdrini merged 23 commits into
masterfrom
fixes/copy-link-to-highlight
May 29, 2026
Merged

Switch "Copy Link to Highlight" to use custom highlight rendering + beginnings of annotations support#1548
cdrini merged 23 commits into
masterfrom
fixes/copy-link-to-highlight

Conversation

@cdrini
Copy link
Copy Markdown
Contributor

@cdrini cdrini commented May 20, 2026

Various code review refactors / fixes to #1538 . Created a separate PR so that we can keep the original for any before/after testing we might want to do.

Known issues:

@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

❌ Patch coverage is 17.08543% with 330 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.80%. Comparing base (452800b) to head (02b848c).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/util/TextSelectionManager.js 16.00% 308 Missing and 7 partials ⚠️
src/plugins/plugin.text_selection.js 23.07% 10 Missing ⚠️
src/BookReader.js 33.33% 2 Missing ⚠️
src/plugins/url/plugin.url.js 60.00% 2 Missing ⚠️
src/plugins/url/UrlPlugin.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1548      +/-   ##
==========================================
- Coverage   65.79%   62.80%   -3.00%     
==========================================
  Files          67       67              
  Lines        5853     6108     +255     
  Branches     1276     1333      +57     
==========================================
- Hits         3851     3836      -15     
- Misses       1968     2231     +263     
- Partials       34       41       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors “Copy Link to Highlight” away from native browser text fragments toward a BookReader-specific text fragment format and custom highlight rendering, and introduces early scaffolding for private (localStorage-backed) highlights/annotations.

Changes:

  • Introduces BookReaderTextFragment serialization/parsing plus DOM-based highlight rendering (renderHighlight) and localStorage-backed saved highlights.
  • Updates URL handling to parse ?text= into a BookReaderTextFragment and render the target highlight when the relevant page becomes visible.
  • Adds/adjusts experiments + selection UI and reorganizes/moves tests around the new utilities.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
src/util/TextSelectionManager.js Adds BookReaderTextFragment, highlight rendering, BRSelectMenu changes, and localStorage highlight support.
src/plugins/url/plugin.url.js Parses ?text= into targetTextFragment and renders target highlight on textLayerVisible.
src/plugins/url/UrlPlugin.js Simplifies getHash() now that text fragments are no longer appended into the hash.
src/BookReader.js Attempts to preserve text query param handling via BookReaderTextFragment during querystring rebuilds.
src/plugins/plugin.text_selection.js Adds an API to enable the highlight/annotation menu.
src/plugins/plugin.experiments.js Updates copy-link experiment metadata and adds an “annotateHighlight” experiment toggle.
src/css/_TextSelection.scss Adds BR highlight styling and prevents hiding text layers containing highlights.
src/css/_BRpages.scss Prevents hiding highlighted text layers during page flip animations.
tests/jest/util/TextSelectionManager.test.js Moves walkBetweenNodes tests here and attempts to adapt to new fragment logic (currently skips fragment tests).
tests/jest/plugins/plugin.text_selection.test.js Removes walkBetweenNodes tests now moved to util tests.
tests/jest/plugins/url/plugin.url.test.js Updates mock URL plugin surface (adds parseToText).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/util/TextSelectionManager.js Outdated
Comment thread src/util/TextSelectionManager.js Outdated
Comment thread src/util/TextSelectionManager.js Outdated
Comment thread src/util/TextSelectionManager.js Outdated
Comment thread src/util/TextSelectionManager.js
Comment thread src/plugins/url/plugin.url.js Outdated
Comment thread src/util/TextSelectionManager.js
Comment thread src/util/TextSelectionManager.js
Comment thread src/util/TextSelectionManager.js
Comment thread tests/jest/util/TextSelectionManager.test.js Outdated
cdrini added 2 commits May 27, 2026 15:08
- Rename SelectionObserver event from focusChanged to changed
- Decouple copy link to higlight and annotate experiments; either can be enabled for the select menu to appear
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Comment thread src/BookReader.js
Comment thread src/util/TextSelectionManager.js
Comment thread src/util/TextSelectionManager.js Outdated
Comment thread src/util/TextSelectionManager.js
Comment thread tests/jest/util/TextSelectionManager.test.js Outdated
cdrini added 2 commits May 27, 2026 22:00
- Rename createTextFragment to BookReaderTextFragment.fromSelection
- Add "Copied" confirmation text to CopyLinkToHighlight
- Simplify text fragment parsing to use a single regex
@cdrini cdrini marked this pull request as ready for review May 29, 2026 14:32
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

@cdrini cdrini merged commit 7b987e9 into master May 29, 2026
12 of 14 checks passed
@cdrini cdrini deleted the fixes/copy-link-to-highlight branch May 29, 2026 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants