-
Notifications
You must be signed in to change notification settings - Fork 14
Configurable Layout/UI Refactor #226
Changes from 22 commits
ca61374
2cc012c
56949c2
db9f32a
d7e83e9
3f51bf5
c25e91f
b178585
705fa04
c446add
5109c5f
c851d69
aaa4337
906fff4
3fadaba
922b531
f516b46
46fbf5a
b1270a2
5d96c13
587fa0b
7f64501
93d3e42
0a81d58
2d53e75
7554e6f
d85ffb0
59c2f85
878c072
f124a08
502d58d
650a718
aa8a812
28d1cda
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,5 +6,6 @@ Developer Documentation | |
|
||
installation | ||
api-documentation | ||
extending-minerva | ||
creating-a-source | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
Extending Minerva | ||
================= | ||
|
||
Creating a Minerva Plugin | ||
~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
Minerva plugins are **identical** to Girder plugins with the exception that they have a hard dependency on Minerva. This ensures that Minerva will be loaded before your plugin is. | ||
|
||
Minerva utilizes the Girder plugin system, so it will be worthwhile to familiarize yourself with their section on `Plugin Development <http://girder.readthedocs.org/en/latest/plugin-development.html>`_. | ||
|
||
|
||
Below is an example configuration for a Minerva plugin, note the dependency on Minerva: | ||
|
||
.. code-block:: python | ||
|
||
{ | ||
"name": "My Minerva Plugin", | ||
"dependencies": ["minerva"] | ||
} | ||
|
||
Your new Minerva plugin should provide no new functionality at this point, and `can be enabled through the administration console <http://girder.readthedocs.org/en/latest/installation.html#initial-setup>`_. | ||
|
||
|
||
Extending the look and feel of Minerva | ||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
||
While Minerva plugins follow the guidelines provided by Girder's client side, there are additional concerns when writing for Minerva. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or we can go with "Minerva plugins" as here, we just need to define it more explicitly above. |
||
|
||
.. note:: Girder has documentation on `extending the client-side application <http://girder.readthedocs.org/en/latest/plugin-development.html#extending-the-client-side-application>`_ which also applies to Minerva. | ||
|
||
Minerva Panel Views | ||
------------------- | ||
Minerva renders panels in a particular way, as a result there are some guidelines that need be followed when creating your own Panels: | ||
|
||
- Panels should never call render within their initialize function | ||
- Panel views must extend the base Panel view | ||
- Panel views need to extend their events to take advantage of collapsible, removable, configurable panels | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this may need to be updated, I'll mention something related in a bit |
||
|
||
|
||
Configure Minerva's layout | ||
-------------------------- | ||
Taking our example plugin from before, we can alter how Minerva displays different panels. | ||
|
||
Let's pretend our use case of Minerva deems the Jobs Panel useless, and is more focused on the datasets available. In this case one might want to disable the Jobs Panel entirely, and move the datasets panel to the top. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the idea of the "fake use case/example", maybe you could set that up a bit more clearly from the start and follow it all the way through. Could always be done in a later PR though. |
||
|
||
From your plugin root, create a JavaScript file at ``web_client/js/some-file.js`` and add the following code: | ||
|
||
.. code-block:: javascript | ||
|
||
girder.events.once('m:pre-render-panel-groups', function (sessionView) { | ||
var leftPanelGroup = sessionView.getPanelGroup('m-left-panel-group'); | ||
|
||
// Disable/remove the jobs panel | ||
sessionView.disablePanel('m-jobs-panel'); | ||
|
||
// Move the 'Available Datasets' panel to the top | ||
leftPanelGroup.panelViews.sort(function (a, b) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could we pass a sort function to the panelgroup instead?
|
||
if (a.id === 'm-data-panel') { | ||
return -1; | ||
} else if (b.id === 'm-data-panel') { | ||
return 1; | ||
} else { | ||
return 0; | ||
} | ||
}); | ||
}); | ||
|
||
Above we utilize the ``m:pre-render-panel-groups`` event to hook into Minerva before any panels are actually rendered, this gives full control over what the final layout looks like. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ | |
return name + 'Widget'; | ||
}; | ||
|
||
minerva.views.AnalysisPanel = minerva.View.extend({ | ||
minerva.views.AnalysisPanel = minerva.views.Panel.extend({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we are asking panels to extend 'minerva.views.Panel' instead of 'minerva.View' now, can you make the below happen automatically in Panel.js, and then specific panel extensions wouldn't need to do it?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe I'm misunderstanding, but if this were to happen it would have to be in the Panel.js initialize, which would mean extending panels would need to call parent initialize in their initialize. Is that what you're talking about? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right, and I think what you've done already is cleanest. Can you add the line
to the Extending Minerva docs for greater explicitness? |
||
|
||
events: { | ||
'click .m-attempt-analysis': 'attemptAnalysis' | ||
|
@@ -57,9 +57,10 @@ | |
}, | ||
|
||
initialize: function (settings) { | ||
this.collection = settings.collection; | ||
this.datasetCollection = settings.datasetCollection; | ||
this.sourceCollection = settings.sourceCollection; | ||
_.extend(this.events, minerva.views.Panel.prototype.events); | ||
this.collection = settings.session.analysisCollection; | ||
this.datasetCollection = settings.session.datasetsCollection; | ||
this.sourceCollection = settings.session.sourceCollection; | ||
this.listenTo(this.collection, 'g:changed', function () { | ||
console.log('AP g:changed'); | ||
this.render(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
minerva.views.MapPanel = minerva.View.extend({ | ||
minerva.views.MapPanel = minerva.views.Panel.extend({ | ||
|
||
events: { | ||
'click .m-save-current-baselayer': function () { | ||
|
@@ -110,6 +110,7 @@ minerva.views.MapPanel = minerva.View.extend({ | |
this.featureInfoWidget.callInfo(0, evt.geo); | ||
}); | ||
} | ||
this.uiLayer = this.map.createLayer('ui'); | ||
this.map.draw(); | ||
} else { | ||
// Assume the dataset provides a reader, so load the data | ||
|
@@ -127,7 +128,6 @@ minerva.views.MapPanel = minerva.View.extend({ | |
reader.read(dataset.fileData, _.bind(function () { | ||
// Add the UI slider back | ||
this.uiLayer = this.map.createLayer('ui'); | ||
this.uiLayer.createWidget('slider'); | ||
this.map.draw(); | ||
}, this)); | ||
}, this); | ||
|
@@ -161,7 +161,8 @@ minerva.views.MapPanel = minerva.View.extend({ | |
}, | ||
|
||
initialize: function (settings) { | ||
this.session = settings.session; | ||
_.extend(this.events, minerva.views.Panel.prototype.events); | ||
this.session = settings.session.model; | ||
this.listenTo(this.session, 'm:mapUpdated', function () { | ||
// TODO for now only dealing with center | ||
if (this.map) { | ||
|
@@ -172,7 +173,7 @@ minerva.views.MapPanel = minerva.View.extend({ | |
this.datasetLayers = {}; | ||
this.legendWidget = {}; | ||
|
||
this.collection = settings.collection; | ||
this.collection = settings.session.datasetsCollection; | ||
this.listenTo(this.collection, 'change:displayed', function (dataset) { | ||
// There is a slight danger of a user trying to add a dataset | ||
// to a session while the map is not yet created. If the map isn't | ||
|
@@ -214,9 +215,10 @@ minerva.views.MapPanel = minerva.View.extend({ | |
} | ||
}) | ||
}); | ||
this.map.createLayer(this.session.sessionJsonContents.basemap); | ||
this.map.createLayer(this.session.sessionJsonContents.basemap, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make an API on the session model rather than mucking with the sessionJsonContents directly There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is essentially what my comment on #225 addresses. I'll change it if you want, but I'm only doing retrieval here and it is identical to how retrieval of session data is done throughout the codebase. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leave it for that later change. |
||
_.has(this.session.sessionJsonContents, 'basemap_args') ? | ||
this.session.sessionJsonContents.basemap_args : {}); | ||
this.uiLayer = this.map.createLayer('ui'); | ||
this.uiLayer.createWidget('slider'); | ||
this.mapCreated = true; | ||
_.each(this.collection.models, function (dataset) { | ||
if (dataset.get('displayed')) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
minerva.views.Panel = minerva.View.extend({ | ||
/** | ||
* The panel view isn't meant to be instantiated on it's own. | ||
**/ | ||
events: { | ||
'click .icon-cancel': 'removePanel', | ||
'show.bs.collapse': 'expandPanel', | ||
'hide.bs.collapse': 'collapsePanel' | ||
}, | ||
|
||
getSessionView: function () { | ||
return this.parentView.parentView; | ||
}, | ||
|
||
getSessionModel: function () { | ||
return this.getSessionView().model; | ||
}, | ||
|
||
/** | ||
* Upon confirmation, physically removes the DOM element while | ||
* storing the change in the session attributes. This also | ||
* enables the save button. | ||
**/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I take it there is no way to add the panel back after removing it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope, unless you were to manually edit the session.json. It seems like having a dropdown with available panels is something @aashish24 wants at some point - so that could add that functionality. |
||
removePanel: function () { | ||
if (confirm('Are you sure you want to remove this panel?')) { | ||
this.getSessionView().disablePanel(this.el.id); | ||
this.getSessionView()._enableSave(); | ||
this.remove(); | ||
minerva.View.prototype.remove.call(this); | ||
} | ||
}, | ||
|
||
/** | ||
* Handles the aftermath of a panel being expanded. Meaning it changes the | ||
* expand/collapse icon, marks the state of the panel in the session attributes, | ||
* and enables the user to save the change. | ||
**/ | ||
expandPanel: function (e) { | ||
$(e.currentTarget).find('i.icon-down-open').attr('class', 'icon-up-open'); | ||
this.getSessionModel().addLayoutAttributes(this.el.id, { | ||
collapsed: false | ||
}); | ||
this.getSessionView()._enableSave(); | ||
}, | ||
|
||
collapsePanel: function (e) { | ||
$(e.currentTarget).find('i.icon-up-open').attr('class', 'icon-down-open'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably beating a 💀 🐴 here, similar to this comment, what would be nice is if the three methods
would trigger a change on the layout of the session model, and then the session view can be listening for changes on the session model which would re-render the panels and also enable the save button at the appropriate times. All changes for #225. |
||
this.getSessionModel().addLayoutAttributes(this.el.id, { | ||
collapsed: true | ||
}); | ||
this.getSessionView()._enableSave(); | ||
} | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
minerva.views.PanelGroup = minerva.View.extend({ | ||
initialize: function (settings) { | ||
this.id = settings.id; | ||
this.panelViews = settings.panelViews || []; | ||
}, | ||
|
||
render: function () { | ||
// Render each of our panels | ||
_.each(this.panelViews, function (panelViewSpec) { | ||
var panelView = new panelViewSpec.view({ | ||
parentView: this, | ||
session: this.parentView | ||
}); | ||
|
||
this.$el.append('<div id="' + panelViewSpec.id + '"></div>'); | ||
panelView.setElement(this.$('#' + panelViewSpec.id)).render(); | ||
}, this); | ||
|
||
return this; | ||
} | ||
}); |
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.
I'm a little confused by this whole section. Maybe we can define what a "Minerva Plugin" is?
A bit later you say "Minerva specific plugins", so is that different from a "Minerva Plugin"?
I think because some of this stuff is tricky, it is better to err on the side of being overly explicit.
Maybe the title can be "Creating a Minerva plugin through the Girder plugin system"?
and then
"For Minerva specific plugins" => "For Girder plugins intended to extend the functionality of Minerva"?