-
Notifications
You must be signed in to change notification settings - Fork 186
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
Highlight Support in ReadiumJS Disabled? #657
Comments
from Ken Jones, ken@circularsoftware.com I've noticed that the method I have been using to highlight overlaid panels on sheet music has stopped working on both readium.github.io/readium-js-viewer and readium.firebaseapp.com e.g. I am using a CSS brightness adjustment to affect the appearance of the panels in time with the SMIL. |
from @danielweck The highlighter feature has been removed from the default build configuration of readium-shared-js (consequently, this impacts the default deployment of the cloud reader at Firebase and Surge). |
from @danielweck Oh, and you probably want to deactivate the Hypothesis annotation plugin as well (comment the line): I am building a version of the cloud reader for you... |
from @danielweck https://readium-highlights.surge.sh Example with a reflowable text document (to verify that the selection highlights work): |
In fact, activating hypothesis by default may have been a bad move, as many implementers are in fact relying on the default highlight module. We may decide to revert to the default highlight module for 0.28; any thoughts? |
Many? Who? Bear in mind that this change only affects those deploying their own instance of CloudReader - not the SDK and not the Chrome app. When we made the decision to enable it, it was to enable it (the hypothes.is plugin) by default and document how to (easily) disable it. |
@llemeurfr I think that the |
The Chrome app is totally controlled by the Readium management team, so they decide what features are shipped (normally, the highlights plugin is deactivated). The cloud reader deployed at Firebase and Surge is Readium's own build preference, and it contains Hypothesis instead of the highlights plugin (which did nothing but visually-select text, anyway). |
I see a problem with native apps though. In the GitHub repository (i.e. the actual source tree, develop and master branches), there is a ready-to-use build of |
@danielweck |
And to be clear: Hypothesis is a proper cloud-based annotation service, thus why its default integration in the deployed Readium web reader made sense for our demo purposes. The highlights plugin is a core utility feature with limited scope, not capable of managing actual annotations. It is meaningless to end-users, and only adds value when developers build functionality on top of it. |
PS: this debate about "which plugin to activate in the default build" is likely to arise again. I think we need a default config for the cloud reader, one for the Chrome app (soon to be deprecated, so perhaps not such a priority), and one of the readium-shared-js |
I have now realised that the reported bug has nothing to do with the highlights plugin that used to be shipped by default in the build of the cloud reader (now superceded by Hypothesis). |
Closed via resolution of #658 |
@danielweck The hypothesis plugin when enabled in the cson file works normally, however the plugin highlights, enable but does not make the markup, what can I do? |
This issue is a Discussion
Related issue(s) and/or pull request(s)
None?
Expected Behaviour
Highlights should work
Observed behaviour
Instead, with release 0.27 they are disabled by default
Test file(s)
Not available?
Product
ReadiumJS, 0.28-alpha
Additional information
None?
The text was updated successfully, but these errors were encountered: