[ML] Explorer Chart Tweaks#22955
Merged
walterra merged 14 commits intoelastic:masterfrom Sep 12, 2018
Merged
Conversation
… beginning of the selected time span.
Contributor
|
Pinging @elastic/ml-ui |
jgowdyelastic
approved these changes
Sep 12, 2018
| let addAnotherTick; | ||
|
|
||
| switch (operator) { | ||
| case 'previous': |
Member
There was a problem hiding this comment.
the TICK_DIRECTION constant could be used here
| const newTickData = getTickData(getNeighourTick(ts)); | ||
|
|
||
| if ( | ||
| newTickData !== false |
Member
There was a problem hiding this comment.
this style with just one condition looks a bit odd.
peteharverson
approved these changes
Sep 12, 2018
Contributor
peteharverson
left a comment
There was a problem hiding this comment.
Some minor comments, otherwise LGTM
|
|
||
| // 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. |
Contributor
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) { |
Contributor
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); |
Contributor
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) { |
Contributor
There was a problem hiding this comment.
I think 'startTimestamp' would be a clearer name for this argument, assuming 'Ts' is a timestamp?
Contributor
💚 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.
Contributor
💚 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.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.