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

Widgets should be (more) self contained #107

Open
azaroth42 opened this issue Feb 12, 2014 · 4 comments
Open

Widgets should be (more) self contained #107

azaroth42 opened this issue Feb 12, 2014 · 4 comments
Milestone

Comments

@azaroth42
Copy link
Contributor

When starting to update my local, modified mirador to the latest github version, the interdependence of different files becomes apparent. It would be advantageous (IMO) to reduce the dependencies, such that developers have to touch as few files as possible in the core mirador code to add additional widgets.

As the project progresses, a widget library of contributed code will likely build up, and this should be kept as easy to maintain as possible.

For example:

  • templates.js could be exploded out to the files that otherwise call it rather than keeping all of the templates together. This would mean that imageView.js maintains its own templates, and newWidget.js can thus look after its templates.
  • If every widget had the same invocation, even if it never used the extra information, then widget.js render*View functions could be reduced to a single render() function.
  • abstract features to a configuration file that can be kept separate from the core. Including the function to call when the load window canvas item is clicked, the features in mirador.js that aren't widget dependent, etc.
  • A standard navbar builder would be useful rather than having to always use a template. Perhaps either included or excluded widgets (other than itself) could be an argument.
  • etc :)

Rob

@azaroth42
Copy link
Contributor Author

So the widget.js fix is actually really trivial, based on the same trick to avoid the case block:

    render: function() {
      var className = this.type[0].toUpperCase() + this.type.substring(1, 1000);
      this.viewObj = new $[className]({
        // standard initialization, not everything will be populated
        imagesList: this.imagesList,
        manifestId: this.manifestId,
        element: this.content.element,
        imageId: this.imageId,
        openAt: this.openAt,
        parent: this
      });
      this.viewObj.render();
    },

@jchristo4 jchristo4 self-assigned this Feb 13, 2014
@jchristo4
Copy link
Contributor

@azaroth42 Makes sense. I'll add these changes to my list of custom widget improvements. Ideally, creating a new widget/view should be a one step process - dropping .js in the /js/src/ directory.

@azaroth42
Copy link
Contributor Author

And in viewer.js:

    loadView: function(type, manifestId, imageId, openAt) {
      $.viewer.addWidget({
        height:     $.DEFAULT_SETTINGS[type].height,
        manifestId: manifestId,
        openAt:     openAt,
        imageId:    imageId,
        type:       type,
        width:      $.DEFAULT_SETTINGS[type].width
      });
    },

Plus then the more tedious work of replacing all the loadFooView() functions with loadView("foo", ...)

I'll rebase, fix and issue a pull request

@azaroth42
Copy link
Contributor Author

Thanks for accepting the pull :)

Proposal: fooView.js becomes responsible for updating $.DEFAULT_SETTINGS with the information for itself.

eg at the beginning of the function($) {, it would do...

  $.DEFAULT_SETTINGS.availableViews.imageView = {'label': 'Image View'};
  $.DEFAULT_SETTINGS.imageView = {
    'height': 600,
    'width': 350,
    'annotationsList': {
      'display':true,
      'width': 200
    }
  };

Also, at the same time, can we compact that down to just one object rather than two by putting all of the info into availableViews.fooView?

Happy to do the legwork if the proposal is accepted.

@snydman snydman added this to the Icebox milestone Mar 19, 2014
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