-
Notifications
You must be signed in to change notification settings - Fork 31
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
PA DEP: Add Front-end Button to Download Worksheet #3216
Conversation
The easiest way to download a file as the result of a request is to submit a form in a new page, rather than AJAX, due to all sorts of browser security issues. This is the same technique we've used in the past with downloading shapefiles, as done in ab15bbc.
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.
Left some comments on the changeset which may be helpful.
@@ -216,7 +216,7 @@ | |||
border-radius: $radius-sm; | |||
padding: 1rem; | |||
|
|||
> .analyze-model-package-link { | |||
> a { |
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.
This was changed because each analyze-model-package-link
is handled by the JavaScript to create a project with the given model package, but we want a button that looks like that but doesn't work like that. Hence the switch to a generic a
tag.
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 ran through a number of scenarios to test and was able to generate the appropriate worksheets each time (multi and single watersheds, drawn and uploaded areas, various parts of the country). I'm curious about the restriction on wkids, is this a requested limitation or is it because of an implementation detail of how we fetch the shapes in that case? Primarily concerned if that's going to be ok.
In terms of UI additions to indicate that the function is not active in wkid case - will create an issue to complete that?
Theoretically, the smaller urban area being analyzed is done so in the context of an encompassing HUC. It will almost always be smaller than a HUC-12. There may be some rare cases when a HUC-10 is needed, and BME conceded that those can be handled manually by the previously existing workflow. I tried removing the WKAOI check: diff --git a/src/mmw/js/src/analyze/models.js b/src/mmw/js/src/analyze/models.js
index b843a5a0..179da155 100644
--- a/src/mmw/js/src/analyze/models.js
+++ b/src/mmw/js/src/analyze/models.js
@@ -28,7 +28,6 @@ var WorksheetModel = coreModels.TaskModel.extend({
name: 'worksheet',
displayName: 'Worksheet',
area_of_interest: null,
- wkaoi: null,
taskName: 'modeling/worksheet',
taskType: 'api',
token: settings.get('api_token'),
@@ -38,12 +37,10 @@ var WorksheetModel = coreModels.TaskModel.extend({
runWorksheetAnalysis: function() {
var self = this,
aoi = self.get('area_of_interest'),
- wkaoi = self.get('wkaoi'),
result = self.get('result');
if (aoi &&
!result &&
- !utils.isWKAoIValid(wkaoi) &&
self.runWorksheetPromise === undefined) {
var taskHelper = {
contentType: 'application/json', and improved the error display in the UI in 0ec3024, which resulted in this error for a HUC-12: I don't recall seeing that before, and don't have a clear understanding of what is a |
Just made an issue for updating the Select Area text: #3217. In previous discussions with BME the idea of covering that detail in the technical documentation was floated, because the actual explanation can be quite verbose and not a good fit for the limited real estate in the sidebar. |
In 4dc40f0 I've added a check for WKAOIs that makes the Worksheet Export button only visible for non-WKAOI shapes: |
For this I was considering the other types of wkids, like County. I'm not sure if that is important or not, I was mostly just trying to surface whether it has been determined to be important or not yet. |
The above error message implementation is in b394717. |
I think that looks good 👍 |
Added the proposed text changes in a2581d8: |
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.
Looks good to me 👍
Thanks for the thorough review. Going to squash and merge now. |
A button is added to the Model tab of the Analyze section. Visually, there is a dividing line between this new button and the two existing modeling buttons. When the button is clicked, we send a request to the initial data gathering endpoint and poll for results. A "Gathering data" message is shown beneath the button with a spinner. The button itself is disabled, so it cannot be double clicked. When the polling finishes successfully, a "Download worksheet" message is shown beneath the button with a check mark, which is clickable. Clicking this submits a hidden form which had been populated with the results of the first endpoint, which opens a new tab and downloads the file. In case of error gathering data, an error message is shown. This process is divided into these two steps on the front-end because if we open a new tab or window programmatically as the effect of a non-user action (such as the first promise finishing), browsers treat that as a pop-up and block it by default. Rather than having users deal with that and manually add an exception for MMW, we have the user explicitly click some text to start the download process.
a2581d8
to
b9a25e7
Compare
Overview
A button is added to the Model tab of the Analyze section. Visually, there is a dividing line between this new button and the two existing modeling buttons.
When the button is clicked, we send a request to the initial data gathering endpoint and poll for results. A "Gathering data" message is shown beneath the button with a spinner. The button itself is disabled, so it cannot be double clicked.
When the polling finishes successfully, a "Download worksheet" message is shown beneath the button with a check mark, which is clickable. Clicking this submits a hidden form which had been populated with the results of the first endpoint, which opens a new tab and downloads the file.
In case of error gathering data, an error message is shown.
This process is divided into these two steps on the front-end because if we open a new tab or window programmatically as the effect of a non-user action (such as the first promise finishing), browsers treat that as a pop-up and block it by default. Rather than having users deal with that and manually add an exception for MMW, we have the user explicitly click some text to start the download process.
Connects #3195
Demo
Notes
Notably, this process does not work if the user selects a WKAOI. This is not effectively communicated in the front-end.
BME has given us some verbiage to add to the shape selection screen to that effect. I will add that as a separate commit shortly.
Testing Instructions
bundle --debug