-
Notifications
You must be signed in to change notification settings - Fork 362
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
Various fixes for SOS. #6059
Various fixes for SOS. #6059
Conversation
- Re-enables procedure and observable selectors - Fixes feature info chart - Fixes feature info table
Not sure why it was done before but this change should be harmless.
@@ -515,7 +515,7 @@ function calculateDomain(chartItems) { | |||
const ymax = Math.max(...chartItems.map(c => c.domain.y[1])); | |||
return { | |||
x: [xmin, xmax], | |||
y: [Math.min(0, ymin), ymax] | |||
y: [ymin, ymax] |
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.
I have a suspicion that this is going to break some dataset, somewhere, that was depending on us clamping the min of the y domain to zero. But this is probably one of those situations where we tell people to fix their own data.
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.
Yeah, i think so too, but this shouldn't be the default behavior. If we find that we need it somewhere we can add a flag to force it per item. The problem with clamping automatically is that the when displaying a single chart that has y value >>> 0 the chart line looks flat loosing all the bumps and details which is bad.
Thanks for the merge @KeyboardSounds! |
What this PR does
title
instead ofname
as property names.Previously:
Test link:
http://ci.terria.io/fix-sos/#clean&https://raw.githubusercontent.com/TerriaJS/saas-catalogs-public/main/neii/water/prod.json
Checklist
doc/
.