feat(y_axis_domain): updated y-axis domain settings to allow min OR max values to be set. #18040
Conversation
…ax values to be set.
asalem1
force-pushed
the
feat/min-or-max-y-axis
branch
from
f8a8fe2
to
99904d8
Compare
…ualization stability. Also updated the CHANGELOG
| @@ -69,6 +58,7 @@ const MinMaxInputs: SFC<MinMaxInputsProps> = ({ | |||
| onChange={e => setMinInput(e.target.value)} | |||
| onBlur={emitIfValid} | |||
| onKeyPress={handleKeyPress} | |||
| testID="min--auto-domain" | |||
There was a problem hiding this comment.
i think the more BEM-y way of doing this is to scope it like css where it starts less specific progresses to more specific. in other words, we want to think of auto-domain as containing things like min and max:
| testID="min--auto-domain" | |
| testID="auto-domain--min" |
There was a problem hiding this comment.
What's BEM stand for?
There was a problem hiding this comment.
oooooof. my bad: http://getbem.com/introduction/
There was a problem hiding this comment.
All good. Thanks for sharing this!
| timeRange: TimeRange | null, | ||
| storedDomain: number[] | ||
| ) => { | ||
| const range = extent((data as number[]) || []) |
There was a problem hiding this comment.
love the enthusiasm around fallback values, but is the || [] necessary here since line 59 sets a default value of []?
are you trying to do this
const range = extent((data as number[])) || []so that range will always be an array?
There was a problem hiding this comment.
It is necessary to set a default for the range since the output of the extent function can be null, in which case we would want to return an empty array.
There was a problem hiding this comment.
so does that mean it should be
const range = extent(data as number[]) || []There was a problem hiding this comment.
Aw, I think I see what you were getting at. Ya, that's a bit awkward. Honestly I don't think I took an introspective look at the existing code to analyze why things were set they were there. You're totally right
| if (storedDomain === null || storedDomain.every(val => val === null)) { | ||
| return getValidRange(data, timeRange) | ||
| } | ||
| if (storedDomain.includes(null)) { | ||
| return getRemainingRange(data, timeRange, storedDomain) | ||
| } |
There was a problem hiding this comment.
this code loops over the same structure twice to perform essentially the same check (is the current item null). this happens while rendering. it's good you're using memoization, but you could write this logic using a single loop. what kind of array sizes are we dealing with in these loops?
There was a problem hiding this comment.
I'm not sure I follow you, would you mind elaborating on this a bit?
…t, testIDs, and safe-guarding types
… parseBounds to useVisXDomainSettings & parseXBounds for greater specificity
…it's only happening once
Closes #17793
Purpose
The purpose of this PR is to allow users to set a Y-domain min/max value without requiring users to set both. In other words, a user should be able to set the Y-domain with either a MIN or a MAX, and should not be required to set both.
Considerations
This feature required a removal of current restrictions on setting domain values, such as:
The ramifications of these changes were addressed by dynamically setting empty values based on the data range that is returned. If a user set their minimum value to 0, and left the maximum value blank, the upper bounds of their range would be set dynamically based on the data range. Similarly, if a user set their maximum value to 100 and left he minimum value blank, the lower bounds of their range would be set dynamically based on the data range.
In effect, this treats the non-blank input values as the source of truth for data range visualization. Therefore, if a user sets the minimum y value to be greater than the upper bounds of the data range values, and leaves the maximum value blank, then no data will show up (since it is beyond the range). Similarly, if a user sets the maximum y value to be less than the lower bounds of the data range values and leaves the minimum value blank, then no data will show up (for the same reason as before).
The images below list off the different use cases:
Auto Set:
Set minimum blank:
Set fields blank:
Set the min greater than the max data range:
Set the min > max:
Set the min, doesn't set the max:
Set Max less than the min data range:
Set Both:
