IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-12 08:35:43 -06:00
Code Issues Packages Projects Releases Wiki Activity

OptionsPicker: Correctly highlight template variable value when filtering (#63495)

Browse Source 
* VariablePicker: Correctly highlight items when filtering

* Change interactions for selecting values in variable options picker

* Review
This commit is contained in:
Dominik Prokop 2023-03-08 03:36:06 -08:00 committed by GitHub
parent 1d1f58f0ed
commit e4d591fc01
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 58 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
public/app/features/variables/pickers/OptionsPicker/OptionsPicker.tsx
View File

@ -150,7 +150,6 @@ export const optionPickerFactory = <Model extends VariableWithOptions | Variable
onNavigate={this.onNavigate}
aria-expanded={true}
aria-controls={`options-${id}`}
currenthighlightindex={picker.highlightIndex}
/>
<VariableOptions
values={picker.options}

57
public/app/features/variables/pickers/OptionsPicker/actions.test.ts
View File

@ -212,27 +212,44 @@ describe('options picker actions', () => {
.whenActionIsDispatched(navigateOptions('key', NavigationKey.moveDown, clearOthers))
.whenAsyncActionIsDispatched(navigateOptions('key', key, clearOthers), true);
const option = createOption(['sameLabel'], ['B'], true);
// Check selecting the second option triggers variables to update
tester.thenDispatchedActionsShouldEqual(
toKeyedAction('key', toggleOption({ option: options[1], forceSelect: true, clearOthers })),
toKeyedAction('key', setCurrentVariableValue(toVariablePayload(variable, { option }))),
toKeyedAction('key', changeVariableProp(toVariablePayload(variable, { propName: 'queryValue', propValue: '' }))),
toKeyedAction('key', hideOptions()),
toKeyedAction('key', setCurrentVariableValue(toVariablePayload(variable, { option })))
toKeyedAction('key', toggleOption({ option: options[1], forceSelect: false, clearOthers }))
);
expect(locationService.partial).toHaveBeenLastCalledWith({ 'var-Constant': ['B'] });
});
describe('when navigateOptions is dispatched with navigation key selectAndClose after highlighting the second option', () => {
it('then correct actions are dispatched', async () => {
it('then correct actions are dispatched for multi-value variable', async () => {
const options = [createOption('A'), createOption('B'), createOption('C')];
const variable = createMultiVariable({ options, current: createOption(['A'], ['A'], true), includeAll: false });
const clearOthers = false;
const key = NavigationKey.selectAndClose;
const tester = await reduxTester<RootReducerType>()
.givenRootReducer(getRootReducer())
.whenActionIsDispatched(
toKeyedAction('key', addVariable(toVariablePayload(variable, { global: false, index: 0, model: variable })))
)
.whenActionIsDispatched(toKeyedAction('key', showOptions(variable)))
.whenActionIsDispatched(navigateOptions('key', NavigationKey.moveDown, clearOthers))
.whenActionIsDispatched(navigateOptions('key', NavigationKey.moveDown, clearOthers))
.whenActionIsDispatched(navigateOptions('key', NavigationKey.moveDown, clearOthers))
.whenActionIsDispatched(navigateOptions('key', NavigationKey.moveUp, clearOthers))
.whenAsyncActionIsDispatched(navigateOptions('key', key, clearOthers), true);
tester.thenDispatchedActionsShouldEqual(
toKeyedAction('key', toggleOption({ option: options[1], forceSelect: false, clearOthers }))
);
});
it('then correct actions are dispatched for single-value variable', async () => {
const options = [createOption('A'), createOption('B'), createOption('C')];
const variable = createVariable({ options, current: createOption('A', 'A', true), includeAll: false });
const clearOthers = false;
const key = NavigationKey.selectAndClose;
const tester = await reduxTester<RootReducerType>()
.givenRootReducer(getRootReducer())
.whenActionIsDispatched(
@ -246,9 +263,9 @@ describe('options picker actions', () => {
.whenAsyncActionIsDispatched(navigateOptions('key', key, clearOthers), true);
const option = {
...createOption(['B']),
...createOption('B'),
selected: true,
value: ['B'],
value: 'B',
};
tester.thenDispatchedActionsShouldEqual(
@ -261,7 +278,7 @@ describe('options picker actions', () => {
toKeyedAction('key', hideOptions()),
toKeyedAction('key', setCurrentVariableValue(toVariablePayload(variable, { option })))
);
expect(locationService.partial).toHaveBeenLastCalledWith({ 'var-Constant': ['B'] });
expect(locationService.partial).toHaveBeenLastCalledWith({ 'var-Constant': 'B' });
});
});
@ -526,7 +543,7 @@ describe('options picker actions', () => {
.whenActionIsDispatched(toggleOptionByHighlight('key', clearOthers));
const optionA = createOption('A');
const optionBC = createOption('BC');
const optionBD = createOption('BD');
tester.thenDispatchedActionsShouldEqual(
toKeyedAction('key', toggleOption({ option: optionA, forceSelect: false, clearOthers })),
@ -534,13 +551,21 @@ describe('options picker actions', () => {
toKeyedAction('key', updateOptionsAndFilter(variable.options)),
toKeyedAction('key', moveOptionsHighlight(1)),
toKeyedAction('key', moveOptionsHighlight(1)),
toKeyedAction('key', toggleOption({ option: optionBC, forceSelect: false, clearOthers }))
toKeyedAction('key', toggleOption({ option: optionBD, forceSelect: false, clearOthers }))
);
});
});
});
function createMultiVariable(extend?: Partial<QueryVariableModel>): QueryVariableModel {
return createVariable({
multi: true,
includeAll: true,
...(extend ?? {}),
});
}
function createVariable(extend?: Partial<QueryVariableModel>): QueryVariableModel {
return {
...initialVariableModelState,
type: 'query',
@ -556,8 +581,8 @@ function createMultiVariable(extend?: Partial<QueryVariableModel>): QueryVariabl
sort: VariableSort.alphabeticalAsc,
refresh: VariableRefresh.never,
regex: '',
multi: true,
includeAll: true,
multi: false,
includeAll: false,
...(extend ?? {}),
};
}

7
public/app/features/variables/pickers/OptionsPicker/actions.ts
View File

@ -34,8 +34,13 @@ export const navigateOptions = (rootStateKey: string, key: NavigationKey, clearO
}
if (key === NavigationKey.selectAndClose) {
const picker = getVariablesState(rootStateKey, getState()).optionsPicker;
if (picker.multi) {
return dispatch(toggleOptionByHighlight(rootStateKey, clearOthers));
}
dispatch(toggleOptionByHighlight(rootStateKey, clearOthers, true));
return await dispatch(commitChangesToVariable(rootStateKey));
return dispatch(commitChangesToVariable(rootStateKey));
}
if (key === NavigationKey.moveDown) {

14
public/app/features/variables/pickers/OptionsPicker/reducer.test.ts
View File

@ -535,7 +535,7 @@ describe('optionsPickerReducer', () => {
],
selectedValues: [{ text: 'All', value: '$__all', selected: true }],
queryValue: 'A',
highlightIndex: -1,
highlightIndex: 0,
});
});
@ -559,7 +559,7 @@ describe('optionsPickerReducer', () => {
options: [{ text: 'All', value: '$__all', selected: true }],
selectedValues: [{ text: 'All', value: '$__all', selected: true }],
queryValue: 'A',
highlightIndex: -1,
highlightIndex: 0,
});
});
});
@ -583,7 +583,7 @@ describe('optionsPickerReducer', () => {
options: [{ text: 'option:1337', value: 'option:1337', selected: false }],
selectedValues: [],
queryValue: 'option:1337',
highlightIndex: -1,
highlightIndex: 0,
});
});
});
@ -635,7 +635,7 @@ describe('optionsPickerReducer', () => {
],
selectedValues: [{ text: 'B', value: 'B', selected: true }],
queryValue: 'A',
highlightIndex: -1,
highlightIndex: 0,
})
.whenActionIsDispatched(updateSearchQuery(''))
.thenStateShouldEqual({
@ -646,7 +646,7 @@ describe('optionsPickerReducer', () => {
],
selectedValues: [{ text: 'B', value: 'B', selected: true }],
queryValue: '',
highlightIndex: -1,
highlightIndex: 0,
})
.whenActionIsDispatched(updateOptionsAndFilter(options))
.thenStateShouldEqual({
@ -658,7 +658,7 @@ describe('optionsPickerReducer', () => {
],
selectedValues: [{ text: 'B', value: 'B', selected: true }],
queryValue: '',
highlightIndex: -1,
highlightIndex: 0,
});
});
});
@ -766,7 +766,7 @@ describe('optionsPickerReducer', () => {
options: [{ text: 'option:11256', value: 'option:11256', selected: false }],
selectedValues: [],
queryValue: 'option:11256',
highlightIndex: -1,
highlightIndex: 0,
});
});
});

