-
Notifications
You must be signed in to change notification settings - Fork 894
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
Add loading indicator and counter to query result #8212
Changes from all commits
f9d478c
5640768
47d1d11
5cd91f5
fec2623
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
feat: | ||
- Add loading indicator and counter to query result ([#8212](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/8212)) |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
/* | ||
* Copyright OpenSearch Contributors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import React from 'react'; | ||
import { mountWithIntl, shallowWithIntl } from 'test_utils/enzyme_helpers'; | ||
import { QueryResult } from './query_result'; | ||
|
||
enum ResultStatus { | ||
UNINITIALIZED = 'uninitialized', | ||
LOADING = 'loading', // initial data load | ||
READY = 'ready', // results came back | ||
NO_RESULTS = 'none', // no results came back | ||
ERROR = 'error', // error occurred | ||
} | ||
|
||
describe('Query Result', () => { | ||
it('shows loading status', () => { | ||
const props = { | ||
queryStatus: { | ||
status: ResultStatus.LOADING, | ||
startTime: Number.NEGATIVE_INFINITY, | ||
}, | ||
}; | ||
const component = shallowWithIntl(<QueryResult {...props} />); | ||
expect(component).toMatchSnapshot(); | ||
}); | ||
|
||
it('shows ready status with complete message', () => { | ||
const props = { | ||
queryStatus: { | ||
status: ResultStatus.READY, | ||
startTime: new Date().getTime(), | ||
}, | ||
}; | ||
const component = mountWithIntl(<QueryResult {...props} />); | ||
const loadingIndicator = component.find(`[data-test-subj="queryResultLoading"]`); | ||
expect(loadingIndicator.exists()).toBeFalsy(); | ||
expect(component.find('EuiText').text()).toEqual('Completed'); | ||
}); | ||
|
||
it('shows ready status with complete in miliseconds message', () => { | ||
const props = { | ||
queryStatus: { | ||
status: ResultStatus.READY, | ||
startTime: new Date().getTime(), | ||
elapsedMs: 500, | ||
}, | ||
}; | ||
const component = mountWithIntl(<QueryResult {...props} />); | ||
expect(component.find('EuiText').text()).toEqual('Completed in 500 ms'); | ||
}); | ||
|
||
it('shows ready status with complete in seconds message', () => { | ||
const props = { | ||
queryStatus: { | ||
status: ResultStatus.READY, | ||
startTime: new Date().getTime(), | ||
elapsedMs: 2000, | ||
}, | ||
}; | ||
const component = mountWithIntl(<QueryResult {...props} />); | ||
expect(component.find('EuiText').text()).toEqual('Completed in 2.0 s'); | ||
}); | ||
|
||
it('shows ready status with split seconds', () => { | ||
const props = { | ||
queryStatus: { | ||
status: ResultStatus.READY, | ||
startTime: new Date().getTime(), | ||
elapsedMs: 2700, | ||
}, | ||
}; | ||
const component = mountWithIntl(<QueryResult {...props} />); | ||
expect(component.find('EuiText').text()).toEqual('Completed in 2.7 s'); | ||
}); | ||
|
||
it('show error status with error message', () => { | ||
const props = { | ||
queryStatus: { | ||
status: ResultStatus.ERROR, | ||
body: { | ||
error: { | ||
reason: 'error reason', | ||
details: 'error details', | ||
}, | ||
statusCode: 400, | ||
}, | ||
}, | ||
}; | ||
const component = shallowWithIntl(<QueryResult {...props} />); | ||
expect(component.find(`[data-test-subj="queryResultError"]`).text()).toMatchInlineSnapshot( | ||
`"<EuiPopover />"` | ||
); | ||
component.find(`[data-test-subj="queryResultError"]`).simulate('click'); | ||
expect(component).toMatchSnapshot(); | ||
}); | ||
|
||
it('returns null when error body is empty', () => { | ||
const props = { | ||
queryStatus: { | ||
status: ResultStatus.ERROR, | ||
}, | ||
}; | ||
const component = shallowWithIntl(<QueryResult {...props} />); | ||
expect(component).toEqual({}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ | |
import './_recent_query.scss'; | ||
import { EuiButtonEmpty, EuiPopover, EuiText, EuiPopoverTitle } from '@elastic/eui'; | ||
|
||
import React, { useState } from 'react'; | ||
import React, { useEffect, useState } from 'react'; | ||
|
||
export enum ResultStatus { | ||
UNINITIALIZED = 'uninitialized', | ||
|
@@ -28,21 +28,77 @@ | |
statusCode?: number; | ||
}; | ||
elapsedMs?: number; | ||
startTime?: number; | ||
} | ||
|
||
// This is the time in milliseconds that the query will wait before showing the loading spinner | ||
amsiglan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const BUFFER_TIME = 3000; | ||
|
||
export function QueryResult(props: { queryStatus: QueryStatus }) { | ||
const [isPopoverOpen, setPopover] = useState(false); | ||
const [elapsedTime, setElapsedTime] = useState(0); | ||
const onButtonClick = () => { | ||
setPopover(!isPopoverOpen); | ||
}; | ||
|
||
const updateElapsedTime = () => { | ||
const time = Date.now() - (props.queryStatus.startTime || 0); | ||
if (time > BUFFER_TIME) { | ||
setElapsedTime(time); | ||
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. Based on the interval below and the logic here, elapsedTime will be Is that expected? 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. i think 1s interval is a reasonable time to get elapsedTime here? since @ashwin-pc also requests we show loading time in seconds 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. Its rounded down to seconds so that should not be a problem. Take a look at the video attached Miki :) |
||
} else { | ||
setElapsedTime(0); | ||
} | ||
}; | ||
|
||
useEffect(() => { | ||
const interval = setInterval(updateElapsedTime, 1000); | ||
|
||
return () => clearInterval(interval); | ||
}); | ||
|
||
if (props.queryStatus.status === ResultStatus.LOADING) { | ||
if (elapsedTime < BUFFER_TIME) { | ||
return null; | ||
} | ||
const time = Math.floor(elapsedTime / 1000); | ||
return ( | ||
Check warning on line 64 in src/plugins/data/public/query/query_string/language_service/lib/query_result.tsx Codecov / codecov/patchsrc/plugins/data/public/query/query_string/language_service/lib/query_result.tsx#L63-L64
|
||
<EuiButtonEmpty | ||
color="text" | ||
size="xs" | ||
onClick={() => {}} | ||
isLoading | ||
data-test-subj="queryResultLoading" | ||
> | ||
{i18n.translate('data.query.languageService.queryResults.completeTime', { | ||
defaultMessage: `Loading ${time} s`, | ||
})} | ||
</EuiButtonEmpty> | ||
); | ||
} | ||
|
||
if (props.queryStatus.status === ResultStatus.READY) { | ||
let message; | ||
if (!props.queryStatus.elapsedMs) { | ||
message = i18n.translate('data.query.languageService.queryResults.completeTime', { | ||
defaultMessage: `Completed`, | ||
}); | ||
} else if (props.queryStatus.elapsedMs < 1000) { | ||
message = i18n.translate( | ||
'data.query.languageService.queryResults.completeTimeInMiliseconds', | ||
{ | ||
defaultMessage: `Completed in ${props.queryStatus.elapsedMs} ms`, | ||
} | ||
); | ||
} else { | ||
message = i18n.translate('data.query.languageService.queryResults.completeTimeInSeconds', { | ||
defaultMessage: `Completed in ${(props.queryStatus.elapsedMs / 1000).toFixed(1)} s`, | ||
}); | ||
} | ||
|
||
return ( | ||
<EuiButtonEmpty iconSide="left" iconType={'checkInCircleEmpty'} size="xs" onClick={() => {}}> | ||
<EuiText size="xs" color="subdued"> | ||
{props.queryStatus.elapsedMs | ||
? `Completed in ${props.queryStatus.elapsedMs} ms` | ||
: 'Completed'} | ||
<EuiText size="xs" color="subdued" data-test-subj="queryResultCompleteMsg"> | ||
{message} | ||
</EuiText> | ||
</EuiButtonEmpty> | ||
); | ||
|
@@ -55,16 +111,25 @@ | |
return ( | ||
<EuiPopover | ||
button={ | ||
<EuiButtonEmpty iconSide="left" iconType={'alert'} size="xs" onClick={onButtonClick}> | ||
<EuiButtonEmpty | ||
iconSide="left" | ||
iconType={'alert'} | ||
size="xs" | ||
onClick={onButtonClick} | ||
data-test-subj="queryResultErrorBtn" | ||
> | ||
<EuiText size="xs" color="subdued"> | ||
{'Error'} | ||
{i18n.translate('data.query.languageService.queryResults.error', { | ||
defaultMessage: `Error`, | ||
})} | ||
</EuiText> | ||
</EuiButtonEmpty> | ||
} | ||
isOpen={isPopoverOpen} | ||
closePopover={() => setPopover(false)} | ||
panelPaddingSize="s" | ||
anchorPosition={'downRight'} | ||
data-test-subj="queryResultError" | ||
> | ||
<EuiPopoverTitle>ERRORS</EuiPopoverTitle> | ||
abbyhu2000 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
<div style={{ width: '250px' }}> | ||
|
@@ -82,7 +147,7 @@ | |
{i18n.translate('data.query.languageService.queryResults.details', { | ||
defaultMessage: `Details:`, | ||
})} | ||
</strong>{' '} | ||
</strong> | ||
{props.queryStatus.body.error.details} | ||
</p> | ||
</EuiText> | ||
|
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.
Can you add a unit test for this functionality?
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.
added