-
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
Custom Weather: Text and Links #3314
Conversation
The sample CSV is bolstered to have 5 years of values, and be something that would pass our validations. It's provenance is `phillyRCP4520802100daily.csv` from Scott Ensign's Future Weather dataset, with 2080 replaced with 2000. Furtheremore, we now reuse it for our testing. This reduces repetition of CSV files in the repository, and ensures that the sample CSV is valid. It is a little unusual to use static files for testing, but they are placed in the right place in the VM during provisioning, so it works.
This may potentially go through some revision from the clients. Refs #3308
Better UX.
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 great. I found one rendering quirk and one existing UI quirk that you may want to drive-by fix:
After uploading a CWD file, the Source: $MY_FILE
text on the Hydrology tab renders as Source: false
before re-rendering correctly. It's brief, but noticeable (and doesn't happen after uploading subsequent to a delete)
The existing UI quirk is that the (i)
popup targets for Hydrology and Water Quality are rendered in different colors. Elsewhere in the app, they are only rendered in black.
I don't think fixing either of these quirks are critical, but could be addressed if timing/resources permit.
Good catch on the |
Tooltip color fixed in 4330e28. |
The previously used .black class was not specified anywhere, instead other targeting was used for the coloring of the icons, which was not consistently applied everywhere. By adding a utility class of .black-74 and using that, we ensure that all tooltip info icons have the same color.
747bb45
to
4330e28
Compare
Thanks for reviewing! Merging this now. |
Overview
Wires up the sample CSV file to be downloaded. It is now also used in testing, to store fewer CSV files in the repo, and to also validate our sample against our own rules.
Adds real text to the weather upload modal. May receive further tweaks from the client. Also makes clear that uploads need to be in centimeters and Celsius (the numbers themselves are not validated).
Also updates the scenario sidebar to report the correct number of years based on the custom weather file.
See commit messages for details.
Connects #3308
Connects #3310
Demo
Updated text:
Updated weather information in sidebars:
Testing Instructions
bundle --debug --vendor