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

[399] Fix build by integrating with Mirador 3 + Image Tools Plugin #402

Merged
merged 8 commits into from
Feb 17, 2022

Conversation

jsavell
Copy link
Member

@jsavell jsavell commented Feb 16, 2022

This PR resolves #399 by manually building Mirador 3 with grunt-webpack to enable packaging the image tools plugin and potentially other future plugins.

This required the implementation of a wrapper object to provide access to Mirador's state while in the context of the angularjs app.

The destroy method in particular is needed to properly unmount the Mirador instance when the contentViewer directive is unloaded by angularjs. This is necessary to prevent rendering issues if a user navigates to another Mirador based discovery view.

This solution appears to be working without issue, but I couldn't find any documented examples of dynamically generating and destroying Mirador instances. The 'proper' use of Mirador 3 seems to be loading one instance and then using redux actions to update it as needed.

We are using redux actions to manipulate the state of the Mirador instance, but due to angularjs's dom element manipulation, we can't sanely preserve a single instance across user navigation.

This wrapper does provide a good starting point for a more generalized component that's not specific to SAGE.

@jsavell jsavell changed the title [399] Fix build by integratig with Mirador 3 + Image Tools Plugin [399] Fix build by integrating with Mirador 3 + Image Tools Plugin Feb 16, 2022
@coveralls
Copy link

coveralls commented Feb 16, 2022

Coverage Status

Coverage decreased (-0.07%) to 45.567% when pulling 023796c on 399-mirador3-webpack into 280ec1d on master.

@jsavell jsavell requested review from a user and jeremythuff February 16, 2022 22:38
ghost
ghost previously approved these changes Feb 17, 2022
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Looks good for code review. Can we get this deployed to dev for testing?

@jsavell
Copy link
Member Author

jsavell commented Feb 17, 2022

I switched back to using Mirador.viewer to generate the Mirador instance. I had needed to bypass that at one point for an earlier approach that didn't pan out.

@jmicah jmicah merged commit d27ad0f into master Feb 17, 2022
@jmicah jmicah deleted the 399-mirador3-webpack branch February 17, 2022 19:36
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.

NPM build is failing due to Mirador 2 dependency
3 participants