Skip to content

Commit

Permalink
[ML] Single metric viewer: Fixes full page refresh behavior. (#44884) (
Browse files Browse the repository at this point in the history
…#45102)

Fixes the behavior where a refresh triggered by timefilter would completely reload the page and e.g. reset the visibility and state of the Create rules flyout.

The refresh() method now features a fullRefresh flag which defaults to the previous behaviour of a full reload. A full reload still gets triggered e.g. if a user changes the selection in one of the detector or entity dropdowns. A 'soft' refresh using fullRefresh=false is used if timefilter triggers a refresh. To be able to indicate that a 'soft' refresh is going on, a full-width progress bar was added below the tab menu item's horizontal line.
  • Loading branch information
walterra committed Sep 7, 2019
1 parent 4d92b49 commit cbcba48
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -333,3 +333,12 @@
}
}
}

/* Hides the progress bar's background so it doesn't look like it increases the thickness
of the horizontal bar below the tab menu elements in its inactive state. */
.mlTimeSeriesExplorerProgress {
background-color: $euiColorEmptyShade;
&::-moz-progress-bar, &::-webkit-progress-bar {
background-color: $euiColorEmptyShade;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
EuiFlexGroup,
EuiFlexItem,
EuiFormRow,
EuiProgress,
EuiSelect,
EuiSpacer,
EuiText,
Expand Down Expand Up @@ -103,6 +104,7 @@ function getTimeseriesexplorerDefaultState() {
focusAnnotationData: [],
focusChartData: undefined,
focusForecastData: undefined,
fullRefresh: true,
hasResults: false,
jobs: [],
// Counter to keep track of what data sets have been loaded.
Expand All @@ -127,9 +129,14 @@ function getTimeseriesexplorerDefaultState() {
};
}

const TimeSeriesExplorerPage = ({ children, jobSelectorProps, resizeRef }) => (
const TimeSeriesExplorerPage = ({ children, jobSelectorProps, loading, resizeRef }) => (
<Fragment>
<NavigationMenu tabId="timeseriesexplorer" />
{/* Show animated progress bar while loading */}
{loading && (<EuiProgress className="mlTimeSeriesExplorerProgress" color="primary" size="xs" />)}
{/* Show a progress bar with progress 0% when not loading.
If we'd just show no progress bar when not loading it would result in a flickering height effect. */}
{!loading && (<EuiProgress className="mlTimeSeriesExplorerProgress" value="0" max="100" color="primary" size="xs" />)}
<JobSelector {...jobSelectorProps} />
<div className="ml-time-series-explorer" ref={resizeRef} >
{children}
Expand Down Expand Up @@ -466,8 +473,8 @@ export class TimeSeriesExplorer extends React.Component {
});
}

refresh = () => {
if (this.state.loading) {
refresh = (fullRefresh = true) => {
if (this.state.loading && fullRefresh === false) {
return;
}

Expand All @@ -485,17 +492,22 @@ export class TimeSeriesExplorer extends React.Component {

this.contextChartSelectedInitCallDone = false;

// Only when `fullRefresh` is true we'll reset all data
// and show the loading spinner within the page.
this.setState({
chartDetails: undefined,
contextChartData: undefined,
contextForecastData: undefined,
focusChartData: undefined,
focusForecastData: undefined,
fullRefresh,
loadCounter: currentLoadCounter + 1,
loading: true,
modelPlotEnabled: isModelPlotEnabled(currentSelectedJob, +currentDetectorId, currentEntities),
hasResults: false,
dataNotChartable: false
...(fullRefresh ? {
chartDetails: undefined,
contextChartData: undefined,
contextForecastData: undefined,
focusChartData: undefined,
focusForecastData: undefined,
modelPlotEnabled: isModelPlotEnabled(currentSelectedJob, +currentDetectorId, currentEntities),
hasResults: false,
dataNotChartable: false
} : {}),
}, () => {
const { detectorId, entities, loadCounter, jobs, modelPlotEnabled, selectedJob } = this.state;
const detectorIndex = +detectorId;
Expand Down Expand Up @@ -797,7 +809,7 @@ export class TimeSeriesExplorer extends React.Component {
this.subscriptions.add(annotationsRefresh$.subscribe(this.refresh));
this.subscriptions.add(interval$.subscribe(tableControlsListener));
this.subscriptions.add(severity$.subscribe(tableControlsListener));
this.subscriptions.add(mlTimefilterRefresh$.subscribe(this.refresh));
this.subscriptions.add(mlTimefilterRefresh$.subscribe(() => this.refresh(false)));

// Listen for changes to job selection.
this.subscriptions.add(this.jobSelectService.subscribe(({ selection: selectedJobIds }) => {
Expand Down Expand Up @@ -927,6 +939,7 @@ export class TimeSeriesExplorer extends React.Component {
focusAnnotationData,
focusChartData,
focusForecastData,
fullRefresh,
hasResults,
jobs,
loading,
Expand Down Expand Up @@ -1005,7 +1018,7 @@ export class TimeSeriesExplorer extends React.Component {
this.previousShowModelBounds = showModelBounds;

return (
<TimeSeriesExplorerPage jobSelectorProps={jobSelectorProps} resizeRef={this.resizeRef}>
<TimeSeriesExplorerPage jobSelectorProps={jobSelectorProps} loading={loading} resizeRef={this.resizeRef}>
<div className="series-controls" data-test-subj="mlSingleMetricViewerSeriesControls">
<EuiFlexGroup>
<EuiFlexItem grow={false}>
Expand Down Expand Up @@ -1045,19 +1058,19 @@ export class TimeSeriesExplorer extends React.Component {
</EuiFlexGroup>
</div>

{(loading === true) && (
{(fullRefresh && loading === true) && (
<LoadingIndicator
label={i18n.translate('xpack.ml.timeSeriesExplorer.loadingLabel', {
defaultMessage: 'Loading',
})}
/>
)}

{(jobs.length > 0 && loading === false && hasResults === false) && (
{(jobs.length > 0 && (fullRefresh === false || loading === false) && hasResults === false) && (
<TimeseriesexplorerNoChartData dataNotChartable={dataNotChartable} entities={entities} />
)}

{(jobs.length > 0 && loading === false && hasResults === true) && (
{(jobs.length > 0 && (fullRefresh === false || loading === false) && hasResults === true) && (
<EuiText className="results-container">
<span className="panel-title">
{i18n.translate('xpack.ml.timeSeriesExplorer.singleTimeSeriesAnalysisTitle', {
Expand Down

0 comments on commit cbcba48

Please sign in to comment.