Closed alxp closed 1 year ago
Hi folks - just a heads up that this is in our queue to review as it is foundational to some work we need to do in the new year. Sorry this has sat so long - we'll be doing this work in the first couple weeks of January. CC @patdunlavey @dmer
I tested this update (in a copy of isle-dc) and successfully reproduced the behavior described under "How this should be tested": (quick demo).
My thoughts:
My take is that this PR does what it says it does. It's a prerequisite (necessary but not sufficient) for integrating search with Mirador. Thumbs-up on a merge from me, with the proviso that a detailed documentation task be created.
Put together some thoughts towards how things might be better structured in https://github.com/Islandora/islandora_mirador/compare/update-mirador...adam-vessey:islandora_mirador:madscience
This could still go a ways further towards introducing plugins for the various methods to install things, potentially having them be configurable. Unknown if further things, such as reporting the Mirador plugins supported by the installation would be feasible.
@alxp Out of curiosity, have you done any thinking/planning around IIIF Search API in relation to Mirador? Pat and I have a goal of being able to pass a search term from Drupal to an object displaying in Mirador and have that term highlighted. As he mentioned in his comment above, it looks like this requires a Search API endpoint that can return the specific annotations from the HOCR that should be highlighted and blurbed. Just getting started, but if you are interested, we'd love to chat about it with you. Definitely open to collaborating. I'll share the more detailed dev plans as soon as they're at least in rough-draft state.
@adam-vessey I've made an update that removes the remote library option since anyone who can compile a custom version of the app can certainly deploy it locally.
WRT the default library location, I intend to submit the Islandora-targeted Mirador app to the Islandora repo so there isn't a RobLib dependency
@dmer @patdunlavey I've created an issue to discuss search / annotation highlighting. I think we can adapt a lot of what UT Scarborough did but would be worth discussing the architecture for sure.
ping @adam-vessey
@rosiel : As mentioned in the tech call, I'm fine with things here now; however, there's reviewers requested explicitly by this PR though, of which I'm not one.
I believe that the reviews by Pat and Adam are sufficient. I looked at some of this with Pat and so I am going to merge the PR. At the time, I wanted to be a reviewer in the event that nobody could get to this in a timely manner.
What does this Pull Request do?
Provide a brief description of what the intended result of the PR will be and/or what problem it solves.
https://github.com/Islandora/islandora_mirador/issues/4
What's new?
Updates Mirador to be able to use either a local copy of the Mirador app or one hosted on a CDN.
Changes the default to a remote Mirador hosted on Github Pages that contains several plugins and is updated to Mirador 3.3.
Adds support to enable individual plugins in the admin interface.
Updated Mirador so any new behaviours will appear in Islandora.
How should this be tested?
Test that the update hook runs properly.
Go to the Admin settings page for Islandora Mirador and test the new settings.
To test text overlay integration specifically:
Follow the testing instructions for https://github.com/Islandora/islandora/pull/897
GO to Admin > Structure > Views and edit the IIIF Manifest view.
In the manifest you want to use, add the hOCR media field you created. Then click the Settings for the IIIF Manifest views plugin, and select that field in the list for hOCR Structured Text.
GO to the settings for Islandora Mirador and enable the Text Overlay and Image Tools plugins.
You may need to clear cache. Go to the node for the Paged Content item you created and observe that the plugins are working.
Documentation Status
Additional Notes:
Any additional information that you think would be helpful when reviewing this PR.
Interested parties
Tag (@ mention) interested parties or, if unsure, @Islandora/committers