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

Refactor MiradorViewer.js #2200

Closed
charbugs opened this issue Mar 15, 2019 · 2 comments
Closed

Refactor MiradorViewer.js #2200

charbugs opened this issue Mar 15, 2019 · 2 comments

Comments

@charbugs
Copy link
Collaborator

  • MiradorViewer is defined as a class, but it works like a function as it doesn't return a class instance but a plain object.

  • It's tightly coupled to a bunch of modules which makes testing difficult.

  • The respective unit test is actually an integration test.

@aeschylus
Copy link
Collaborator

aeschylus commented Mar 15, 2019

I agree this should be refactored, especially since the class isn't doing anything here, and the object literal is simpler and closer to what the language is actually doing (there are no actual classes in javascript, everything is an object instance, including functions):

Screen Shot 2019-03-15 at 09 53 50Screen Shot 2019-03-15 at 10 42 51

However, this touches on a bunch of different topics, so I want us to do more analysis here so we have a shared understanding of what the end result should look like. I'm trying to keep these various issues together in the Developer API Requirements ticket: #1693.

Let's talk more about what the outermost API should look like. So far we have the function, the ES modules, and the returned instance object/wrapper. Can we create some psuedocode for how the API or wrapper should behave?

Here is an example of how I imagine the react component API working (came up in #2197):
https://codesandbox.io/s/wkyw556xp7

Here's an example of how I imagine the function invocation API working in the browser from a global object (currently works as imagined, similar to Mirador 2):
https://codepen.io/anon/pen/VRPgxq
This syntax is necessary because it is the most accessible to the most developers, even those with very little experience or deep language knowledge who just want to use the viewer on a webpage. Very copy+paste-able and easy to modify without having to understand the syntax deeply.

What should the pseudocode be for non-react apps trying to embed the viewer? Or apps that just need to instantiate the viewer and then drive it through the event API? I think this will be a useful way to "design" the "UI" the developer uses before we attempt to implement a system that makes these interfaces possible.

@charbugs
Copy link
Collaborator Author

@aeschylus @mejackreed

Ok, let's wait with refactoring until we have a concept for this kind of external api. I closed #2203.

Here's an example of how I imagine the function invocation API working in the browser from a global object (currently works as imagined, similar to Mirador 2):
https://codepen.io/anon/pen/VRPgxq

Is this the correct link here? I only see a Mirador instantiation with config.

@cbeer cbeer closed this as completed Jun 14, 2021
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

No branches or pull requests

3 participants