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

Paged content config #143

Merged
merged 8 commits into from
Oct 3, 2019
Merged

Conversation

dannylamb
Copy link
Member

GitHub Issue: Islandora/documentation#932

What does this Pull Request do?

This pulls together all the config needed for the work from the Paged Content sprint. It ties together all the great work from everyone. Plus it installs a few extra things we've collected over time but aren't installed by default (oai-pmh and breadcrumbs). Also: admin_toolbar.

What's new?

Paged content!

How should this be tested?

  • Pull down this PR
  • vagrant up
  • Make a 'Repository Item' and give it a model of 'Paged Content'
  • Add some children to it
    • Give each one a model of 'Page'
    • Give each one a TIFF or JPEG for an 'Original File'
  • Clear your cache
  • Visit the 'Paged Content' item and you should see openseadragon pop up with arrows to go left/right through the pages. You should also see a 'reference strip' at the bottom of the viewer so you can see all the pages and where you are in the sequence.
  • From your 'Paged Content' node, click its Children tab, and then the "Reorder Children" button
  • Re-order the pages using the drag and drop
  • Clear your cache
  • Visit your 'Paged Content' node again, and the order of the pages should be reflected.

Interested parties

Fun one to test that shows all the great work the community has done! @Islandora-Devops/committers @rangel35 @dbernstein @elizoller @manez

@DonRichards
Copy link
Member

What's the story with JP2 support?

@dannylamb
Copy link
Member Author

It would work the same way as a tiff. If you already have JP2s you're already fine (try one though, please!). If you need to generate them, I think we're hung up on reliably generating them. But its been a while since we've investigated that.

@rangel35
Copy link

@dannylamb great, we are generating the JP2s already so I can test that tomorrow

@rangel35
Copy link

@dannylamb @DonRichards I tested this with ingested JP2s and it worked as advertised...so if you have JP2s pre-created then you can use them...

@rangel35
Copy link

any chance the thumbnail panel at the bottom could be made smaller....

@dannylamb
Copy link
Member Author

@rangel35 Yep. In Configuration > Media > Openseadragon settings, there's a slew of options to play with.

Screenshot from 2019-09-27 07-49-52

@rangel35
Copy link

is there a way to add page level metadata to the display...currently only the top book level metadata shows up for every page but if the individual children also contain metadata that is lost (not displayed)

@dannylamb
Copy link
Member Author

Off the top of my head I don't know. If you can put it in the manifest, and i can pass it to the viewer, then in theory it's possible.

@dannylamb
Copy link
Member Author

FYI I've noticed that when viewing individual pages with openseadragon, there's some issues. Sequence mode when there's only one item really messes with things. I'll see if I can make the viewer autodetect if they need to go into sequence mode.

@seth-shaw-unlv
Copy link
Contributor

@rangel35, I mentioned that to @dannylamb on a thread in Slack. OpenSeadragon does support overlays that can add metadata (see the "Overlaying Complex HTML" example in the linked page); although the provided example didn't persist across the sequence paging. I'm sure we could do it, but not in time for IslandoraCon or even 1.1 unless someone got cracking on it now.

Brainstorming here: we would have to add a relationship between the media at it's node in the IIIF Manifest view to get the fields we want to display and then update the IIIF Manifest formatter to include it. Although, does the IIIF manifest have a place for descriptive metadata? 🤷‍♂️

I'm actually interested in how Mirador works with it.

@rangel35
Copy link

thanks @seth-shaw-unlv I'll take a look at the link...the current install works great for generic books but we have legal documents that are paged and have page level metadata

@dannylamb I also noticed that if you start in page 2 it won't go to page 3 and beyond even if they do exist....this would prevent people from starting at say page 100 and continuing to finish if there is a 500 page document.

@seth-shaw-unlv
Copy link
Contributor

@rangel35 I have some basic views that help with that scenario (although they need refinement). I still need to get them to jump to a certain point in the listing... but it is better than nothing.

@rangel35
Copy link

thanks @seth-shaw-unlv I was thinking along the lines of trying to create a block for the metadata

@dannylamb
Copy link
Member Author

I've pushed up changes for autodetecting sequence mode, with collection mode overriding it if user chooses to select it. I'm testing the build now.

@dannylamb
Copy link
Member Author

This passes my smoke test and is ready to be tested by the larger group again.

@elizoller
Copy link
Member

@seth-shaw-unlv mirador has some capabilities built in to display metadata. i'll see if i can throw together a demo of what i mean real quick.

@rangel35
Copy link

I think the main issue will be getting the page level metadata into the manifest so that mirador can access it

@elizoller
Copy link
Member

