cu-mkp / editioncrafter

Software for the development of EditionCrafter, digital critical edition publication tool
https://cu-mkp.github.io/editioncrafter/
MIT License
8 stars 3 forks source link

Week of 5/22 changes #46

Closed camdendotlol closed 1 year ago

camdendotlol commented 1 year ago

Note

Since Nick is away this week and I have several interconnected tasks, I've decided to make all the changes in one PR. I know it's not the best as far as code review goes, but it's difficult to work across several branches that build upon one another and I know the rebases would be a whole project in themselves.

Summary

13 - Configurable transcription types

15 - Inline figures

This assumes a structure of:

<figure>
    <graphic url="/cats/hungry.png" />
    <figDesc>I'm a description!</figDesc>
</figure>

The figDesc is optional, but if it exists the function will add an alt tag and a figCaption HTML element containing the text.

14 - Moving comments from JSON file to TEI

Note

The note contents are still parsed as HTML using html-react-parser. It may be desirable to simplify this to a simple string display, but I didn't want to assume anything.

16 - Glossary file in manifest

The changes on the CLI side at https://github.com/cu-mkp/editioncrafter-cli/pull/11 are more substantial and more likely to require changes.

Screenshots

Inline figure

Screenshot 2023-05-23 at 11 48 13 AM

NickLaiacona commented 1 year ago

This is great - I tried out #13 with some test data and that worked great for me. On #16, I was hitting an error here switching pages. It was blocking me, so I fixed it with this modification:

function* resolveGlossary(manifest) {
  const glossary = yield select(justGlossary);
  if (!glossary.loaded) {
    if (
      !manifest?.seeAlso
      || manifest.seeAlso.length === 0
      || !manifest.seeAlso[0].id
    ) {
      throw new Error('Missing glossary link in seeAlso array.');
    }

    const glossaryURL = manifest.seeAlso[0].id;  
    const response = yield axios.get(glossaryURL);
    yield putResolveAction('GlossaryActions.loadGlossary', response.data);
  }
}

If this looks good, please include it in your PR, I didn't check it in on my end. Also, I think it will be necessary to make the glossary an optional thing, not every project will have one. This is probably a seperate issue/feature.

I haven't looked at #14 and #15 yet but will feedback here when I do.

NickLaiacona commented 1 year ago

Hi @camdendotlol I looked at this for a bit, it would be a lot easier for me to test if you could patch in the code for the glossary, makes stuff on my branch of editioncrafter-cli work beter.

camdendotlol commented 1 year ago

@NickLaiacona Just seeing this now - I'll set a reminder to do it first thing Monday.

camdendotlol commented 1 year ago

@NickLaiacona Done.