Display subbasin results view via water quality tab link #2744
Conversation
kellyi
force-pushed
the
ki/add-hotspot-view-toggle
branch
6 times, most recently
from
3be522b
to
cae4d61
Compare
kellyi
force-pushed
the
ki/add-hotspot-view-toggle
branch
from
cae4d61
to
f47b8ca
Compare
kellyi
changed the title
|
Testing this out now! |
There was a problem hiding this comment.
Thanks for working this all out!
The most important things from the attached comments are:
- the
undefined modelerror when clicking the subbasin button - "Exit subbasin mode" taking you back to hydrology instead of water quality.
As a complete aside, the way you wired up the subbasin view toggling seems to be more akin to our react projects (passing a function bound to a parent view down the child views). This isn't a bad thing at all, but it might be worth noting some other ways MMW has accomplished the same thing:
- Similar to your implementation, listen in the parent for bubbled up method triggers. We do this in the modals, layer selector and some BiG-CZ views. It's kind of weird.
- Add a model to
ResultsDetailsViewthat has an attribute if it's showing the primary results or the secondary (subbasin) results. Put the model on another accessible model like the active scenario, or pass the model as an option down.
| }, | ||
|
|
||
| viewSubbasinResults: function() { | ||
| this.options.showSubbasinHotSpotView(); |
There was a problem hiding this comment.
Consider putting the function on this in initialize with the other options so its clear at a glance what options the view takes.
src/mmw/js/src/modeling/views.js
Outdated
| this.showHydrologyAndWaterQualityResults(); | ||
| }, | ||
|
|
||
| showHydrologyAndWaterQualityResults: function() { |
There was a problem hiding this comment.
Consider renaming to showPrimaryTabResults or something more generic. TR-55 has Runoff and Water Quality results, so we shouldn't make reference to Hydrology.
src/mmw/js/src/modeling/views.js
Outdated
|
|
||
| this.subbasinRegion.show(new gwlfeSubbasinViews.ResultView({ | ||
| hideSubbasinHotSpotView: this.hideSubbasinHotSpotView, | ||
| model: this.collection.findWhere({ name: 'subbasin' }), |
There was a problem hiding this comment.
The first time this button is clicked, the subbasin model hasn't run yet, so the model will be undefined in the result view and break.
There was a problem hiding this comment.
Good catch -- I removed the mock subbasin model (the stray "remove this" comment was supposed to go with it) since we currently don't persist the results.
src/mmw/js/src/modeling/views.js
Outdated
|
|
||
| showSubbasinHotSpotView: function() { | ||
| this.panelsRegion.empty(); | ||
| this.contentRegion.empty(); |
There was a problem hiding this comment.
When you click Exit subbasin attenuated results you should go back to the Water Quality view, not Hydrology. Is there a way to preserve the view or at the least the Water Quality active state?
There was a problem hiding this comment.
👍 Sounds good. I'll adjust this.
src/mmw/js/src/modeling/views.js
Outdated
| this.subbasinRegion.empty(); | ||
| this.showHydrologyAndWaterQualityResults(); | ||
|
|
||
| return this; |
There was a problem hiding this comment.
Is it necessary to return this in all these functions?
There was a problem hiding this comment.
This isn't necessary but if I remember correctly it's semi-common practice in Backbone/Marionette views which allows one to chain method calls together.
However, I don't think we really chain these now so I can remove.
src/mmw/js/src/modeling/views.js
Outdated
|
|
||
| // TODO: remove this! |
There was a problem hiding this comment.
Yep -- this was an artifact of setting up a subbasin model and including it in the other results. Removed the result but apparently forgot this one.
|
Pushed fixups to address most of the comments above. Still to come is starting on the water quality tab on clicking back from subbasin. As best I can tell the state of what tab we're on isn't really tracked anywhere except by adjusting an |
I don't think so. The tabs all manage their own business. Perhaps it is finally time for it to be a modal? |
|
6f34ce9 fixes the water quality thing by replacing the I noticed that due to something, I think, in the task model, it appears that polling will start on the main mapshed views at the point that the calculating results step starts with subbasin. Not sure what to do with this but I'm inclined to treat it as part of #2650 since I believe it'll require figuring out the exact task model for subbbasin. |
- enable displaying subbasin results view via a link on the water quality tab - empty subbasin view and return to hydrology/water quality tab view on clicking subbasin back button - add mock subbasin result to collection temporarily
- display subbasin panel and retrieve (but do not yet display) subbasin results on clicking the link for valid WKAOIs from the water quality tab
kellyi
force-pushed
the
ki/add-hotspot-view-toggle
branch
from
6f34ce9
to
bb2a080
Compare
|
Made an adjustment to only set the polling state true if we're in a non-subbasin task model. Also squashed and rebased on develop. @arottersman and I discussed designing the subbasin task/scenario/results models as part of a revised and larger version of #2650. |
|
Thanks for your help with this! |
Overview
This PR adds a link to MapShed water quality results for valid WKAOIs to kick off subbasin modeling, along with a new panel which will be used to display subbasin results in #2650.
In this PR we don't add the subbasin results to the new scenario/results/collection, but just log to the console that it's succeeded. However, we do keep the old mapshed results around to afford not having them replaced by the incoming subbasin results.
Connects #2647
Demo
... and meanwhile, in the console ...
Testing