IIIF / discovery

8 stars 3 forks source link

Comments on Content State API 0.2 draft #51

Closed aisaac closed 3 years ago

aisaac commented 5 years ago

Here are some comments on https://preview.iiif.io/api/content-state-0.3/api/content-state/0.2/ Some editorial, some not...

  1. In the intro, "open the found resource in a compatible environment" sounds too simple for a first explanation (in the first paragraph). I feel readers would be better introduced to the goal if the bit on "initialize the view of that resource in any client that implements this specification" (in the second paragraph) would be present in the very first paragraph.

  2. In 1.1 "typically Manifests or Collections, or parts thereof." could be confusing in the sense that parts of collections are manifests (aren't they). Maybe "Manifests or parts or Collections thereof" (or whatever would be a grammatically correct expression of this!) would be less confusing for readers who are not sure of their mastery of the IIIF specs.

  3. In 2 the transition between "A content state is a JSON-LD data structure [...]. Viewers built to those specifications will already understand at least some of these models." and "This specification shows how a content state [...]" is not great as the former is quite specific and the latter comes back to the general goals of the spec. In fact I think that the bit that starts from "This specification shows how a content state [...]" and including the bullet list of use cases could be moved to the introduction. In 1.1 or maybe a new 1.x titled "Use Cases". Having this in the introduction would be the opportunity to add a sentence that reminds reader of the methodology we follow (i.e. based on use cases) and points to the set of IIIF Discovery use cases on github. By the way, "while load Manifest M" sounds a bit rough without any explanation of this case earlier. Maybe the suggestions above would create a context that makes this a bit smoother for the reader.

  4. In 2 "Such a description" would read better if something had been called a description earlier in the section (currently only 'data' is mentioned). Note that as per the re-structuring suggested in point 3, the new section 2 text could be [ Descriptions of content states to be exchanged according to this specification are expressed as annotations that target the intended part of a IIIF resource. A content state is a JSON-LD data structure that uses the models described by the IIIF Presentation API and W3C Web Annotation Data Model specifications. ]

  5. Still in 2, "Viewers built to those specifications will already understand at least some of these models" sounds like it's missing something like "according" before "to". And it also seems a bit tautological.

  6. In 2.2 I am a bit puzzled by the first introduction of contentState as a motivation. An explanation of the example below may help readers figure out why this motivation is chosen, without having to go back to earlier parts (and it seems good anyway to have a brief description of such an example).

  7. In 2.2.4 I don't understand "this form is not capable of expressing content states that are part of a IIIF resource, such as a Canvas within a Manifest, or a region of a Canvas." Canvas can have URIs and it should be possible to use them as target URL, shouldn't it? Or is the pattern of 2.2.4 expected to be used only with manifest URLs? In this case the constraint should be made explicit in the text.

  8. In 2.3. it would be great to have some examples with the different protocols - at least have a forward reference to what is shown in section 3. And the query string parameter is mentioned in the sentence "It is recommended that when passing the content state as JSON-LD in a query string parameter" without having been introduced properly before (with this term).

  9. In fact maybe my call for examples comes from a misunderstanding of what the section 2 is about based on a too ambitious title for section 2. "Content State" hints that the section is going to include all that is needed to understand what a content state is, while section 2 is rather an overview of what the spec uses, and how it fits together.

  10. The various questions in section 3 (as well as the entire 3.2 which seems about questions) could be flagged more visibly, from an editorial standpoint.

aisaac commented 4 years ago

@tomcrane have you ever seen these comments in fact? I could try to see if they still apply, as the draft has probably quite changed in between.

tomcrane commented 4 years ago

Hi @aisaac - if you could update them for the current draft that would be great!

aisaac commented 4 years ago

@tomcrane it's updated! One comment was moved to #57 in fact.

zimeon commented 4 years ago

I think it would be helpful to include the import base64 in the Python example, and also remove the space after the first "id" to be consistent:

>>> import base64
>>> base64.urlsafe_b64encode(b'{"id":"https://example.org/object1/canvas7#xywh=1000,2000,1000,2000","type":"Canvas","partOf":[{"id":"https://example.org/object1/manifest","type":"Manifest"}]}')
b'eyJpZCI6Imh0dHBzOi8vZXhhbXBsZS5vcmcvb2JqZWN0MS9jYW52YXM3I3h5d2g9MTAwMCwyMDAwLDEwMDAsMjAwMCIsInR5cGUiOiJDYW52YXMiLCJwYXJ0T2YiOlt7ImlkIjoiaHR0cHM6Ly9leGFtcGxlLm9yZy9vYmplY3QxL21hbmlmZXN0IiwidHlwZSI6Ik1hbmlmZXN0In1dfQ=='
aisaac commented 3 years ago

Most of my comments above have been fixed!

What remain is:

Point 8 still applies I think: In 2.3. it would be great to have some examples with the different protocols - at least have a forward reference to what is shown in section 3.

Point 10 still applies though differently: in 3.1.2 there is an "outstanding question" that is not super visible and a "question" box that is well visible. Why is there this difference?

tomcrane commented 3 years ago

@zimeon's comment addressed here: https://github.com/IIIF/api/pull/1935/commits/cabea7462e6b1ea860001a0fd11d66ea29e18c32

tomcrane commented 3 years ago

@aisaac's 2.3 comment addressed here: https://github.com/IIIF/api/pull/1935/commits/cd247cbca4254d24e177f338e1a13b4e80b5a6af#diff-6c7aa26b493f99591a421991e240fc05R203

tomcrane commented 3 years ago

I've moved the other question out into an issue (#75).

The 0.3 draft doesn't have any questions in it, I think.

(Also fixed the mixed identity - some of it said 0.2, some said 0.3).

tomcrane commented 3 years ago

Updated Preview URL (the version number now matches the text!)

https://preview.iiif.io/api/content-state-comments/api/content-state/0.3/

aisaac commented 3 years ago

ok I guess none of my comments hold, now! Thanks @tomcrane

tomcrane commented 3 years ago

Travis is taking forever, but eventually there should be an update with all the changes we went through on the last Discovery call:

https://preview.iiif.io/api/content-state-comments/api/content-state/0.3/