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

Compare View Revamp: TR-55 Water Quality Charts and Table #2177

Merged

Conversation

rajadain
Copy link
Member

Overview

Adds Water Quality charts and table for TR-55. Also enables tab switching, and adds visualization for chart / table switching. Builds on the work done in #2148.

I talked to @jfrankl about how to show the many values of Water Quality in the app:

image

On his recommendation, we're showing only the Loading Rate (kg/ha) values, corresponding to the values shown in the charts. All the values are simple mathematical transformations of the same value, thus the trend seen in the charts would hold for all of them.

Connects #2076

Demo

2017-08-22 15 50 30

Testing Instructions

  • Check out this branch and bundle
  • Go to :8000/
  • Load a saved project, or make a new one. Ensure it has at least 2 scenarios.
  • Open the Compare View
  • Switch between table and chart modes. Ensure the buttons update correctly.
  • Switch between Runoff and Water Quality tabs. Ensure the contents and the tab buttons update correctly.
  • Ensure the numbers in the Water Quality tab are accurate.
  • Ensure that changing the precipitation slider updates all values in all tabs

@arottersman
Copy link

Finally taking a look!

@arottersman
Copy link

Taking a look at the code now. Testing went well except I think the rounding is making the loading rates all zero:

screen shot 2017-08-23 at 3 21 57 pm

@rajadain
Copy link
Member Author

The tooltips will be redone in #2158, so can be ignored for now.

@arottersman
Copy link

It's on the y-axis ticks as well.

@rajadain
Copy link
Member Author

It's on the y-axis ticks as well.

Ah, that's a real problem. Looking in to it.

.findWhere({ name: 'quality' })
.get('result');
}),
get = function(key) {

Choose a reason for hiding this comment

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

Could also name go

@arottersman
Copy link

The code for this looks good! Once the loading rates axis is fixed, we should be good to go.

@rajadain
Copy link
Member Author

Okay, so we have 0s in the charts because the values are too small, as small as 0.006 kg / ha:

image

There's two ways to solve this:

  1. Make the axis have three decimal places of precision
  2. Change the units from kg/ha to g/ha and multiply with 1000

I'm leaning towards the latter, but want some feedback /cc @ajrobbins. Suspended Solids will remain in kg/ha, but Nitrogen and Phosphorus may work better as g/ha. Not sure if we'll want to change the values in the table as well, to match the charts, or keep them as kg/ha to better align with the first row.

@rajadain
Copy link
Member Author

Just pushed a fix that makes it look like this:

image

image

@ajrobbins
Copy link

I think for comparison, we have to use the same units for all metrics on the page. Let's do three units of precision for now.

@rajadain rajadain force-pushed the tt/compare-view-revamp-tr55-water-quality-charts-table branch from 10054e0 to dbdcee8 Compare August 24, 2017 15:53
@rajadain
Copy link
Member Author

Added a commit that makes tick precision dynamic on the chart:

image

while maintaining the same units. Table is left as before, so same units all the way through. One final look @arottersman ?

Copy link

@arottersman arottersman left a comment

Choose a reason for hiding this comment

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

+1, ticks are fixed! I dig the one-line functions. We don't need ES6.

@arottersman arottersman assigned rajadain and unassigned arottersman Aug 24, 2017
@rajadain
Copy link
Member Author

Thanks for reviewing! Going to squash, rebase on develop, then merge.

When a tab is clicked, it's model is set to have {active: true}.
We use Marionette events that bubble up the chain, triggering
re-renders of all relevant features.
Previously the switching would work, but the icons would not
update. Now we add or remove the `active` class as necessary.

I tried doing this via `modelEvents: { 'change:mode': 'render' }`,
but that would get rid of the precipitation control added in
`onShow`. This seemed like the best second option.
In case of a stacked chart, or when the minimum y-value is
more than 1, we use the 0.1f format which is 1 decimal place.
For when the value is less than 1, we calculate how deep is
the most significant digit of the smallest non-zero y-value,
and set that as the tick precision.

See https://github.com/d3/d3-format for details.
@rajadain rajadain force-pushed the tt/compare-view-revamp-tr55-water-quality-charts-table branch from 5b28a56 to 85a3820 Compare August 24, 2017 16:48
@rajadain rajadain merged commit 0184de4 into develop Aug 24, 2017
@rajadain rajadain deleted the tt/compare-view-revamp-tr55-water-quality-charts-table branch August 24, 2017 18:13
@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

4 participants