esmero / format_strawberryfield

Set of Display formatters, extractors and utils to make Strawberry field data glow
GNU Lesser General Public License v3.0
6 stars 9 forks source link

ISSUE-292: IIIF driven paging/sequence + order drive hints + cover page #419

Closed DiegoPino closed 4 months ago

DiegoPino commented 4 months ago

See #292

@alliomeria and @patdunlavey @giancarlobi don't test yet, I need to add a few extra config options for the formatter too. Thanks

DiegoPino commented 4 months ago

@alliomeria and @patdunlavey this is ready for review:

What is new (or ho to test). Get the code of course (this is based on 1.4.0)

for the cover (see the explanation on the new setting for this formatter, set a RAW JSON key with either true/false values in the ADO itself will default to "hasCover": true (normal book heavier)

This pull also brings exposed metadata displays to the IABookreader. This is better than the internal Metadata display because it will allow IABookreader to adapt without a cache reset (because the actual URL is the one dealing with the cache)

I might add more cache goodies once you test this ok?

Thanks you both

alliomeria commented 4 months ago

Awesome, thank you @DiegoPino !

Following up to note the issue we talked about internally earlier today related to the placement of the viewingHint for v2. Technically, valid IIIF v2 specs permit placement of viewingHint nested within "sequence", but for this iteration of the PR, it needs to be outside/out a level.

Will check on the cover image setup also and let you know if I have any questions or feedback about that.

Thanks again, Diego! Think people are going to groove on the extra options you're providing. :)

DiegoPino commented 4 months ago

@alliomeria thanks for the feedback. Will make sure I also take the sequence in consideration, I think our default shipped V2 one has it there, so it is a must. :)

DiegoPino commented 4 months ago

@alliomeria code/issue addressed! Thanks. @patdunlavey can you please also give this a try?spin? Would love to merge as soon as possible if all works given that there is another open pull against the same code that will need a rebase from me. Thanks a lot

patdunlavey commented 4 months ago

@DiegoPino thank you for all your work on this!! I will try to test it out tomorrow.

DiegoPino commented 4 months ago

@patdunlavey I'm rebasing this pull (because of #420 ) so if testing don't try to merge against 1.4.0. Just fetch this branch and test. Thanks

DiegoPino commented 4 months ago

@patdunlavey and @roromedia. This was rebased and includes also the code from #420. test at will. If I don't get from anyone "this does not work" I will merge tomorrow by noon. Thanks!

patdunlavey commented 4 months ago

@DiegoPino thanks, I tested this with both API 2.1 and 3.0 manifests. After some futzing around with the metadatadisplay templates and the display settings, I got the "viewingHint": "individuals" (v2.1 manifest) working, and I got "behavior": ["individuals"](v3.0 manifest) working. That was a while ago. However now it's not working and I haven't figured out why.

After the initial success with the individuals view, I set about trying to figure the "hasCover" option. I set "hasCover": 0, in the json for an object and configured both manifest versions to specify the "paged" mode. I assumed that this should result in the first page being displayed 2-up (page 1 and 2), but I did not see that. It just displayed a single page in the right half of the viewer for page 1.

Subsequently I tried to re-create the 1-up ("individuals") configuration that I had working earlier, and so far no amount of futzing with the v2.1 and v3.0 templates or display settings has given me back my vertical scrolling single page view. I've performed regular drupal cache-clear repeatedly, as well as switching to incognito and a different browser, and the single page view refuses to return. Presumably I missed something in my trips through the configs, but I'll be damned if I can see what it is.

DiegoPino commented 4 months ago

Ok, let me double check (I just checked but maybe my rebase just messed up stuff). Did you clear also your browser caches? Any errors on your browser JS console.

DiegoPino commented 4 months ago

Will share on slack a tiny video ok @patdunlavey ?

patdunlavey commented 4 months ago

:man_facepalming: I found the problem - "individual" vs "individuals"! NEVERMIND!

DiegoPino commented 4 months ago

Good! Ok, will still make the video bc I love movies! Will share soon! hahahah

patdunlavey commented 4 months ago

And I figured out my "hascover" problem. Once again an itty-bitty typo! "hasCover" is not the same as "hascover"! :man_facepalming: :fist_left:

With these two things working, I think that complete my review. I'll indicate such with a proper review comment.

DiegoPino commented 4 months ago

Thanks a lot @patdunlavey key sensitive, yep a thing in JSON (funny not in PHP ... but yes if an array key... anyways.... appreciate the testing! Video shared too. hugs

DiegoPino commented 4 months ago

Thanks to you @patdunlavey !