[ML] Explorer Chart Tweaks #22955
Merged
[ML] Explorer Chart Tweaks #22955
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… beginning of the selected time span.
|
Pinging @elastic/ml-ui |
jgowdyelastic
approved these changes
Sep 12, 2018
| let addAnotherTick; | ||
|
|
||
| switch (operator) { | ||
| case 'previous': |
There was a problem hiding this comment.
the TICK_DIRECTION constant could be used here
| const newTickData = getTickData(getNeighourTick(ts)); | ||
|
|
||
| if ( | ||
| newTickData !== false |
There was a problem hiding this comment.
this style with just one condition looks a bit odd.
peteharverson
approved these changes
Sep 12, 2018
| .tickFormat(d => moment(d).format(xAxisTickFormat)); | ||
|
|
||
| // With tooManyBuckets the chart would end up with no x-axis labels | ||
| // because the ticks are based on the span of the emphasis section, | ||
| // but that one spans the whole chart width with tooManyBuckets. |
There was a problem hiding this comment.
I don't really understand what this is saying. Maybe something like 'and the highlighted area would span the entire chart'?
| // ones where we remove the label in the next steps. | ||
| axis.selectAll('g.tick').select('line').classed('ml-tick-emphasis', true); | ||
|
|
||
| function getNeighourTickFactory(operator) { |
There was a problem hiding this comment.
Typo in the function name. Should be getNeighbourTickFactory or I suppose getNeighborTickFactory if we are sticking with American English ;)
| } | ||
|
|
||
| function getTickDataFactory(operator) { | ||
| const getNeighourTick = getNeighourTickFactory(operator); |
There was a problem hiding this comment.
Same typos here. Should be 'Neighbor' / 'Neighbour'.
| // that is required/wanted to show up. The code then traverses to both sides along the axis | ||
| // and decides which labels to keep or remove. All vertical tick lines will be kept visible, | ||
| // but those which still have their text label will be emphasized using the ml-tick-emphasis class. | ||
| export function removeLabelOverlap(axis, startTs, tickInterval, width) { |
There was a problem hiding this comment.
I think 'startTimestamp' would be a clearer name for this argument, assuming 'Ts' is a timestamp?
💚 Build Succeeded |
walterra
added a commit
to walterra/kibana
that referenced
this pull request
Sep 12, 2018
- The aim of this is to more clearly visualize how the timerange of the cell selected in the swimlane relates to the time span shown in the charts. - The most important change is that the vertical date axis ticks no longer are randomly positioned by d3. Instead they are aligned with the cell interval of the swimlane. This way, the date information shown in the swimlane tooltip will always align with the date tick shown left of the emphasized area in the chart. - The highlighted area now features a gray rounded border to resemble the styling of the selected cell in the swimlane. - The chart also fixes where to long chart headers would wrap the "View" link to a new line. - The x/y axis labels blackness has been reduced to reduce emphasis on the labels.
💚 Build Succeeded |
walterra
added a commit
that referenced
this pull request
Sep 12, 2018
- The aim of this is to more clearly visualize how the timerange of the cell selected in the swimlane relates to the time span shown in the charts. - The most important change is that the vertical date axis ticks no longer are randomly positioned by d3. Instead they are aligned with the cell interval of the swimlane. This way, the date information shown in the swimlane tooltip will always align with the date tick shown left of the emphasized area in the chart. - The highlighted area now features a gray rounded border to resemble the styling of the selected cell in the swimlane. - The chart also fixes where to long chart headers would wrap the "View" link to a new line. - The x/y axis labels blackness has been reduced to reduce emphasis on the labels.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #18067.
This tweaks the design of the Anomaly Explorer Charts as discussed in the issue mentioned above.
The aim of this is to more clearly visualize how the timerange of the cell selected in the swimlane relates to the time span shown in the charts.
Here's a comparison showing the previous layout (left) vs the updated on (right):
The time span shown in the charts is chosen dynamically and can differ, here's another comparison:
The new version shows more vertical ticks now, again, they correspond to the interval in the swimlane. Only certain ticks are highlighted with date labels to avoid label overlap, but it's made sure the label at the start of the highlighted area is shown to correspond with the tooltip information of the swimlane. The density of the vertical ticks provides some context about how the shown time span relates to the amount of represented cells in the swimlane. The right chart also shows a fix where to long chart headers would wrap the "View" link to a new line. The x/y axis labels blackness has been reduced to reduce emphasis on the labels.