Core mirador behaviors to provide a plugin target for text resources #4086
Conversation
barmintor
commented
Jan 29, 2025
barmintor
commented
- refactor type-based filters into a module
- MiradorCanvas.imagesResources does not assume any service is an image service
- stub TextViewer shows empty div, source elements for text resources, and canvas navigation
- fixes There is no component target for a plugin targeting paintable resources of type 'Text' #4085
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4086 +/- ##
==========================================
+ Coverage 95.05% 95.07% +0.02%
==========================================
Files 315 317 +2
Lines 15905 15973 +68
Branches 2494 2509 +15
==========================================
+ Hits 15119 15187 +68
Misses 782 782
Partials 4 4 ☔ View full report in Codecov by Sentry. |
barmintor
force-pushed
the
4085-text-viewer
branch
from
7c63af6 to
093d303
Compare
src/lib/serviceProfiles.js
Outdated
| 'level2', | ||
| 'level1', | ||
| 'level0', | ||
| 'http://iiif.io/api/image/2/level2.json', |
There was a problem hiding this comment.
I think there's an even older set of image profile urls, e.g.:
http://library.stanford.edu/iiif/image-api/compliance.html#level1
src/lib/canvasTypes.js
Outdated
| /** values for type/@type that indicate a video content resource */ | ||
| const videoTypes = ['Video', 'MovingImage', 'dctypes:Video', 'dctypes:MovingImage']; | ||
|
|
||
| export default { |
There was a problem hiding this comment.
I'm not sure how painful it'd be, but I wonder if we can push these into the config ( + redux store) so downstream applications can extend these lists? We've done something similar with thumbnails.preferredFormats, auth.serviceProfiles, and annotations.filteredMotivations.
There was a problem hiding this comment.
I don't think it would be that bad, with the usual caveat of "naming is hard"
There was a problem hiding this comment.
ok, I did this but it's a significant change - you need access to Mirador config everywhere you create a MiradorCanvas - so I put it in a separate branch. Do we want this in the PR? cul@d6e9db8
There was a problem hiding this comment.
Yeah, ew. Having to pass it around to make it available for ThumbnailFactory (especially) seems pretty bad. Maybe that has to wait until we get around to doing something about Manifesto and can inject that configuration early?
There was a problem hiding this comment.
Another option might be to push ThumbnailFactory into the redux store behind a memoized selector?
There was a problem hiding this comment.
(I should add that some of this is also background for Auth2, where you can't assume that image services are the only associated services)
There was a problem hiding this comment.
There was a problem hiding this comment.
☝️ that commit takes the selector approach. I'm not sure it saves a lot of code, but it has a more appealing separation of concerns?
barmintor
force-pushed
the
4085-text-viewer
branch
from
093d303 to
5120821
Compare
- stub TextViewer shows empty div, source elements for text resources, and canvas navigation - fixes ProjectMirador#4085
barmintor
force-pushed
the
4085-text-viewer
branch
from
5120821 to
cf770ed
Compare
|
@cbeer noting that it is separately desirable to have a TextViewer target component and to have configurable canvas types and service profiles, I'm pushing a branch here that only does the former. |
|
👍 Handling those two concerns separately makes sense to me. |
