search

Islandora / openseadragon

Drupal Field Formatter for OpenSeadragon
https://www.drupal.org/project/openseadragon
GNU General Public License v2.0
3 stars 15 forks source link

Cdn #26

Closed dannylamb closed 4 years ago

dannylamb commented 4 years ago

GitHub Issue:

What does this Pull Request do?

Uses a CDN for the openseadragon JS and images, which simplifies deployment. One less step to get the module up and running.

What's new?

Changed the library definition so that it points to a CDN for openseadragon. Plus changed the prefixUrl config for the JS to use the CDN as well, so the button images for the player are loaded over https.

How should this be tested?

Additional Notes:

This started out as me wanting to ship openseadragon with composer, but there's no "community way" to do that yet, and it involves a lot of consistency across root composer files for all Islandora sites using openseadragon. You can't just do it from the module's composer.json. So I did this. Maybe it's useful to someone?

Interested parties

@Islandora/8-x-committers

dannylamb commented 4 years ago

If folks don't have objections to this approach, I'll update the ansible and ISLE stuff appropriately afterwards :fire: :fire: :fire:

elizoller commented 4 years ago

@dannylamb do you still need a reviewer on this?

dannylamb commented 4 years ago

There's still work to do here. I've got to figure out how to programatically get the version from the library config. I made an attempt a week or so ago and it thwarted me.

nigelgbanks commented 4 years ago

This fixes the problem I've had with displaying openseadragon on subsites! :+1:

elizoller commented 4 years ago

@dannylamb i just tested this and it worked wonders for an issue i was having with multisite - is it waiting for anything or can i go ahead and review + merge?

dannylamb commented 4 years ago

Let's move it in and we'll do a follow up to fix it. Right now, if you want to update the library, you'll have to touch up 3 or 4 spots. I could figure out how to make it a single variable you have to touch up, but it gave me grief last i tried.

dannylamb commented 4 years ago

Ah, @bseeger ++!!!!

dannylamb commented 4 years ago

Love it when i'm ninja'd by a merge!