add debounce for GSelect and RemoteSelect (#2466)
# What this PR does Fix performance Issue in GSelect component when searching ## Which issue(s) this PR fixes https://github.com/grafana/oncall/issues/1628 ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --------- Co-authored-by: Joey Orlando <joseph.t.orlando@gmail.com> Co-authored-by: Joey Orlando <joey.orlando@grafana.com>
This commit is contained in:
parent
b2fc9635bf
commit
b951b6b6bd
6 changed files with 26 additions and 33 deletions
|
|
@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
|
|||
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
|
||||
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
|
||||
## Unreleased
|
||||
|
||||
### Fixed
|
||||
|
||||
- Add debounce on Select UI components to avoid making API search requests on each key-down event by
|
||||
@maskin25 ([#2466](https://github.com/grafana/oncall/pull/2466))
|
||||
|
||||
## v1.3.8 (2023-07-11)
|
||||
|
||||
### Added
|
||||
|
|
|
|||
|
|
@ -88,14 +88,7 @@ const chooseDropdownValue = async ({ page, value, optionExactMatch = true }: Sel
|
|||
|
||||
export const selectDropdownValue = async (args: SelectDropdownValueArgs): Promise<Locator> => {
|
||||
const selectElement = await openSelect(args);
|
||||
|
||||
/**
|
||||
* use the select search to filter down the options
|
||||
* TODO: get rid of the slice when we fix the GSelect component..
|
||||
* without slicing this would fire off an API request for every key-stroke
|
||||
*/
|
||||
await selectElement.type(args.value.slice(0, 5));
|
||||
|
||||
await selectElement.type(args.value);
|
||||
await chooseDropdownValue(args);
|
||||
|
||||
return selectElement;
|
||||
|
|
|
|||
|
|
@ -38,7 +38,7 @@ const config: PlaywrightTestConfig = {
|
|||
* to flaky tests.. let's just retry failed tests. If the same test fails 3 times, you know something must be up
|
||||
*/
|
||||
retries: !!process.env.CI ? 3 : 0,
|
||||
workers: !!process.env.CI ? 2 : 1,
|
||||
workers: 3,
|
||||
/* Reporter to use. See https://playwright.dev/docs/test-reporters */
|
||||
reporter: 'html',
|
||||
/* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import { get, isNil } from 'lodash-es';
|
|||
import { observer } from 'mobx-react';
|
||||
|
||||
import { useStore } from 'state/useStore';
|
||||
import { useDebouncedCallback } from 'utils/hooks';
|
||||
|
||||
import styles from './GSelect.module.scss';
|
||||
|
||||
|
|
@ -89,30 +90,22 @@ const GSelect = observer((props: GSelectProps) => {
|
|||
[model, onChange]
|
||||
);
|
||||
|
||||
/**
|
||||
* without debouncing this function when search is available
|
||||
* we risk hammering the API endpoint for every single key stroke
|
||||
* some context on 250ms as the choice here - https://stackoverflow.com/a/44755058/3902555
|
||||
*/
|
||||
const loadOptions = (query: string) => {
|
||||
return model.updateItems(query).then(() => {
|
||||
const loadOptions = useDebouncedCallback((query: string, cb) => {
|
||||
model.updateItems(query).then(() => {
|
||||
const searchResult = model.getSearchResult(query);
|
||||
let items = Array.isArray(searchResult.results) ? searchResult.results : searchResult;
|
||||
if (filterOptions) {
|
||||
items = items.filter((opt: any) => filterOptions(opt[valueField]));
|
||||
}
|
||||
|
||||
return items.map((item: any) => ({
|
||||
const options = items.map((item: any) => ({
|
||||
value: item[valueField],
|
||||
label: get(item, displayField),
|
||||
imgUrl: item.avatar_url,
|
||||
description: getDescription && getDescription(item),
|
||||
}));
|
||||
cb(options);
|
||||
});
|
||||
};
|
||||
|
||||
// TODO: why doesn't this work properly?
|
||||
// const loadOptions = debounce(_loadOptions, showSearch ? 250 : 0);
|
||||
}, 250);
|
||||
|
||||
const values = isMulti
|
||||
? (value ? (value as string[]) : [])
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import React, { useCallback, useEffect, useMemo, useReducer, useState } from 'react';
|
||||
import React, { useCallback, useMemo, useReducer, useState } from 'react';
|
||||
|
||||
import { SelectableValue } from '@grafana/data';
|
||||
import { AsyncMultiSelect, AsyncSelect } from '@grafana/ui';
|
||||
|
|
@ -6,6 +6,7 @@ import { inject, observer } from 'mobx-react';
|
|||
|
||||
import { makeRequest, isNetworkError } from 'network';
|
||||
import { UserAction, generateMissingPermissionMessage } from 'utils/authorization';
|
||||
import { useDebouncedCallback } from 'utils/hooks';
|
||||
|
||||
interface RemoteSelectProps {
|
||||
autoFocus?: boolean;
|
||||
|
|
@ -67,24 +68,20 @@ const RemoteSelect = inject('store')(
|
|||
|
||||
const [options, setOptions] = useReducer(mergeOptions, []);
|
||||
|
||||
const loadOptionsCallback = useCallback(async (query?: string): Promise<SelectableValue[]> => {
|
||||
const loadOptionsCallback = useDebouncedCallback(async (query: string, cb) => {
|
||||
try {
|
||||
const data = await makeRequest(href, { params: { search: query } });
|
||||
const options = getOptions(data.results || data);
|
||||
setOptions(options);
|
||||
|
||||
return options;
|
||||
cb(options);
|
||||
} catch (e) {
|
||||
if (isNetworkError(e) && e.response.status === 403 && requiredUserAction) {
|
||||
setNoOptionsMessage(generateMissingPermissionMessage(requiredUserAction));
|
||||
}
|
||||
return [];
|
||||
cb([]);
|
||||
}
|
||||
}, []);
|
||||
|
||||
useEffect(() => {
|
||||
loadOptionsCallback();
|
||||
}, []);
|
||||
}, 250);
|
||||
|
||||
const onChangeCallback = useCallback(
|
||||
(option) => {
|
||||
|
|
@ -127,7 +124,7 @@ const RemoteSelect = inject('store')(
|
|||
isSearchable={showSearch}
|
||||
value={value}
|
||||
onChange={onChangeCallback}
|
||||
defaultOptions={options}
|
||||
defaultOptions
|
||||
loadOptions={loadOptionsCallback}
|
||||
getOptionLabel={getOptionLabel}
|
||||
noOptionsMessage={noOptionsMessage}
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { action, observable } from 'mobx';
|
|||
|
||||
import BaseStore from 'models/base_store';
|
||||
import { GrafanaTeam } from 'models/grafana_team/grafana_team.types';
|
||||
import { makeRequest } from 'network';
|
||||
import { RootStore } from 'state';
|
||||
|
||||
export class GrafanaTeamStore extends BaseStore {
|
||||
|
|
@ -29,7 +30,9 @@ export class GrafanaTeamStore extends BaseStore {
|
|||
|
||||
@action
|
||||
async updateItems(query = '') {
|
||||
const result = await this.getAll();
|
||||
const result = await makeRequest(`${this.path}`, {
|
||||
params: { search: query },
|
||||
});
|
||||
|
||||
this.items = {
|
||||
...this.items,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue