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

Issue #26: Fix 'Use local library' option. #29

Merged
merged 1 commit into from
Aug 9, 2023
Merged

Conversation

alxp
Copy link
Contributor

@alxp alxp commented Jul 16, 2023

GitHub Issue: #26

What does this Pull Request do?

Fix broken option to use a local instance of Mirador Integration Islandora.

What's new?

  1. Add a more helpful description to the use local library option on the Islandora Mirador admin settings page.
  2. Fix the library info alter hook to work correctly and not just accidentally.
  • Does this change add any new dependencies? No
  • Does this change require any other modifications to be made to the repository
    (i.e. Regeneration activity, etc.)? No
  • Could this change impact execution of existing code? No

How should this be tested?

  1. Alone the Mirador Integration Islandora app.
  2. Follow the instructions to compile the Webpack output.
    2.1. $ npm install
    2.2. $ npm run web pack
    2.3 $ cp web pack/dist/main.js [drupal root]/web/libraries/mirador/dist #create if needed
  3. Go to /admin/config/media/mirador, choose 'Local' in the library location setting and save.
  4. Clear your cache and load a page with Mirador on it.
  5. In web inspector, check the Network tab for where 'main.js' is retrieved from . It should say your local site and not GitHub.
    5.1. Alternatively, rename the local dist/main.js to something random, clear caches and reload. The Mirador viewer should no longer show up.

Documentation Status

  • Does this change existing behaviour that's currently documented? No
  • Does this change require new pages or sections of documentation? No
  • Who does this need to be documented for? Developers
  • Associated documentation pull request(s): ___ or documentation issue ___ To come.

Additional Notes:

Any additional information that you think would be helpful when reviewing this

Interested parties

Tag (@ mention) interested parties or, if unsure, @Islandora/committers

@rosiel

@alxp alxp marked this pull request as draft July 16, 2023 00:45
@seth-shaw-asu seth-shaw-asu requested a review from rosiel August 2, 2023 17:34
@alxp alxp marked this pull request as ready for review August 6, 2023 19:28
@alxp
Copy link
Contributor Author

alxp commented Aug 6, 2023

The mirador-integration-islandora app on Roblib's repository has been updated with newer Webpack dependency so it compiles again.

This PR can now be properly tested so I'm marking it as ready for review.

@rosiel
Copy link
Member

rosiel commented Aug 7, 2023

I am still seeing GET https://roblib.github.io/mirador-integration-islandora/islandora-mirador-0.1.0.js. Perhaps my setup isn't correct. I have checked "Local library..." in Mirador settings.

@rosiel
Copy link
Member

rosiel commented Aug 7, 2023

From debugging, it shows that in line 97 of islandora_mirador.module, setting the value of $libraries['mirador']['js']['/libraries/mirador/dist/main.js'] causes `$libraries['mirador']['js'] to have 2 values, as the old value (roblib.github.io...) isn't being deleted.

@rosiel rosiel merged commit 5aa8802 into 2.x Aug 9, 2023
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.

2 participants