-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Replace Discover chart with elastic-charts #43788
Merged
Merged
Changes from 14 commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
3048b58
[discover] use elastic-charts for histogram
a3f5802
[discover] add class accessibility
b19e606
[discover] bump elastic-charts version and specify onElementClick typ…
eb49a08
[discover] set chartElement tooltip type to Follow
ce5c42d
[discover] use moment methods for now annotation logic
ab1092d
Merge branch 'master' into discover-chart-replace
markov00 b78fb2c
refactor: move historam inside directive folder
markov00 1f455c5
refactor: remove unused timechart directive
markov00 5fc881f
refactor: remove dependency from tsvb brush handler
markov00 aeaf295
refactor: remove unrequired class to fix tooltip overflow
markov00 d7d763b
WIP
markov00 6f9fc20
Upgrade elastichart library and change the cursor/crosshair
markov00 0b41910
Merge branch 'master' into discover-chart-replace
markov00 8850f18
Merge branch 'master' into discover-chart-replace
markov00 49bd4f3
upgrade elastic charts library
nickofthyme 149d687
Merge branch 'master' into discover-chart-replace
markov00 335fad8
Merge branch 'discover-chart-replace' of github.com:markov00/kibana i…
markov00 3d3b76f
fix(ie11): add fixed width for header text
markov00 fc17973
fix: annotation colors on dark theme
markov00 aebac5c
unskip click and brush ui tests
nickofthyme f545d6a
Merge branch 'master' into discover-chart-replace
nickofthyme c65a7c3
move functional tests to percy
nickofthyme 72cf456
wait for loading to complete for percy screenshots
nickofthyme ff9f75b
Merge branch 'master' into discover-chart-replace
nickofthyme f9bbb01
Merge branch 'master' into discover-chart-replace
nickofthyme a396959
update partial bucket message
nickofthyme 6298388
fix merge conflicts and functional test errors
nickofthyme 0141ddb
update discover page with changes to broser util function
nickofthyme File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
3 changes: 3 additions & 0 deletions
3
src/legacy/core_plugins/kibana/public/discover/directives/_histogram.scss
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
.dscHistogram__header--partial { | ||
font-weight: $euiFontWeightRegular; | ||
} |
1 change: 1 addition & 0 deletions
1
src/legacy/core_plugins/kibana/public/discover/directives/_index.scss
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
@import 'no_results'; | ||
@import 'histogram'; |
255 changes: 255 additions & 0 deletions
255
src/legacy/core_plugins/kibana/public/discover/directives/histogram.tsx
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,255 @@ | ||
/* | ||
* Licensed to Elasticsearch B.V. under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch B.V. licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
import { EuiFlexGroup, EuiFlexItem, EuiIcon, EuiSpacer } from '@elastic/eui'; | ||
import moment from 'moment-timezone'; | ||
import React, { Component } from 'react'; | ||
import PropTypes from 'prop-types'; | ||
|
||
import { | ||
AnnotationDomainTypes, | ||
Axis, | ||
Chart, | ||
HistogramBarSeries, | ||
GeometryValue, | ||
getAnnotationId, | ||
getAxisId, | ||
getSpecId, | ||
LineAnnotation, | ||
Position, | ||
ScaleType, | ||
Settings, | ||
RectAnnotation, | ||
TooltipValue, | ||
TooltipType, | ||
} from '@elastic/charts'; | ||
|
||
import { i18n } from '@kbn/i18n'; | ||
|
||
import { getChartTheme } from 'ui/elastic_charts'; | ||
import chrome from 'ui/chrome'; | ||
// @ts-ignore: path dynamic for kibana | ||
import { timezoneProvider } from 'ui/vis/lib/timezone'; | ||
|
||
export interface DiscoverHistogramProps { | ||
chartData: any; | ||
timefilterUpdateHandler: (ranges: { from: number; to: number }) => void; | ||
} | ||
|
||
export class DiscoverHistogram extends Component<DiscoverHistogramProps> { | ||
public static propTypes = { | ||
chartData: PropTypes.object, | ||
timefilterUpdateHandler: PropTypes.func, | ||
}; | ||
|
||
public onBrushEnd = (min: number, max: number) => { | ||
const range = { | ||
from: min, | ||
to: max, | ||
}; | ||
|
||
this.props.timefilterUpdateHandler(range); | ||
}; | ||
|
||
public onElementClick = (xInterval: number) => (elementData: GeometryValue[]) => { | ||
const startRange = elementData[0].x; | ||
|
||
const range = { | ||
from: startRange, | ||
to: startRange + xInterval, | ||
}; | ||
|
||
this.props.timefilterUpdateHandler(range); | ||
}; | ||
|
||
public formatXValue = (val: string) => { | ||
const xAxisFormat = this.props.chartData.xAxisFormat.params.pattern; | ||
|
||
return moment(val).format(xAxisFormat); | ||
}; | ||
|
||
public renderBarTooltip = (xInterval: number, domainStart: number, domainEnd: number) => ( | ||
headerData: TooltipValue | ||
): JSX.Element | string => { | ||
const headerDataValue = headerData.value; | ||
const formattedValue = this.formatXValue(headerDataValue); | ||
|
||
const partialDataText = i18n.translate('kbn.discover.histogram.partialData.bucketTooltipText', { | ||
defaultMessage: | ||
'Part of this bucket may contain partial data. The selected time range does not fully cover it.', | ||
}); | ||
|
||
if (headerDataValue < domainStart || headerDataValue + xInterval > domainEnd) { | ||
return ( | ||
<React.Fragment> | ||
<EuiFlexGroup | ||
alignItems="center" | ||
className="dscHistogram__header--partial" | ||
responsive={false} | ||
gutterSize="xs" | ||
> | ||
<EuiFlexItem grow={false}> | ||
<EuiIcon type="iInCircle" /> | ||
</EuiFlexItem> | ||
<EuiFlexItem>{partialDataText}</EuiFlexItem> | ||
</EuiFlexGroup> | ||
<EuiSpacer size="xs" /> | ||
<p>{formattedValue}</p> | ||
</React.Fragment> | ||
); | ||
} | ||
|
||
return formattedValue; | ||
}; | ||
|
||
public render() { | ||
const uiSettings = chrome.getUiSettingsClient(); | ||
const timeZone = timezoneProvider(uiSettings)(); | ||
const { chartData } = this.props; | ||
|
||
if (!chartData || !chartData.series[0]) { | ||
return null; | ||
} | ||
|
||
const data = chartData.series[0].values; | ||
|
||
/** | ||
* Deprecation: [interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval]. | ||
* see https://github.com/elastic/kibana/issues/27410 | ||
* TODO: Once the Discover query has been update, we should change the below to use the new field | ||
*/ | ||
const xInterval = chartData.ordered.interval; | ||
|
||
const xValues = chartData.xAxisOrderedValues; | ||
const lastXValue = xValues[xValues.length - 1]; | ||
|
||
const domain = chartData.ordered; | ||
const domainStart = domain.min.valueOf(); | ||
const domainEnd = domain.max.valueOf(); | ||
|
||
const domainMin = data[0].x > domainStart ? domainStart : data[0].x; | ||
const domainMax = domainEnd - xInterval > lastXValue ? domainEnd - xInterval : lastXValue; | ||
|
||
const xDomain = { | ||
min: domainMin, | ||
max: domainMax, | ||
minInterval: xInterval, | ||
}; | ||
|
||
// Domain end of 'now' will be milliseconds behind current time, so we extend time by 1 minute and check if | ||
// the annotation is within this range; if so, the line annotation uses the domainEnd as its value | ||
const now = moment(); | ||
const isAnnotationAtEdge = | ||
moment(domainEnd) | ||
.add(60000) | ||
.isAfter(now) && now.isAfter(domainEnd); | ||
const lineAnnotationValue = isAnnotationAtEdge ? domainEnd : now; | ||
|
||
const lineAnnotationData = [ | ||
{ | ||
dataValue: lineAnnotationValue, | ||
}, | ||
]; | ||
|
||
const lineAnnotationStyle = { | ||
line: { | ||
strokeWidth: 2, | ||
stroke: '#c80000', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in fc17973 |
||
opacity: 0.3, | ||
}, | ||
}; | ||
|
||
const rectAnnotations = []; | ||
if (domainStart !== domainMin) { | ||
rectAnnotations.push({ | ||
coordinates: { | ||
x1: domainStart, | ||
}, | ||
}); | ||
} | ||
if (domainEnd !== domainMax) { | ||
rectAnnotations.push({ | ||
coordinates: { | ||
x0: domainEnd, | ||
}, | ||
}); | ||
} | ||
|
||
const rectAnnotationStyle = { | ||
stroke: 'rgba(0, 0, 0, 0)', | ||
strokeWidth: 1, | ||
opacity: 1, | ||
fill: 'rgba(0, 0, 0, 0.1)', | ||
dmlemeshko marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}; | ||
|
||
const tooltipProps = { | ||
headerFormatter: this.renderBarTooltip(xInterval, domainStart, domainEnd), | ||
type: TooltipType.VerticalCursor, | ||
}; | ||
|
||
return ( | ||
<Chart size="100%"> | ||
<Settings | ||
xDomain={xDomain} | ||
onBrushEnd={this.onBrushEnd} | ||
onElementClick={this.onElementClick(xInterval)} | ||
tooltip={tooltipProps} | ||
theme={getChartTheme()} | ||
/> | ||
<Axis | ||
id={getAxisId('discover-histogram-left-axis')} | ||
position={Position.Left} | ||
ticks={5} | ||
title={chartData.yAxisLabel} | ||
/> | ||
<Axis | ||
id={getAxisId('discover-histogram-bottom-axis')} | ||
position={Position.Bottom} | ||
title={chartData.xAxisLabel} | ||
tickFormat={this.formatXValue} | ||
ticks={10} | ||
/> | ||
<LineAnnotation | ||
annotationId={getAnnotationId('line-annotation')} | ||
domainType={AnnotationDomainTypes.XDomain} | ||
dataValues={lineAnnotationData} | ||
hideTooltips={true} | ||
style={lineAnnotationStyle} | ||
/> | ||
<RectAnnotation | ||
dataValues={rectAnnotations} | ||
annotationId={getAnnotationId('rect-annotation')} | ||
zIndex={2} | ||
style={rectAnnotationStyle} | ||
hideTooltips={true} | ||
/> | ||
<HistogramBarSeries | ||
id={getSpecId('discover-histogram')} | ||
xScaleType={ScaleType.Time} | ||
yScaleType={ScaleType.Linear} | ||
xAccessor="x" | ||
yAccessors={['y']} | ||
data={data} | ||
timeZone={timeZone} | ||
name={chartData.yAxisLabel} | ||
/> | ||
</Chart> | ||
); | ||
} | ||
} |
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
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
Oops, something went wrong.
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.
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.
"Part of...partial data" feels a little redundant.
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.
See a396959