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

Add NLCD code to analyze land results csv #2176

Merged
merged 2 commits into from
Aug 23, 2017
Merged

Conversation

kellyi
Copy link
Contributor

@kellyi kellyi commented Aug 22, 2017

Overview

This PR adds an "NLCD Code" column to the exported CSV, but not to the visible analyze results table. We accomplish this by adding a hidden column to the table which is registered to be exported along with the CSV.

Since both the land and soils tab use the TableView component, the changes here also include some checks for whether or not to add the hidden column. If we decide we want to show a soil code for that table, we can drop the checks in the template and instead pick the value and header text conditionally.

Connects #2127

Demo

screen shot 2017-08-22 at 3 01 41 pm

->

screen shot 2017-08-22 at 3 02 15 pm

Testing Instructions

  • get this branch, then vagrant up and bundle
  • draw an AOI and wait for the analysis results to complete. Once they're back check that the tabs all render as before and that all the csv exports work as before -- aside from the land export, which now includes a "NLCD Code" column in its csv

- add hidden yet exportable column to TableView and TableRowView added
only when the table displays NLCD results
Copy link
Member

@caseycesari caseycesari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. Tested. We may have to rework this if there are more tables that need to have hidden columns, but this looks good for now.

@@ -2,6 +2,9 @@
<thead>
<tr>
<th data-sortable="true">Type</th>
{% if isLandTable %}
<th class="hidden-analyze-table-column" data-tableexport-display="always">NLCD Code</th>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider dropping this class and using the HTML hidden attribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried this out & it seemed like there's some quirk getting the hidden attribute to take hold via the Nunjucks template.

Going to leave this as is for now, but we should try it again if we end up adjusting these to include more hidden columns.

@caseycesari caseycesari assigned kellyi and unassigned caseycesari Aug 23, 2017
@kellyi
Copy link
Contributor Author

kellyi commented Aug 23, 2017

Thanks for taking a look at this!

@kellyi kellyi merged commit eed00f3 into develop Aug 23, 2017
@kellyi kellyi deleted the ki/add-nlcd-id-to-csv branch August 23, 2017 17:33
@rajadain rajadain mentioned this pull request Oct 16, 2017
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

Successfully merging this pull request may close these issues.

None yet

3 participants