From bdd8c4b896d5b9f65ddc756be1aeefd9968d1ebc Mon Sep 17 00:00:00 2001 From: Gido Manders Date: Tue, 23 Apr 2024 23:52:55 +0200 Subject: [PATCH] improvement: add disabled to FormGroup around options When displaying options in the CheckboxMultipleSelect, RadioGroup, or ModalPicker components, the FormGroup wrapper around disabled options should also be disabled. Added disabled to FormGroup around options that can be displayed in disabled status. --- .../CheckboxMultipleSelect.test.tsx | 5 +- .../CheckboxMultipleSelect.tsx | 113 ++++++++++-------- .../multiple/ModalPickerMultiple.tsx | 2 +- .../ModalPicker/single/ModalPickerSingle.tsx | 2 +- src/form/RadioGroup/RadioGroup.test.tsx | 44 ++++--- src/form/RadioGroup/RadioGroup.tsx | 53 ++++---- 6 files changed, 129 insertions(+), 90 deletions(-) diff --git a/src/form/CheckboxMultipleSelect/CheckboxMultipleSelect.test.tsx b/src/form/CheckboxMultipleSelect/CheckboxMultipleSelect.test.tsx index d4ff6704d..bd0e157cc 100644 --- a/src/form/CheckboxMultipleSelect/CheckboxMultipleSelect.test.tsx +++ b/src/form/CheckboxMultipleSelect/CheckboxMultipleSelect.test.tsx @@ -244,10 +244,13 @@ describe('Component: CheckboxMultipleSelect', () => { expect(checkboxes[1]).toBeDisabled(); expect(checkboxes[2]).toBeDisabled(); - expect(isOptionEnabledSpy).toHaveBeenCalledTimes(3); + expect(isOptionEnabledSpy).toHaveBeenCalledTimes(6); expect(isOptionEnabledSpy.mock.calls).toEqual([ [adminUser()], + [adminUser()], + [coordinatorUser()], [coordinatorUser()], + [userUser()], [userUser()] ]); }); diff --git a/src/form/CheckboxMultipleSelect/CheckboxMultipleSelect.tsx b/src/form/CheckboxMultipleSelect/CheckboxMultipleSelect.tsx index c581f3abd..da0a132e5 100644 --- a/src/form/CheckboxMultipleSelect/CheckboxMultipleSelect.tsx +++ b/src/form/CheckboxMultipleSelect/CheckboxMultipleSelect.tsx @@ -3,7 +3,11 @@ import React from 'react'; import { FormGroup, Input as RSInput, Label } from 'reactstrap'; import { Loading } from '../../core/Loading/Loading'; import { t } from '../../utilities/translation/translation'; -import { FieldCompatibleWithPredeterminedOptions, getKeyForOption, isOptionSelected } from '../option'; +import { + FieldCompatibleWithPredeterminedOptions, + getKeyForOption, + isOptionSelected +} from '../option'; import { FieldCompatible } from '../types'; import { useOptions } from '../useOptions'; import { alwaysTrue, doBlur } from '../utils'; @@ -20,49 +24,49 @@ export type Text = { export type Props = Omit, 'valid'> & FieldCompatibleWithPredeterminedOptions & { - /** - * Optionally customized text within the component. - * This text should already be translated. - */ - text?: Text; - - /** - * Whether to show the CheckboxMultipleSelect horizontally. - * - * Defaults to `false` - */ - horizontal?: boolean; - - /** - * Whether the form element should always contain the value - * which is selected. - * - * It should be `true` when using it in the following situation: - * The form element receives a value which is no longer part - * of the options. In that case it is handy to have the value - * automatically added to the options, so the user still sees - * the select value. - * - * It should be `false` when using it in the following situations: - * - * 1. The selected `value` is displayed separately from the - * selection of values. In this case it does not make sense - * to add the `value` to the options because it is already - * displayed. - * - * 2. The form element represents a sub selection of a larger - * value. For example, you have an array of permissions of what - * the user can do in the system, visually you display grouped - * by parts of the domain. This means giving the same `value` - * to various form element components to represent parts of the - * same array of permissions. If `optionsShouldAlwaysContainValue` - * were `true` it would add all permissions to each permission - * group. - * - * This value is `true` by default. - */ - optionsShouldAlwaysContainValue?: boolean; -}; + /** + * Optionally customized text within the component. + * This text should already be translated. + */ + text?: Text; + + /** + * Whether to show the CheckboxMultipleSelect horizontally. + * + * Defaults to `false` + */ + horizontal?: boolean; + + /** + * Whether the form element should always contain the value + * which is selected. + * + * It should be `true` when using it in the following situation: + * The form element receives a value which is no longer part + * of the options. In that case it is handy to have the value + * automatically added to the options, so the user still sees + * the select value. + * + * It should be `false` when using it in the following situations: + * + * 1. The selected `value` is displayed separately from the + * selection of values. In this case it does not make sense + * to add the `value` to the options because it is already + * displayed. + * + * 2. The form element represents a sub selection of a larger + * value. For example, you have an array of permissions of what + * the user can do in the system, visually you display grouped + * by parts of the domain. This means giving the same `value` + * to various form element components to represent parts of the + * same array of permissions. If `optionsShouldAlwaysContainValue` + * were `true` it would add all permissions to each permission + * group. + * + * This value is `true` by default. + */ + optionsShouldAlwaysContainValue?: boolean; + }; /** * CheckboxMultipleSelect is a form element for which the values can @@ -112,7 +116,7 @@ export function CheckboxMultipleSelect(props: Props) { // Otherwise, the selection will be the same as the value, which // causes values to be committed and the cancel button will not // do anything. - let selected = Array.isArray(value) ? [ ...value ] : []; + let selected = Array.isArray(value) ? [...value] : []; if (isSelected) { selected = selected.filter( @@ -198,7 +202,12 @@ export function CheckboxMultipleSelect(props: Props) { }); return ( - +