-
Notifications
You must be signed in to change notification settings - Fork 9
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
Make the mirador config more easily alterable, Drupal-side. #22
Conversation
Object.entries(settings.mirador.viewers).forEach(entry => { | ||
const [base, values] = entry; | ||
once('mirador-viewer', base, context).forEach(() => | ||
Mirador.viewer(values, window.miradorPlugins || {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The window.miradorPlugins
thing here seems like we could have saner expectations? Like, have the integration itself include it itself, so there's not this arbitrary JS extension point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With something like roblib/mirador-integration-islandora#1, could possibly avoid having to touch window.miradorPlugins
here? Could just become:
Mirador.viewer(values, window.miradorPlugins || {}) | |
Mirador.viewer(values) |
... apparently, is strict about the keys being numeric.
This looks fine here to me but given the in-flight work being undertook in the community I am hesitant to merge this prior to having more sets of eyes on this, primarily in regards to the |
Just noticed that this was no longer marked as Draft. I tested it and the plugins and everything still works as expected so this looks good. |
What does this Pull Request do?
Moves building up the "object" of Mirador viewer config PHP-side, so we can alter/adjust/augment it more easily, minimizing the size of the actual Javascript "integration" piece required.
Related GitHub Issue: (link)
Other Relevant Links: (Google Groups discussion, related pull requests,
Release pull requests, etc.)
What's new?
A in-depth description of the changes made by this PR. Technical details and
possible side effects.
(i.e. Regeneration activity, etc.)?
How should this be tested?
A description of what steps someone could take to:
Documentation Status
Additional Notes:
Any additional information that you think would be helpful when reviewing this
PR.
Interested parties
Tag (@ mention) interested parties or, if unsure, @Islandora/committers