Skip to content

Commit

Permalink
improvement: add disabled to FormGroup around options
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Gido Manders committed Apr 23, 2024
1 parent 6436297 commit bdd8c4b
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 90 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()]
]);
});
Expand Down
113 changes: 65 additions & 48 deletions src/form/CheckboxMultipleSelect/CheckboxMultipleSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -20,49 +24,49 @@ export type Text = {

export type Props<T> = Omit<FieldCompatible<T[], T[]>, 'valid'> &
FieldCompatibleWithPredeterminedOptions<T> & {
/**
* 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
Expand Down Expand Up @@ -112,7 +116,7 @@ export function CheckboxMultipleSelect<T>(props: Props<T>) {
// 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(
Expand Down Expand Up @@ -198,7 +202,12 @@ export function CheckboxMultipleSelect<T>(props: Props<T>) {
});

return (
<FormGroup check key={key} inline={horizontal}>
<FormGroup
check
key={key}
inline={horizontal}
disabled={!isOptionEnabled(option)}
>
<Label check>
<RSInput
type="checkbox"
Expand All @@ -218,9 +227,17 @@ export function CheckboxMultipleSelect<T>(props: Props<T>) {
/**
* Variant of the CheckboxMultipleSelect which can be used in a Jarb context.
*/
export const JarbCheckboxMultipleSelect = withJarb<any[], any[] | null, Props<any>>(CheckboxMultipleSelect);
export const JarbCheckboxMultipleSelect = withJarb<
any[],
any[] | null,
Props<any>
>(CheckboxMultipleSelect);

/**
* Variant of the CheckboxMultipleSelect which can be used in a final form.
*/
export const FieldCheckboxMultipleSelect = withField<any[], any[] | null, Props<any>>(CheckboxMultipleSelect);
export const FieldCheckboxMultipleSelect = withField<
any[],
any[] | null,
Props<any>
>(CheckboxMultipleSelect);
2 changes: 1 addition & 1 deletion src/form/ModalPicker/multiple/ModalPickerMultiple.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ export function ModalPickerMultiple<T>(props: Props<T>) {
});

return (
<FormGroup key={key} check>
<FormGroup key={key} check disabled={!isOptionEnabled(option)}>
<Label check>
<Input
type="checkbox"
Expand Down
2 changes: 1 addition & 1 deletion src/form/ModalPicker/single/ModalPickerSingle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ export function ModalPickerSingle<T>(props: Props<T>) {
});

return (
<FormGroup key={key} check>
<FormGroup key={key} check disabled={!isOptionEnabled(option)}>
<Label check>
<Input
type="radio"
Expand Down
44 changes: 27 additions & 17 deletions src/form/RadioGroup/RadioGroup.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,14 @@ import '@testing-library/jest-dom';

import { RadioGroup, Text } from './RadioGroup';
import { User } from '../../test/types';
import { adminUser, coordinatorUser, listOfUsers, pageOfUsers, pageOfUsersFetcher, userUser } from '../../test/fixtures';
import {
adminUser,
coordinatorUser,
listOfUsers,
pageOfUsers,
pageOfUsersFetcher,
userUser
} from '../../test/fixtures';
import { IsOptionEnabled } from '../option';

import { pageOf } from '../../utilities/page/page';
Expand Down Expand Up @@ -80,9 +87,7 @@ describe('Component: RadioGroup', () => {
hiddenLabel: !hasLabel
};

const { container, rerender } = render(
<RadioGroup<User> {...props} />
);
const { container, rerender } = render(<RadioGroup<User> {...props} />);

return {
container,
Expand Down Expand Up @@ -111,12 +116,16 @@ describe('Component: RadioGroup', () => {

test('with placeholder', () => {
setup({ hasPlaceholder: true });
expect(screen.queryByText('Please enter your subject')).toBeInTheDocument();
expect(
screen.queryByText('Please enter your subject')
).toBeInTheDocument();
});

test('without placeholder', () => {
setup({ hasPlaceholder: false });
expect(screen.queryByText('Please enter your subject')).not.toBeInTheDocument();
expect(
screen.queryByText('Please enter your subject')
).not.toBeInTheDocument();
});

test('with label', () => {
Expand All @@ -131,7 +140,9 @@ describe('Component: RadioGroup', () => {

test('horizontal', () => {
setup({ horizontal: true });
expect(screen.getAllByRole('radio')[0].parentNode?.parentNode).toHaveClass('form-check-inline');
expect(
screen.getAllByRole('radio')[0].parentNode?.parentNode
).toHaveClass('form-check-inline');
});
});

Expand Down Expand Up @@ -196,11 +207,14 @@ describe('Component: RadioGroup', () => {
expect(screen.getAllByRole('radio')[1]).toBeDisabled();
expect(screen.getAllByRole('radio')[2]).toBeDisabled();

expect(isOptionEnabledSpy).toHaveBeenCalledTimes(3);
expect(isOptionEnabledSpy).toHaveBeenCalledTimes(6);
expect(isOptionEnabledSpy.mock.calls).toEqual([
[ adminUser() ],
[ coordinatorUser() ],
[ userUser() ]
[adminUser()],
[adminUser()],
[coordinatorUser()],
[coordinatorUser()],
[userUser()],
[userUser()]
]);
});
});
Expand All @@ -222,9 +236,7 @@ describe('Component: RadioGroup', () => {
value: undefined
};

rerender(
<RadioGroup {...newProps} />
);
rerender(<RadioGroup {...newProps} />);

expect(screen.getAllByRole('radio')[0]).not.toBeChecked();
expect(screen.getAllByRole('radio')[1]).not.toBeChecked();
Expand All @@ -246,9 +258,7 @@ describe('Component: RadioGroup', () => {
value: coordinatorUser()
};

rerender(
<RadioGroup {...newProps} />
);
rerender(<RadioGroup {...newProps} />);

expect(screen.getAllByRole('radio')[0]).not.toBeChecked();
expect(screen.getAllByRole('radio')[1]).toBeChecked();
Expand Down
53 changes: 31 additions & 22 deletions src/form/RadioGroup/RadioGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { FormGroup, Input, Label } from 'reactstrap';

import { withJarb } from '../withJarb/withJarb';
import { t } from '../../utilities/translation/translation';
import { FieldCompatibleWithPredeterminedOptions, getKeyForOption, isOptionSelected } from '../option';
import {
FieldCompatibleWithPredeterminedOptions,
getKeyForOption,
isOptionSelected
} from '../option';
import { alwaysTrue, doBlur } from '../utils';
import { Loading } from '../../core/Loading/Loading';
import { useOptions } from '../useOptions';
Expand All @@ -26,26 +30,26 @@ export type Text = {

export type Props<T> = FieldCompatible<T, T | undefined> &
FieldCompatibleWithPredeterminedOptions<T> & {
/**
* Optionally customized text within the component.
* This text should already be translated.
*/
text?: Text;

/**
* Whether to show the RadioGroup horizontally.
*
* Defaults to `false`
*/
horizontal?: boolean;

/**
* Whether to show a "clear" button.
*
* Defaults to `false`
*/
canClear?: boolean;
};
/**
* Optionally customized text within the component.
* This text should already be translated.
*/
text?: Text;

/**
* Whether to show the RadioGroup horizontally.
*
* Defaults to `false`
*/
horizontal?: boolean;

/**
* Whether to show a "clear" button.
*
* Defaults to `false`
*/
canClear?: boolean;
};

/**
* RadioGroup is a form element for which the value can be selected
Expand Down Expand Up @@ -147,7 +151,12 @@ export function RadioGroup<T>(props: Props<T>) {
});

return (
<FormGroup key={key} check inline={horizontal}>
<FormGroup
key={key}
check
inline={horizontal}
disabled={!isOptionEnabled(option)}
>
<Label check>
<Input
type="radio"
Expand Down

0 comments on commit bdd8c4b

Please sign in to comment.