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

First proposal for ONIX Techniques #223

Merged
merged 4 commits into from
Feb 2, 2024
Merged

First proposal for ONIX Techniques #223

merged 4 commits into from
Feb 2, 2024

Conversation

gregoriopellegrino
Copy link
Collaborator

As agreed, I made an initial proposal for the ONIX Techniques, specifically:

  • I set the structure of the sections according to the organization of the principles document by key information
  • I eliminated the example ONIX record id, let's evaluate if it makes sense to reinsert it later, I don't see it in the techniques for EPUB there
  • I wrote some "Conventions for implementations" which I would ask you to read, they seem to me to be the basis for the EPUB techniques document as well; I should still think about how to handle a data structure in the form of an array, but I am still thinking about it
  • I have re-written using ONIX codes the techniques for Visual adjustments, if you can read it if it seems clear to you; then it will need Chris's supervision to be sure it is fully correct, but right now we are more interested in the form than the content

<li>For EPUB Accessibility, WCAG-A: <a href="https://ns.editeur.org/onix/en/196">List: 196</a>; Code: 02: EPUB Accessibility Specification 1.0 A</li>
<li>For EPUB Accessibility, WCAG-AA: <a href="https://ns.editeur.org/onix/en/196">List: 196</a>; Code: 03: EPUB Accessibility Specification 1.0 AA</li>
<li>For EPUB Accessibility, WCAG-AAA: Not available in ONIX.</li>
<li><b>if</b> <var>codelist 196: code 36 (All textual content can be modified)</var> is present
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure about the usability of lists of one element. I would stick with only one level lists.
I'm wondering if the use of DL, DT, DD elements would be more representatives of what we want to achieve here. My only concern is that we are inverting here DD and DT (i.e. we give the definition before the term).

Copy link
Member

@mattgarrish mattgarrish Dec 13, 2023

Choose a reason for hiding this comment

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

These would be fine left as single sentences. "if codelist 196: ... is present, then ..."

I'd leave using nested lists for cases where there are multiple sub-options to pick from.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. I thought about the use of lists to have an indentation that is not only graphical, but also semantic, but we can use other options without problems.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems to me that if we keep the if statement and the instructions on one line the lines become very long and the readability is not very good: normally in the code the instructions related to an if statement are inserted in the next block and indented... But I have no idea how to solve it

Copy link
Member

Choose a reason for hiding this comment

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

normally in the code the instructions related to an if statement are inserted in the next block and indented.

I haven't seen that done in specifications. None of the algorithms in publishing specs we've done to date have formatted conditionals this way unless there are nested conditionals within them.

Here are a couple of examples from the infra spec just for variety:

If you really think it's important, put spans around them and format them as inline blocks with the expected behaviour indented. It should still read as a sentence.

Copy link
Member

Choose a reason for hiding this comment

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

For example:

<li>
   <span class="condition"><b>if</b> <var>codelist 196: code 36 (All textual content can be modified)</var> is present</span>
   <span class="result">then display <code>Appearance can be modified</code>.</span>
</li>

and:

span.condition {
   display: inline-block;
}
span.result {
   display: inline-block;
   margin-left: 2rem;
}

Copy link
Collaborator

@clapierre clapierre left a comment

Choose a reason for hiding this comment

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

Great start

@mattgarrish
Copy link
Member

For big changes like this, it's helpful to add the preview and diff links:

(You can copy these links from PR to PR; you just have to change the "gregorio-updates" branch name in the URLs to whatever you're using next. For the other techniques doc, you'd also flip the directory to "epub-metadata".)

@GeorgeKerscher
Copy link
Collaborator

I like Gautier's idea of using a definition list with the DD being the conditions and the display statement in the dt. This seems to be semantically more correct for our purposes.

I did test this and received no errors.

@gregoriopellegrino
Copy link
Collaborator Author

Thank you all for your comments and suggestions. I think the easiest thing is to discuss which way to go in the editors' call, so we can decide how to proceed.

The techniques document is now made of pseudo-code examples.
@gregoriopellegrino
Copy link
Collaborator Author

Here is a new version with many changes to ONIX techniques, thanks to the team of editors who provided many suggestions and contributions.

You can see the preview here.

Some of the changes made:

  • techniques are now written in real pseudo-code
  • the tagging used to identify the procedures is sematically rich, this also allows screenreader users to understand the content well
  • we have made an effort to make it more readable by introducing some CSS
  • we tried to make the code more understandable with the definition of some functions common to all techniques
  • each technique is followed by an explanation (in HTML details element) that explains in a human-understandable form the metadata that comes into play, with links to ONIX codelists

We await feedback from the group.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not an ONIX expert and it seems that the word "text" is an important term in ONIX. It is refering to real text that can be enlarged and modified. I think we might make it easier to read if we changed some of the variable names that use the word"text".

For example where it says:
LET onix be the result of calling pre processing given text.
Here perhaps instead of text we might say "ONIX-Record-as-text"