Well instead of trying to do that in islandora real fast - https://projectmirador.org/demo/ if you click the i icon in the upper right corner you get a little metadata hover. I agree with @rangel35 it'd just be a matter of getting that into the manifest, which would probably involve some additional work in https://github.com/Islandora-CLAW/islandora/blob/8.x-1.x/modules/islandora_iiif/src/Plugin/views/style/IIIFManifest.php

@whikloj
Copy link
Member

whikloj commented Sep 30, 2019

vagrant up

@elizoller
Copy link
Member

elizoller commented Sep 30, 2019

When I run vagrant up on this I'm getting a composer error: ./composer.json has been updated
I did a fresh clone of the claw-playbook, checked out this branch and ran vagrant up.
Screen Shot 2019-09-30 at 10 08 26 AM

@dannylamb
Copy link
Member Author

dannylamb commented Sep 30, 2019

@elizoller I was getting that earlier. Do you have this commit? ff50c3a

I have to put the openseadragon version in islandora_default's composer.json and in that inventory variable from that commit.

@elizoller
Copy link
Member

No because i have claw-playbook:paged-content-config instead of dannylamb:paged-content-config :(

@whikloj
Copy link
Member

whikloj commented Sep 30, 2019

Pro-tip: https://wiki.duraspace.org/display/FF/Guide+for+New+Developers#GuideforNewDevelopers-ConfigureforPerformingCodeReviews

Then I just do:

[/sw/var/www/DAM2/claw-playbook] 
> git fetch origin
remote: Enumerating objects: 18, done.
remote: Counting objects: 100% (18/18), done.
remote: Compressing objects: 100% (4/4), done.
remote: Total 30 (delta 13), reused 17 (delta 13), pack-reused 12
Unpacking objects: 100% (30/30), done.
From https://github.com/Islandora-Devops/claw-playbook
   f3be6d5..3563dca  dev                  -> origin/dev
 * [new branch]      paged-content-config -> origin/paged-content-config
   5099a15..6912b14  refs/pull/141/head   -> origin/pull/141
 + a62a8e8...4117f91 refs/pull/142/head   -> origin/pull/142  (forced update)
 * [new ref]         refs/pull/143/head   -> origin/pull/143

[/sw/var/www/DAM2/claw-playbook] 
> git checkout origin/pull/143
Note: checking out 'origin/pull/143'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b <new-branch-name>

HEAD is now at ff50c3a Missed an inventory var

@whikloj
Copy link
Member

whikloj commented Sep 30, 2019

To keep this up-to-date. On the paged_content object I am seeing

Notice: Undefined index: #id in at_core_theme_suggestions_block_alter() (line 115 of themes/contrib/adaptivetheme/at_core/includes/suggestions.inc).

at_core_theme_suggestions_block_alter(Array, Array, 'block') (Line: 449)
Drupal\Core\Theme\ThemeManager->alterForTheme(Object, 'theme_suggestions', Array, Array, 'block') (Line: 458)
Drupal\Core\Theme\ThemeManager->alter(Array, Array, Array, 'block') (Line: 245)
Drupal\Core\Theme\ThemeManager->render('block', Array) (Line: 437)
Drupal\Core\Render\Renderer->doRender(Array) (Line: 450)
Drupal\Core\Render\Renderer->doRender(Array) (Line: 450)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array) (Line: 501)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 72)
__TwigTemplate_62c3f4d8ab97d047fc5cc76dc106b2ac0da09586210503f6da3bfa8ed478ce17->doDisplay(Array, Array) (Line: 443)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 414)
Twig\Template->display(Array) (Line: 422)
Twig\Template->render(Array) (Line: 64)
twig_render_template('themes/contrib/adaptivetheme/at_core/templates/layout/row.html.twig', Array) (Line: 384)
Drupal\Core\Theme\ThemeManager->render('row', Array) (Line: 437)
Drupal\Core\Render\Renderer->doRender(Array) (Line: 450)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array) (Line: 501)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 63)
__TwigTemplate_1ec3110f8b54593a7ec27ae3ad01e01d2e7188bd1a773d4909efb03da2cbc6b8->doDisplay(Array, Array) (Line: 443)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 414)
Twig\Template->display(Array) (Line: 422)
Twig\Template->render(Array) (Line: 64)
twig_render_template('themes/contrib/carapace/templates/generated/page.html.twig', Array) (Line: 384)
Drupal\Core\Theme\ThemeManager->render('page', Array) (Line: 437)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array) (Line: 501)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1) (Line: 123)
__TwigTemplate_785de26570213494849bebc5870f66c8749014f7d3d42b96680dd828d65d266d->doDisplay(Array, Array) (Line: 443)
Twig\Template->displayWithErrorHandling(Array, Array) (Line: 414)
Twig\Template->display(Array) (Line: 422)
Twig\Template->render(Array) (Line: 64)
twig_render_template('themes/contrib/adaptivetheme/at_core/templates/layout/html.html.twig', Array) (Line: 384)
Drupal\Core\Theme\ThemeManager->render('html', Array) (Line: 437)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 195)
Drupal\Core\Render\Renderer->render(Array) (Line: 147)
Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() (Line: 582)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 148)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object)
call_user_func(Array, Object, 'kernel.view', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object) (Line: 156)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 693)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