6
public/app/features/variables/pickers/OptionsPicker/reducer.ts
View File

@ -1,5 +1,5 @@
import { createSlice, PayloadAction } from '@reduxjs/toolkit';
import { cloneDeep, isString, trim } from 'lodash';
import { cloneDeep, isString, trimStart } from 'lodash';
import { applyStateChanges } from '../../../../core/utils/applyStateChanges';
import { ALL_VARIABLE_VALUE } from '../../constants';
@ -197,7 +197,7 @@ const optionsPickerSlice = createSlice({
return state;
},
updateOptionsAndFilter: (state, action: PayloadAction<VariableOption[]>): OptionsPickerState => {
const searchQuery = trim((state.queryValue ?? '').toLowerCase());
const searchQuery = trimStart((state.queryValue ?? '').toLowerCase());
state.options = action.payload.filter((option) => {
const optionsText = option.text ?? '';
@ -205,7 +205,7 @@ const optionsPickerSlice = createSlice({
return text.toLowerCase().indexOf(searchQuery) !== -1;
});
state.highlightIndex = -1;
state.highlightIndex = 0;
return applyStateChanges(state, updateDefaultSelection, updateOptions);
},

6
public/app/features/variables/pickers/shared/VariableInput.tsx
View File

@ -8,15 +8,11 @@ export interface Props extends Omit<React.HTMLProps<HTMLInputElement>, 'onChange
onChange: (value: string) => void;
onNavigate: (key: NavigationKey, clearOthers: boolean) => void;
value: string | null;
currenthighlightindex?: number;
}
export class VariableInput extends PureComponent<Props> {
onKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => {
if (
NavigationKey[event.keyCode] &&
!(event.keyCode === NavigationKey.select && this.props.currenthighlightindex === -1)
) {
if (NavigationKey[event.keyCode] && event.keyCode !== NavigationKey.select) {
const clearOthers = event.ctrlKey || event.metaKey || event.shiftKey;
this.props.onNavigate(event.keyCode as NavigationKey, clearOthers);
event.preventDefault();