-
Notifications
You must be signed in to change notification settings - Fork 6
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
Adjusting rescale value for small/large numbers #1272
Conversation
✅ Deploy Preview for veda-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
app/scripts/components/exploration/components/datasets/colorRangeSlider/color-range-slider.scss
Show resolved
Hide resolved
59fc32c
to
f7016a0
Compare
@snmln I assume you're looking at NO2 diff, not CO2 diff. Just commenting here for posterity, I think this is an acceptable visualization of these small/large numbers. Only question is, were you able to test this out where the magnitude of the min/max were both <0.01? |
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.
Manual testing looks good on this one. Still need a review from an engineer
85f994b
to
7fdb52e
Compare
migrating changes from color-maps-range-slider #1190
459eef5
to
1d4d878
Compare
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 work was previously approved in: #1190. Then reverted when bug was identified. I needed to pull those changes back in to correctly reflect updates.
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 work was previously approved in: #1190. Then reverted when bug was identified. I needed to pull those changes back in to correctly reflect updates.
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 work was previously approved in: #1190. Then reverted when bug was identified. I needed to pull those changes back in to correctly reflect updates.
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 work was previously approved in: #1190. Then reverted when bug was identified. I needed to pull those changes back in to correctly reflect updates.
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 work was previously approved in: #1190. Then reverted when bug was identified. I needed to pull those changes back in to correctly reflect updates.
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 work was previously approved in: #1190. Then reverted when bug was identified. I needed to pull those changes back in to correctly reflect updates.
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 work was previously approved in: #1190. Then reverted when bug was identified. I needed to pull those changes back in to correctly reflect updates.
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 work was previously approved in: #1190. Then reverted when bug was identified. I needed to pull those changes back in to correctly reflect updates.
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 work was previously approved in: #1190. Then reverted when bug was identified. I needed to pull those changes back in to correctly reflect updates.
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 work was previously approved in: #1190. Then reverted when bug was identified. I needed to pull those changes back in to correctly reflect updates.
@snmln when visiting the dataset you recommended, I was able to see that the inputs are always on scientific notation and I was able to edit then. However, I noticed a couple of small things:
|
@vgeorge Thank you for these call out! I wanted to respond to a few:
The scientific notation is a solution to ensure as much of the number is visible within the boundaries of the input space as possible. We still want to display the numbers in non-scientific notation outside of the input box. This will also only happen when the number in place is 7 characters or larger.
Good catch there was a few raster-layer changes that got lost in the shuffle with re-implementing the changes from #1190. I have gone ahead an implemented them and it should now be functioning correctly. |
Related Ticket: https://github.com/orgs/NASA-IMPACT/projects/17?pane=issue&itemId=85258058&issue=NASA-IMPACT%7Cveda-ui%7C1226
Description of Changes
Notes & Questions About Changes
When observing a value in either the max or min inputs if that value has a character count of 6 or greater the component will convert that number into scientific notation as a display value.
Validation / Testing
Run this branch on the veda-config code base, navigate to the E&A page and select the difference CO2 dataset. This is an extreme data set that should display all changes in this component.