Skip to content
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

Upgrade to Apache Annotator #18

Open
BigBlueHat opened this issue Apr 29, 2020 · 1 comment
Open

Upgrade to Apache Annotator #18

BigBlueHat opened this issue Apr 29, 2020 · 1 comment

Comments

@BigBlueHat
Copy link
Owner

@Treora and/or @tilgovi would love your thoughts on this upgrade/move.

The current selector creation code uses the dom-anchor- libraries directly:
https://github.com/BigBlueHat/page-notes/blob/master/src/annotate.js#L9-L17

The highlight code currently only uses the dom-anchor-text-position toRange stuff:
https://github.com/BigBlueHat/page-notes/blob/master/src/highlighter.js#L29

It should be a simple refactor to move to Apache Annotator, but I'm also wondering what we might learn for informing Apache Annotator's code (or "pitch") for existing projects using the dom-anchor- libraries directly.

@Treora
Copy link

Treora commented May 3, 2020

This indeed looks like an occasion to taste your own dog food. :)

In such a case I like to try refactor the code to make it look the way I feel it should be; then if that mismatches what is actually provided, update the library to make it work. (or tweak both to settle in between, and perhaps add library documentation to explain why it is not following the intuition you had as a consumer).

As of yet, Annotator lacks support for TextPosition selectors, but this may be a good moment to add it; I opened an issue for that.

For TextQuote selectors, you could swap in Annotator’s implementation, at the cost of losing the fuzzy string matching feature until we get that implemented.

I see your highlighter uses the wrap-range-text; this module does nearly exactly the same as Annotator’s highlighter (in fact, if I knew about that module, I might not even have written ours!). Should be easy to swap out.

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

No branches or pull requests

2 participants