-
Notifications
You must be signed in to change notification settings - Fork 31
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
added method to get homepage #78
Conversation
getHomepage(): string | null { | ||
let homepage: any = this.getProperty("homepage"); | ||
if (!homepage) return null; | ||
if (typeof homepage == "string") return homepage; | ||
if (Array.isArray(homepage) && homepage.length) { | ||
homepage = homepage[0]; | ||
} | ||
return homepage["@id"] || homepage.id; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Presentation 3 the homepage is part of the provider
https://iiif.io/api/presentation/3.0/#provider but maybe that can be part of the #73 task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello Stephen,
I'm reading the spec for the V3 Presentation API differently- under https://iiif.io/api/presentation/3.0/#homepage It says that "Any resource type MAY have the homepage property". Then, later on at https://iiif.io/api/presentation/3.0/#52-manifest, the example for manifests shows homepage in two different places. First it's inside provider, but also on its own.
(This json schema, from the presentation validator, also allows homepage in places other than inside the provider element.)
I don't mind jumping in to help with #73 in any way that I can, but could you take another look at the spec and let me know if I'm misinterpreting it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh you're right, that's confusing eh! Maybe #73 will change the semantics of this to also check a provider if there is no homepage on the provider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add some logic to this PR to take providers into account. Does the following make sense?
Because any resource type may have the homepage property, the getHomepage() method still makes sense in the IIIFResource class.
When an instance of one of the subclasses of IIIFResource calls this method, it should first check for the existence of the homepage object as above. If the homepage object isn't present, it should check to see if a provider with a homepage is present on the instance.
The getHomepage() method should be analogous to the getLogo() method, in that if multiple homepages, or providers with homepages, exist, it should return the first one only. It should return the homepage URL as a string.
Please let me know what you think- and again if you'd prefer to push forward on #73 and I can help with that, please let me know too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure that this should take provider into account, as it's semantically different. In this context we're looking for some way to work out what the canonical uri is of the viewer, whereas in provider the homepage is the homepage of the provider (and that could be who hosts the content, who hosts the viewer, who hosts the transcription, etc).
I think what you have now is good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's great, thank you both so much for taking a look at this. If there is anything else I can put together for this PR, please just let me know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we've come full circle, we can maybe look at #73 completely separately and merge in what you've put together 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's excellent Stephen- thank you so much. Please let me know what the release schedule is like when you have a chance. When will this be in the main branch?
Hello,
I'm having some trouble modifying the "share URL" link in the Universal Viewer. Because we're using version 3 of the Presentation API, we no longer have the related property- but in a conversation in the #dev channel on the UniversalViewer Slack, Andy Irving recommended using homepage as a replacement.
This PR is to allow manifesto.js to get the homepage from a manifest- if this PR is ok, I'll follow up with a PR to the UniversalViewer code next.
Please let me know if there are any changes I can make to this or any additional code I can submit. Thanks so much for considering this.
John