-
Notifications
You must be signed in to change notification settings - Fork 164
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] export annotation set [Round 2] #2704
Conversation
…ously used to import from main process without virtualDom
…the state root level. Now not specific to the search, search rely on this resourceCache just like import/export annotations routine
TODO: dispatch annotationQueue shift action across reader
main-process and broadcasted to every other browserWindow opened ANNOTATION_SHIFT_FROM_ANNOTATION_IMPORT_QUEUE need this mechanism
…our advice about this !
yes I would also use the HTML body as the DOM tree root to compute text position and quote. |
Hello @danielweck, I let you review this PR for the import/export annotationSet part |
makeRefinable from apache-annotator TODO: need to define an heuristic of selector priority/importance
…note and do not create a duplicate note to the list
So, I have been running a bunch of tests and I found this bug when attempting to import 4 saved annotations in the same document, after deleting 2 of the original ones:
|
Note that I successfully ran the same scenario in a different publication (4 annotations in a single document, obviously with different DOM Ranges):
|
The EPUB which causes the problem is protected by LCP, and I notice that the annotation set does not contain annotations with selector |
… import (LCP protected EPUB?), also added DOM Range normalisation prior to convertRange() to guard against ranges created outside the navigator
Adding EDIT: I found |
…daries, also fixed the collapsed range checks
… ... basic normalisation from user selection that can be reversed => same selected text can yield different annotations
I found bugs in "creator" filtering, this affected export as well because of hidden / not-shown annotations in the list. I will merge this PR as-is and we can fix this later. |
Well done @panaC great work as usual! :) |
Follow-up issue #2758 |
This PR allows :
For the CFI Selector : vivliostyle-cfi doesn't support CFI Range with the comma separator cf (only position supported)
This PR is on a blocking state as long as the CFI library issue is not resolved.
We leave the choice to use CFI for the moment
for the benefice to use a CssSelector refined by TextPositionSelector
Cf the spec made by Laurent : https://github.com/readium/annotations
I reintroduced the ProgressionSelector to keep this information across export/import in thorium annotation format. Progression is generated from the r2-navigator to chrome ui and not from the ui to the r2-navigator.