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

IIIF v3 seeAlso and textual AnnotationPage support #208

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sauterl
Copy link

@sauterl sauterl commented Oct 30, 2021

This PR enables seeAlso support on canvas-level for IIIF v3 manifests as well as "inline" resources, such as supplementing annotaitons with a body of TextualBody.

This at least partially addresses #186

There is one caveat though, as of right now, I did not manage to get the tests running, which might be due to some minor refactoring on my side. Contributions are welcome.

* @param annotationJson Annotation-like json sturct
*/
const naiveIIIFv3Check = (annotationJson) =>
annotationJson && annotationJson.type && annotationJson.type === 'AnnotationPage';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
annotationJson && annotationJson.type && annotationJson.type === 'AnnotationPage';
annotationJson?.type === 'AnnotationPage';

@@ -123,7 +130,22 @@ export async function fetchAnnotationResource(url) {
}

/** Saga for fetching external annotation resources */
export function* fetchExternalAnnotationResources({ targetId, annotationId, annotationJson }) {
export function fetchExternalAnnotationResources({ targetId, annotationId, annotationJson }) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure that this is working as expected? This should be a Generator that yields Redux-Saga effects and not a plain function.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for pointing this out. I did thorougly test the seeAlso variant and only checked the "inline" one.

/** Saga for processing texts from IIIF annotations */
export function* processTextsFromAnnotations({ targetId, annotationId, annotationJson }) {
export function processTextsFromAnnotations({ targetId, annotationId, annotationJson }) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above, is this working as expected?

@sauterl
Copy link
Author

sauterl commented Nov 8, 2021

Thanks, @jbaiter for your review. I will work on the requested changes during this week and especially try to come up with a dedicated IIIF v3 collection JSON for testing and demo purposes

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

Successfully merging this pull request may close these issues.

2 participants