Dashboard: Fix Variable Dropdown to Enforce Minimum One Selection when 'All' Option is Configured (#69839)

pull/73938/head
Alexa V 2 years ago committed by GitHub
parent e100fc927e
commit 97c3cd1af3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 17
      public/app/features/variables/pickers/OptionsPicker/reducer.test.ts
  2. 53
      public/app/features/variables/pickers/OptionsPicker/reducer.ts
  3. 16
      public/app/features/variables/pickers/shared/VariableOptions.tsx

@ -461,7 +461,7 @@ describe('optionsPickerReducer', () => {
});
});
it('should toggle all values to false when $_all is selected', () => {
it('should toggle each individual value to true when $_all is selected and mark ALL as selected, not supporting empty values', () => {
const { initialState } = getVariableTestContext({
options: [
{ text: 'All', value: '$__all', selected: true },
@ -479,14 +479,17 @@ describe('optionsPickerReducer', () => {
...initialState,
options: [
{ text: 'All', value: '$__all', selected: false },
{ text: 'A', value: 'A', selected: false },
{ text: 'B', value: 'B', selected: false },
{ text: 'A', value: 'A', selected: true },
{ text: 'B', value: 'B', selected: true },
],
selectedValues: [
{ text: 'A', value: 'A', selected: true },
{ text: 'B', value: 'B', selected: true },
],
selectedValues: [],
});
});
it('should toggle all values to false when a option is selected', () => {
it('should toggle to ALL value when one regular option is selected, as empty values are not accepted', () => {
const { initialState } = getVariableTestContext({
options: [
{ text: 'All', value: '$__all', selected: false },
@ -503,11 +506,11 @@ describe('optionsPickerReducer', () => {
.thenStateShouldEqual({
...initialState,
options: [
{ text: 'All', value: '$__all', selected: false },
{ text: 'All', value: '$__all', selected: true },
{ text: 'A', value: 'A', selected: false },
{ text: 'B', value: 'B', selected: false },
],
selectedValues: [],
selectedValues: [{ text: 'All', value: '$__all', selected: true }],
});
});
});

@ -106,6 +106,15 @@ const updateAllSelection = (state: OptionsPickerState): OptionsPickerState => {
return state;
};
// Utility function to select all options except 'ALL_VARIABLE_VALUE'
const selectAllOptions = (options: VariableOption[]) =>
options
.filter((option) => option.value !== ALL_VARIABLE_VALUE)
.map((option) => ({
...option,
selected: true,
}));
const optionsPickerSlice = createSlice({
name: 'templating/optionsPicker',
initialState: initialOptionPickerState,
@ -178,21 +187,51 @@ const optionsPickerSlice = createSlice({
highlightIndex: nextIndex,
};
},
/**
* Toggle the 'All' option or clear selections in the Options Picker dropdown.
* 1. If 'All' is configured but not selected, and some other options are selected, it deselects all other options and selects only 'All'.
* 2. If only 'All' is selected, it deselects 'All' and selects all other available options.
* 3. If some options are selected but 'All' is not configured in the variable,
* it clears all selections and defaults to the current behavior for scenarios where 'All' is not configured.
* 4. If no options are selected, it selects all available options.
*/
toggleAllOptions: (state, action: PayloadAction): OptionsPickerState => {
if (state.selectedValues.length > 0) {
// Check if 'All' option is configured by the user and if it's selected in the dropdown
const isAllSelected = state.selectedValues.find((option) => option.value === ALL_VARIABLE_VALUE);
const allOptionConfigured = state.options.find((option) => option.value === ALL_VARIABLE_VALUE);
// If 'All' option is not selected from the dropdown, but some options are, clear all options and select 'All'
if (state.selectedValues.length > 0 && !!allOptionConfigured && !isAllSelected) {
state.selectedValues = [];
state.selectedValues.push({
text: allOptionConfigured.text ?? 'All',
value: allOptionConfigured.value,
selected: true,
});
return applyStateChanges(state, updateOptions);
}
state.selectedValues = state.options
.filter((option) => option.value !== ALL_VARIABLE_VALUE)
.map((option) => ({
...option,
selected: true,
}));
// If 'All' option is the only one selected in the dropdown, unselect "All" and select each one of the other options.
if (isAllSelected && state.selectedValues.length === 1) {
state.selectedValues = selectAllOptions(state.options);
return applyStateChanges(state, updateOptions);
}
// If some options are selected, but 'All' is not configured by the user, clear the selection and let the
// current behavior when "All" does not exist and user clear the selected items.
if (state.selectedValues.length > 0 && !allOptionConfigured) {
state.selectedValues = [];
return applyStateChanges(state, updateOptions);
}
// If no options are selected and 'All' is not selected, select all options
state.selectedValues = selectAllOptions(state.options);
return applyStateChanges(state, updateOptions);
},
updateSearchQuery: (state, action: PayloadAction<string>): OptionsPickerState => {
state.queryValue = action.payload;
return state;

@ -78,6 +78,7 @@ class VariableOptions extends PureComponent<Props> {
styles.variableOption,
{
[styles.highlighted]: index === highlightIndex,
[styles.variableAllOption]: isAllOption,
},
styles.noStyledButton
)}
@ -98,15 +99,15 @@ class VariableOptions extends PureComponent<Props> {
}
renderMultiToggle() {
const { multi, selectedValues, theme } = this.props;
const { multi, selectedValues, theme, values } = this.props;
const styles = getStyles(theme);
const isAllOptionConfigured = values.some((option) => option.value === ALL_VARIABLE_VALUE);
if (!multi) {
return null;
}
const tooltipContent = () => <Trans i18nKey="variable.picker.option-tooltip">Clear selections</Trans>;
return (
<Tooltip content={tooltipContent} placement={'top'}>
<button
@ -114,7 +115,8 @@ class VariableOptions extends PureComponent<Props> {
clearButtonStyles(theme),
styles.variableOption,
styles.variableOptionColumnHeader,
styles.noStyledButton
styles.noStyledButton,
{ [styles.noPaddingBotton]: isAllOptionConfigured }
)}
role="checkbox"
aria-checked={selectedValues.length > 1 ? 'mixed' : 'false'}
@ -201,6 +203,14 @@ const getStyles = stylesFactory((theme: GrafanaTheme2) => {
display: 'table',
width: '100%',
}),
variableAllOption: css({
borderBottom: `1px solid ${theme.colors.border.weak}`,
paddingBottom: theme.spacing(1),
}),
noPaddingBotton: css({
paddingBottom: 0,
}),
};
});

Loading…
Cancel
Save