-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add apple books highlight links #24
feat: add apple books highlight links #24
Conversation
* test(db): update migrations to enable new database tests * test: add basic highlight links tests for db * test: update mock tests, plugin docs and info * refactor(core): update default template to include highlight location * refactor(core): update types, constants, schemas, seedData and methods * fix: template for colored highlights. register handlebars custom helpers. Minor grammar fixes * docs(README): update preview and template-colors screenshots to show highlight links feature * docs(README): add highlight location template variable * docs(README): update template with highlight link * linter: update tsconfig to remove unnecessary type checks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@absorpheus Thank you for the contribution, the PR looks great.
However, I see multiple updates that you're trying to deliver:
- highlights link feature + tests and migrations
- fix for Issue with Template Import #25 (I will take care of that one, it needs tests to be fixed as well)
- fix for grammar
I would recommend you to split this PR into two, each one containing the corresponding changes. Please use this repository directly (not a fork) to make those changes, because the required checks weren't triggered for some reason for the PR. I will investigate why.
Thank you.
@@ -66,6 +66,7 @@ The plugin uses Handlebars and Markdown to customize the output of your highligh | |||
- If you highlight parts of two adjacent sentences, the `contextualText` will contain both sentences. | |||
- `{{{highlight}}}` - The highlighted text. | |||
- `{{{note}}}` - A note you added for the highlight. | |||
- `{{{highlightLocation}}}` - The EPUB CFI of the highlighted text. It is used to create a link to the highlighted text in Apple Books: `[Apple Books Highlight Link](ibooks://assetid/{{bookId}}#{{highlightLocation}})`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- `{{{highlightLocation}}}` - The EPUB CFI of the highlighted text. It is used to create a link to the highlighted text in Apple Books: `[Apple Books Highlight Link](ibooks://assetid/{{bookId}}#{{highlightLocation}})`. | |
- `{{{highlightLocation}}}` - a unique identifier of the highlighted text. It is used to create a link to the highlighted text in Apple Books. For example: `[Apple Books Highlight Link](ibooks://assetid/{{bookId}}#{{highlightLocation}})`. |
@@ -132,6 +133,7 @@ Number of annotations:: {{annotations.length}} | |||
{{#if (eq highlightStyle "4")}}- 🎯 Highlight:: <mark style="background:rgb(242,178,188); color:#000; padding:2px;">{{{highlight}}}</mark>{{/if}} | |||
{{#if (eq highlightStyle "5")}}- 🎯 Highlight:: <mark style="background:rgb(214,192,238); color:#000; padding:2px;">{{{highlight}}}</mark>{{/if}} | |||
- 📝 Note:: {{#if note}}{{{note}}}{{else}}N/A{{/if}} | |||
- 📙 Highlight Link:: {{#if highlightLocation}}[Apple Books Highlight Link](ibooks://assetid/{{../bookId}}#{{highlightLocation}}){{else}}N/A{{/if}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it's not necessary to add a highlight link to this template, as its main focus is on colors.
- 📙 Highlight Link:: {{#if highlightLocation}}[Apple Books Highlight Link](ibooks://assetid/{{../bookId}}#{{highlightLocation}}){{else}}N/A{{/if}} |
template-colors.png
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it's not necessary to add a highlight link to this template, as its main focus is on colors. So you can revert to the original screenshot.
@bandantonio Thank you for your feedback on the PR. I will work on making these changes very soon. |
@absorpheus No worries, I just released a new version to fix the bug with custom templates, in order to prevent user experience disruption. So there is no rush, take your time, please. |
@bandantonio Please give me permission to access the repository so I can push the feature branches and create the PR's. Thank you |
5f627f2
to
e5b071a
Compare
This PR was split into several ones:
|
What is it?
This feature will fetch the EPUB Canonical Fragment Identifier for highlights so that they can be opened at the exact highlight location with Apple Books.
Summary
test(db): update migrations to enable new database tests
test: add basic highlight links tests for db
test: update mock tests, plugin docs and info
refactor(core): update default template to include highlight location
refactor(core): update types, constants, schemas, seedData and methods
fix: template for colored highlights. register handlebars custom helpers. Minor grammar fixes
fix: new lines no longer appear at the end of highlight blocks
docs(README): update preview and template-colors screenshots to show highlight links feature
docs(README): add highlight location template variable
docs(README): update template with highlight link
linter: update tsconfig to remove unnecessary type checks