@dannylamb is looking into setting the #id as it seems to be missing from the OSD block.

@kayakr
Copy link
Contributor

kayakr commented Sep 30, 2019

I followed @whikloj's notes above but at

(my_env) 12:54:48 jonathan:~/Documents/CatalystIT/vagrant-claw-playbook-vm12 ((HEAD detached at origin/pull/143)) $ git branch
(HEAD detached at origin/pull/143)

I get

TASK [Islandora-Devops.drupal-openseadragon : Install Openseadragon with
composer] *** Tuesday 01 October 2019  12:53:21 +1300 (0:00:00.120)      
0:08:26.447 ******* fatal: [default]: FAILED! => {"changed": false, "msg":
"[InvalidArgumentException] Could not find package islandora/openseadragon in
a version matching dev-paged-content-config require [--dev] [--prefer-source]
[--prefer-dist] [--no-progress] [--no-suggest] [--no-update] [--no-scripts]
[--update-no-dev] [--update-with-dependencies]
[--update-with-all-dependencies] [--ignore-platform-reqs] [--prefer-stable]
[--prefer-lowest] [--sort-packages] [-o|--optimize-autoloader]
[-a|--classmap-authoritative] [--apcu-autoloader] [--] [<packages>]...",
"stdout": "\n \n  [InvalidArgumentException]                                  
                                  \n  Could not find package
islandora/openseadragon in a version matching dev-paged-content-config  \n    
                                                                              
             \n\nrequire [--dev] [--prefer-source] [--prefer-dist]
[--no-progress] [--no-suggest] [--no-update] [--no-scripts] [--update-no-dev]
[--update-with-dependencies] [--update-with-all-dependencies]
[--ignore-platform-reqs] [--prefer-stable] [--prefer-lowest] [--sort-packages]
[-o|--optimize-autoloader] [-a|--classmap-authoritative] [--apcu-autoloader]
[--] [<packages>]...\n\n", "stdout_lines": ["", "                             
                                                                   ", " 
[InvalidArgumentException]                                                    
                ", "  Could not find package islandora/openseadragon in a
version matching dev-paged-content-config  ", "                               
                                                                 ", "",
"require [--dev] [--prefer-source] [--prefer-dist] [--no-progress]
[--no-suggest] [--no-update] [--no-scripts] [--update-no-dev]
[--update-with-dependencies] [--update-with-all-dependencies]
[--ignore-platform-reqs] [--prefer-stable] [--prefer-lowest] [--sort-packages]
[-o|--optimize-autoloader] [-a|--classmap-authoritative] [--apcu-autoloader]
[--] [<packages>]...", ""]} 

@kayakr
Copy link
Contributor

kayakr commented Oct 1, 2019

Specifying the specific commit openseadragon_composer_item: "islandora/openseadragon:dev-paged-content-config#fd8a775998b988d90d7e52457103c62e8e7063fc" in webserver/drupal.yml allowed install to complete.

@Natkeeran
Copy link
Contributor

Running into the following issue when trying to vagrant up this PR:

TASK [Islandora-Devops.alpaca : Add Alpaca Repos] ******************************
Tuesday 01 October 2019  12:36:42 -0400 (0:00:00.361)       0:07:02.420 ******* 
failed: [default] (item=mvn:org.apache.camel.karaf/apache-camel/2.20.4/xml/features) => {"ansible_loop_var": "item", "changed": false, "item": "mvn:org.apache.camel.karaf/apache-camel/2.20.4/xml/features", "msg": "client: Ignoring predefined value for KARAF_HOME\n"}
failed: [default] (item=mvn:org.apache.activemq/activemq-karaf/5.15.0/xml/features) => {"ansible_loop_var": "item", "changed": false, "item": "mvn:org.apache.activemq/activemq-karaf/5.15.0/xml/features", "msg": "client: Ignoring predefined value for KARAF_HOME\n"}
failed: [default] (item=mvn:ca.islandora.alpaca/islandora-karaf/1.0.1/xml/features) => {"ansible_loop_var": "item", "changed": false, "item": "mvn:ca.islandora.alpaca/islandora-karaf/1.0.1/xml/features", "msg": "client: Ignoring predefined value for KARAF_HOME\n"}

@dannylamb
Copy link
Member Author

Just successfully spun up a box with these latest changes. This pulls in the newest theme, which fixes an issue when placing blocks with contexts that @whikloj found.

Folks can give it a try.

@seth-shaw-unlv
Copy link
Contributor

Just creating a single item in the new theme caused a bunch of log messages:
Screen Shot 2019-10-01 at 1 07 09 PM

And it looks like we need to handle an empty IIIF manifest a bit more cleanly:

Warning: Invalid argument supplied for foreach() in Drupal\openseadragon\IIIFManifestParser->getTileSources() (line 96 of /var/www/html/drupal/web/modules/contrib/openseadragon/src/IIIFManifestParser.php)
#0 /var/www/html/drupal/web/core/includes/bootstrap.inc(587): _drupal_error_handler_real(2, 'Invalid argumen...', '/var/www/html/d...', 96, Array) #1 /var/www/html/drupal/web/modules/contrib/openseadragon/src/IIIFManifestParser.php(96): _drupal_error_handler(2, 'Invalid argumen...', '/var/www/html/d...', 96, Array) 
#2 /var/www/html/drupal/web/modules/contrib/openseadragon/openseadragon.module(139): Drupal\openseadragon\IIIFManifestParser->getTileSources('http://localhos...') 
#3 /var/www/html/drupal/web/core/lib/Drupal/Core/Theme/ThemeManager.php(287): template_preprocess_openseadragon_iiif_manifest_block(Array, 'openseadragon_i...', Array)

@elizoller
Copy link
Member

I got similar error messages to seth but as i went further into the test i got an additional one: EntityViewMode user.open_seadragon does not exist. View mode cannot be altered.
although the viewer and reordering of pages did actually work

@dannylamb
Copy link
Member Author

The theme update fixed a somewhat edge-y case, so we can always revert if it's problematic and leave it as a 'known issue'. But if you're developing with caching disabled, it's fixes something pretty annoying.

@elizoller That one isn't related to the theme. It shows up a lot anyway. I think I just need to lower the level of that message.

@seth-shaw-unlv I think the manifest is ok and i'm not parsing it correctly. I'll look into it. I've made a paged content node and then been redirected to it after submitting the form so i'm surprised i haven't seen that one yet.

@dannylamb
Copy link
Member Author

Oh noes... when testing the fix for @seth-shaw-unlv's empty paged content issue, I ran into a new one. Make two books. The block gets cached. Visit one and then the other. The second block is the same as for the first object. Gotta get that caching sorted out 😭

@dannylamb
Copy link
Member Author

Alright, I pushed up some changes to openseadragon to do better caching. Now you don't have to clear your caches all the time!

It also works if you have more than one 'paged content' object now. Good thing we caught that before the conference ^_^

I'm vagrant up'ing now to confirm. But this should be good to test one more time.

@dannylamb
Copy link
Member Author

Let me know if you want me to revert the theme changes. Easy enough to do.

@whikloj
Copy link
Member

whikloj commented Oct 2, 2019

Of course I started my build 20 minutes ago, or 15 minutes before the last change.

✊ LAMB!!!!

@whikloj
Copy link
Member

whikloj commented Oct 2, 2019

Ok, I built this except for the last commit. I saw the error that you get if you have not set the IIIF Manifest. But after I set it, it does work.

The IIIF Manifest box has waaaaaay too many choices, but we can review that later.
Screen Shot 2019-10-02 at 12 26 35 PM

👍

@dannylamb
Copy link
Member Author

In theory we can filter that list by views that use the iiif manifest views style plugin. Would def cut the size of that list down.

@whikloj
Copy link
Member

whikloj commented Oct 2, 2019

Yeah, or it would be good to more clearly define that this is a list of views and you are supposed to select one that gives the IIIF Manifest for a resource. Also that the default provided is IIIF Manifest

@dannylamb
Copy link
Member Author

@whikloj Everything's been reverted to point to 8.x branches. This is ready to go.

@whikloj
Copy link
Member

whikloj commented Oct 3, 2019

Testing one last time.

@dannylamb
Copy link
Member Author

Mine just spun up successfully 🤞

Copy link
Member

@whikloj whikloj left a comment

Choose a reason for hiding this comment

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

👍

@whikloj whikloj merged commit 1d1ffc2 into Islandora-Devops:dev Oct 3, 2019
whikloj pushed a commit to whikloj/claw-playbook that referenced this pull request Dec 18, 2019
* Paged content config

* Setting up sequence mode and the reference strip

* Testing autodetecting sequence mode

* Missed an inventory var

* Using new theme with better dependencies

* Setting iiif view in post-install.yml for better caching

* Pointing back to 8.x branches

* Update drupal.yml
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.

8 participants