Further down it says:
LET text be the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/PrimaryContentType[text() = "10"] OR the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/ProductContentType[text() = "10"].

In the above, instead of the variable "text" perhaps it could be "real-text"

I think this is similar to the comment charles made.

Also, typo in the details section: there is an eg and it should be e.g.,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I should have fixed it, thanks for the feedback!

@GeorgeKerscher
Copy link
Collaborator

I have reviewed and it looks good!

Copy link
Collaborator

@clapierre clapierre left a comment

Choose a reason for hiding this comment

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

Looks great Gregorio.

One suggestion might be in the details for Images of Text you could give an example of a newspaper clipping, to help the reader understand the difference between images of text and text within images a little better.

@clapierre
Copy link
Collaborator

Question, is this link invalid because we haven't published this yet?

This technique relates to Visual adjustments key information.

@clapierre
Copy link
Collaborator

clapierre commented Jan 30, 2024

Looking at this I am wondering if we should separate the defining of the variables with the LET xyz ... and then a separate Ordered List for the IF/ELSE IF / ELSE section.

I think that might make it easier to digest.
Something like this.

  1. LET onix be the result of calling pre processing given onix_record_as_text.
  2. LET all_textual_content_can_be_modified be the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/ProductFormFeature[ProductFormFeatureType = "09" and ProductFormFeatureValue = "36"].
  3. LET real_text be the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/PrimaryContentType[text() = "10"] OR the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/ProductContentType[text() = "10"].
  4. LET text_within_images be the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/PrimaryContentType[text() = "45"] OR the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/ProductContentType[text() = "45"].
  5. LET images_of_text be the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/PrimaryContentType[text() = "49"] OR the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/ProductContentType[text() = "49"].

1. IF all_textual_content_can_be_modified: 
     THEN display "Appearance can be modified".
2. ELSE IF real_text AND NOT all_textual_content_can_be_modified: 
     THEN display "Appearance cannot be modified".
3. ELSE IF text_within_images OR images_of_text: 
     THEN display "Appearance cannot be modified for all textual content".
4. ELSE 
     display "Appearance modifiability not known".

@clapierre
Copy link
Collaborator

By separating this into 2 lists you also get the benefit that
visual-adjustments-1 "Appearance can be modified" goes with the IF statement's List item 1.
visual-adjustments-2 "Appearance cannot be modified" goes with the ELSE IF statements list item 2.
etc.

@gregoriopellegrino
Copy link
Collaborator Author

Question, is this link invalid because we haven't published this yet?

This technique relates to Visual adjustments key information.

Right

@gregoriopellegrino
Copy link
Collaborator Author

Looking at this I am wondering if we should separate the defining of the variables with the LET xyz ... and then a separate Ordered List for the IF/ELSE IF / ELSE section.

I think that might make it easier to digest. Something like this.

  1. LET onix be the result of calling pre processing given onix_record_as_text.
  2. LET all_textual_content_can_be_modified be the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/ProductFormFeature[ProductFormFeatureType = "09" and ProductFormFeatureValue = "36"].
  3. LET real_text be the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/PrimaryContentType[text() = "10"] OR the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/ProductContentType[text() = "10"].
  4. LET text_within_images be the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/PrimaryContentType[text() = "45"] OR the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/ProductContentType[text() = "45"].
  5. LET images_of_text be the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/PrimaryContentType[text() = "49"] OR the result of calling check for node on onix, /ONIXMessage/Product/DescriptiveDetail/ProductContentType[text() = "49"].

1. IF all_textual_content_can_be_modified: 
     THEN display "Appearance can be modified".
2. ELSE IF real_text AND NOT all_textual_content_can_be_modified: 
     THEN display "Appearance cannot be modified".
3. ELSE IF text_within_images OR images_of_text: 
     THEN display "Appearance cannot be modified for all textual content".
4. ELSE 
     display "Appearance modifiability not known".

What is your opinion @mattgarrish ?

@GeorgeKerscher
Copy link
Collaborator

I wonder if we added "is true" or is false" would make it more readable. Also the and / or statements may be emphasised by using the words both or either , such as:

  1. IF all_textual_content_can_be_modified: is true,
    THEN display "Appearance can be modified".
  2. ELSE IF both real_text AND NOT all_textual_content_can_be_modified: are true,
    THEN display "Appearance cannot be modified".
    . ELSE IF either text_within_images OR images_of_text: are true,
    THEN display "Appearance cannot be modified for all textual content".
  3. ELSE
    display "Appearance modifiability not known".

@gregoriopellegrino
Copy link
Collaborator Author

@GeorgeKerscher we'll ask the group for a feedback about your proposal, thanks.

@clapierre clapierre merged commit 002a968 into main Feb 2, 2024
@clapierre clapierre deleted the gregorio-updates branch February 2, 2024 16:21
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.

None yet

5 participants