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 watershed information table #407

Merged
merged 3 commits into from
May 19, 2021
Merged

Add watershed information table #407

merged 3 commits into from
May 19, 2021

Conversation

corviday
Copy link
Contributor

@corviday corviday commented Apr 30, 2021

This PR addresses a UI issue with the new streamflow portal. The streamflow portal has the user click on a single point on the map (streamflow does not make sense averaged across an area) but it can be difficult for the user to tell whether they have indeed clicked on the point they're interested in; the UI doesn't offer any feedback. These changes add a table showing some information about the watershed that drains to the point the user clicked on: the latitude and longitude of the selected point (from the map) and information provided by the watershed API: the area of the watershed that drains to that point, highest and lowest elevations, and the Melton ratio.

Changes:

  • Adds AttributeValueTable, a table that will display a collection of objects with attribute, value and units members
  • Adds WatershedSummaryTable, a component that queries the watershed backend for data and passes it to an AtributeValueTable
  • getWatershed() and getWatershedGeographyName() in ce-backend.js for querying the watershed API
  • validateWatershedData() and parseWatershedTableData() in util.js for checking the API response and formatting it into attribute/value/unit objects for the table
  • tests for the new stuff, which I think is more linecount than the new code

Demo.

Resolves #405

@corviday corviday force-pushed the watershed-table branch from 5b15423 to ab67486 Compare May 3, 2021 16:36
@corviday corviday requested a review from rod-glover May 7, 2021 01:47
Copy link
Contributor

@rod-glover rod-glover left a comment

Choose a reason for hiding this comment

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

Nice job. Great testing. (Isn't it amazing how more than half the effort of development is in creating tests?)

A couple of thoughts about the UI:

  • Is it possible to change the hint popup on the circle marker in the map toolbar to read something like "Select a point on the map"? I recall looking into this for some other map tool, but can't remember if it proved intransigent. Alternatively, change the prompt in the unselected state to read "Select a point on the map, using the circle marker tool, to see watershed information."
  • The watershed information is great. May I suggest making it more explicit that the watershed in question is the one draining to the selected point? So maybe a change to the heading of the table, and/or revising the label of the Area and Melton Ratio params to be explicit about this?

}
}

export default AttributeValueTable;
Copy link
Contributor

Choose a reason for hiding this comment

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

Totally minor: You can fold this into the class defn above: export default class ....

import styles from './WatershedSummaryTable.module.css';


// TODO: Use `withAsyncData` to factor out common data-fetching code here
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you feel like trying this? Only if you have time.

src/core/__test_data__/sample-API-results.js Show resolved Hide resolved
src/core/__tests__/util-test.js Show resolved Hide resolved
src/core/__tests__/util-test.js Outdated Show resolved Hide resolved
src/core/__tests__/util-test.js Show resolved Hide resolved
src/core/util.js Show resolved Hide resolved
@corviday
Copy link
Contributor Author

These are both great UI suggestions, thanks!

* Is it possible to change the hint popup on the circle marker in the map toolbar to read something like "Select a point on the map"? I recall looking into this for some other map tool, but can't remember if it proved intransigent. Alternatively, change the prompt in the unselected state to read "Select a point on the map, using the circle marker tool, to see watershed information."

It definitely seems like it should be possible to set a custom tooltip - "Draw a circlemarker" isn't very intuitive - but my efforts have come to naught. I updated the table text instead, as you suggested.

* The watershed information is great. May I suggest making it more explicit that the watershed in question is the one draining to the selected point? So maybe a change to the heading of the table, and/or revising the label of the Area and Melton Ratio params to be explicit about this?

Done, thanks!

@corviday corviday merged commit 15453e2 into master May 19, 2021
@corviday corviday deleted the watershed-table branch May 19, 2021 15:17
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.

More context on streamflow point selection
2 participants