Skip to content

Commit 1d7076f

Browse files
evanaronsontyhonchik
evanaronson
and
tyhonchik
authored
feat(APP-823): Decoded action form input UX improvements (#701)
Co-authored-by: Andrew <30229764+tyhonchik@users.noreply.github.com>
1 parent 172430f commit 1d7076f

13 files changed

Lines changed: 336 additions & 47 deletions

File tree

‎.changeset/proposal-actions-decoder-fields.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@aragon/gov-ui-kit": minor
3+
---
4+
5+
Display the parameter type next to the parameter name instead of as placeholder on `ProposalActionsDecoder` fields, render boolean parameters as radio group with no default selection on edit mode

‎.gitattributes‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* text=auto eol=lf

‎src/core/components/forms/inputContainer/inputContainer.api.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export interface IInputContainerBaseProps {
1818
/**
1919
* Label of the input.
2020
*/
21-
label?: string;
21+
label?: ReactNode;
2222
/**
2323
* Variant of the input.
2424
* @default default

‎src/core/components/forms/radioGroup/radioGroup.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { RadioGroup as PrimitiveRadioGroup } from '@radix-ui/react-radio-group';
22
import classNames from 'classnames';
3-
import { forwardRef, type ReactNode } from 'react';
3+
import { type FocusEventHandler, forwardRef, type ReactNode } from 'react';
44
import { useRandomId } from '../../../hooks';
55
import { type IInputContainerBaseProps, InputContainer } from '../inputContainer';
66

@@ -22,6 +22,10 @@ export interface IRadioGroupProps
2222
* The name of the radio group.
2323
*/
2424
name?: string;
25+
/**
26+
* Callback when the radio group loses focus.
27+
*/
28+
onBlur?: FocusEventHandler<HTMLDivElement>;
2529
/**
2630
* Whether the radio group is disabled.
2731
*/
@@ -37,7 +41,7 @@ export interface IRadioGroupProps
3741
}
3842

3943
export const RadioGroup = forwardRef<HTMLDivElement, IRadioGroupProps>((props, ref) => {
40-
const { className, value, defaultValue, onValueChange, name, disabled, children, ...otherProps } = props;
44+
const { className, value, defaultValue, onValueChange, name, onBlur, disabled, children, ...otherProps } = props;
4145

4246
const randomId = useRandomId();
4347

@@ -49,6 +53,7 @@ export const RadioGroup = forwardRef<HTMLDivElement, IRadioGroupProps>((props, r
4953
disabled={disabled}
5054
id={randomId}
5155
name={name}
56+
onBlur={onBlur}
5257
onValueChange={onValueChange}
5358
ref={ref}
5459
value={value}

‎src/modules/assets/copy/modulesCopy.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ export const modulesCopy = {
6161
copyData: 'Copy data',
6262
add: 'Add',
6363
noParametersMessage: 'This write function has no parameters.',
64+
booleanTrue: 'true',
65+
booleanFalse: 'false',
6466
validation: {
6567
required: (label: string) => `${label} is required.`,
6668
boolean: (label: string) => `${label} must be set to "true" or "false".`,

‎src/modules/components/proposal/proposalActions/proposalActionsDecoder/proposalActionsDecoder.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ export const ProposalActionsDecoder: React.FC<IProposalActionsDecoderProps> = (p
104104
const handleCopyDataClick = () => clipboardUtils.copy(action.data);
105105

106106
return (
107-
<div className={classNames('flex w-full flex-col gap-3', className)} {...otherProps}>
107+
<div className={classNames('flex w-full flex-col gap-5 md:gap-6', className)} {...otherProps}>
108108
{(view === ProposalActionsDecoderView.RAW || isPayableAction) && (
109109
<ProposalActionsDecoderTextField
110110
fieldName="value"

‎src/modules/components/proposal/proposalActions/proposalActionsDecoder/proposalActionsDecoderBooleanField/index.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
export {
2+
type IProposalActionsDecoderBooleanFieldProps,
3+
ProposalActionsDecoderBooleanField,
4+
} from './proposalActionsDecoderBooleanField';

‎src/modules/components/proposal/proposalActions/proposalActionsDecoder/proposalActionsDecoderBooleanField/proposalActionsDecoderBooleanField.test.tsx‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
import { render, screen } from '@testing-library/react';
2+
import userEvent from '@testing-library/user-event';
3+
import * as ReactHookForm from 'react-hook-form';
4+
import { modulesCopy } from '../../../../../assets';
5+
import {
6+
type IProposalActionsDecoderBooleanFieldProps,
7+
ProposalActionsDecoderBooleanField,
8+
} from './proposalActionsDecoderBooleanField';
9+
10+
jest.mock('react-hook-form', () => ({
11+
...jest.requireActual<typeof ReactHookForm>('react-hook-form'),
12+
__esModule: true,
13+
}));
14+
15+
describe('<ProposalActionsDecoderBooleanField /> component', () => {
16+
const useControllerSpy = jest.spyOn(ReactHookForm, 'useController');
17+
18+
beforeEach(() => {
19+
useControllerSpy.mockReturnValue({
20+
fieldState: { error: undefined },
21+
field: { value: undefined, onBlur: jest.fn(), onChange: jest.fn(), ref: jest.fn() },
22+
} as unknown as ReactHookForm.UseControllerReturn);
23+
});
24+
25+
afterEach(() => {
26+
useControllerSpy.mockReset();
27+
});
28+
29+
const createTestComponent = (props?: Partial<IProposalActionsDecoderBooleanFieldProps>) => {
30+
const completeProps: IProposalActionsDecoderBooleanFieldProps = {
31+
parameter: { name: 'boolParam', type: 'bool', value: undefined },
32+
fieldName: 'testName',
33+
...props,
34+
};
35+
36+
return <ProposalActionsDecoderBooleanField {...completeProps} />;
37+
};
38+
39+
it('registers the field on the form context and renders a radio group with no default selection', () => {
40+
const fieldName = 'parameter.0.value';
41+
render(createTestComponent({ fieldName }));
42+
43+
expect(useControllerSpy).toHaveBeenCalledWith({
44+
name: fieldName,
45+
rules: { validate: expect.any(Function) as unknown },
46+
});
47+
48+
const radios = screen.getAllByRole('radio');
49+
expect(radios).toHaveLength(2);
50+
radios.forEach((radio) => expect(radio).not.toBeChecked());
51+
expect(screen.getByRole('radio', { name: modulesCopy.proposalActionsDecoder.booleanTrue })).toBeInTheDocument();
52+
expect(
53+
screen.getByRole('radio', { name: modulesCopy.proposalActionsDecoder.booleanFalse }),
54+
).toBeInTheDocument();
55+
});
56+
57+
it('prefixes the field name with the formPrefix property when set', () => {
58+
const fieldName = 'value';
59+
const formPrefix = 'actions.0';
60+
render(createTestComponent({ fieldName, formPrefix }));
61+
expect(useControllerSpy).toHaveBeenCalledWith(expect.objectContaining({ name: `${formPrefix}.${fieldName}` }));
62+
});
63+
64+
it('renders the parameter name, type and notice as field labels', () => {
65+
const parameter = { name: 'tryExecute', notice: 'description', type: 'bool', value: undefined };
66+
render(createTestComponent({ parameter }));
67+
expect(screen.getByText(`(${parameter.type})`)).toBeInTheDocument();
68+
expect(screen.getByText(parameter.notice)).toBeInTheDocument();
69+
// eslint-disable-next-line testing-library/no-node-access
70+
expect(screen.getByText(`(${parameter.type})`).parentElement).toHaveTextContent(
71+
`${parameter.name} (${parameter.type})`,
72+
);
73+
});
74+
75+
it('does not render the field labels when hideLabels property is set to true', () => {
76+
const parameter = { name: 'tryExecute', notice: 'description', type: 'bool', value: undefined };
77+
render(createTestComponent({ parameter, hideLabels: true }));
78+
expect(screen.queryByText(`(${parameter.type})`)).not.toBeInTheDocument();
79+
expect(screen.queryByText(parameter.notice)).not.toBeInTheDocument();
80+
});
81+
82+
it('checks the radio item matching the current field value', () => {
83+
useControllerSpy.mockReturnValue({
84+
fieldState: { error: undefined },
85+
field: { value: false, onChange: jest.fn() },
86+
} as unknown as ReactHookForm.UseControllerReturn);
87+
render(createTestComponent());
88+
expect(screen.getByRole('radio', { name: modulesCopy.proposalActionsDecoder.booleanFalse })).toBeChecked();
89+
expect(screen.getByRole('radio', { name: modulesCopy.proposalActionsDecoder.booleanTrue })).not.toBeChecked();
90+
});
91+
92+
it('triggers the onChange callback with a boolean value on radio selection', async () => {
93+
const onChange = jest.fn();
94+
useControllerSpy.mockReturnValue({
95+
fieldState: { error: undefined },
96+
field: { value: undefined, onBlur: jest.fn(), onChange, ref: jest.fn() },
97+
} as unknown as ReactHookForm.UseControllerReturn);
98+
render(createTestComponent());
99+
await userEvent.click(screen.getByRole('radio', { name: modulesCopy.proposalActionsDecoder.booleanTrue }));
100+
expect(onChange).toHaveBeenCalledWith(true);
101+
});
102+
103+
it('passes form blur and ref handlers to the radio group', () => {
104+
const onBlur = jest.fn();
105+
const ref = jest.fn();
106+
useControllerSpy.mockReturnValue({
107+
fieldState: { error: undefined },
108+
field: { value: undefined, onBlur, onChange: jest.fn(), ref },
109+
} as unknown as ReactHookForm.UseControllerReturn);
110+
render(createTestComponent());
111+
const radioGroup = screen.getByRole('radiogroup');
112+
expect(ref).toHaveBeenCalledWith(radioGroup);
113+
114+
radioGroup.focus();
115+
radioGroup.blur();
116+
expect(onBlur).toHaveBeenCalled();
117+
});
118+
119+
it('renders an alert when the form field has an error', () => {
120+
const error = { message: 'invalid field' };
121+
useControllerSpy.mockReturnValue({
122+
fieldState: { error },
123+
field: { value: undefined, onBlur: jest.fn(), onChange: jest.fn(), ref: jest.fn() },
124+
} as unknown as ReactHookForm.UseControllerReturn);
125+
render(createTestComponent());
126+
expect(screen.getByRole('alert')).toBeInTheDocument();
127+
expect(screen.getByText(error.message)).toBeInTheDocument();
128+
});
129+
});

‎src/modules/components/proposal/proposalActions/proposalActionsDecoder/proposalActionsDecoderBooleanField/proposalActionsDecoderBooleanField.tsx‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { useController } from 'react-hook-form';
2+
import { Radio, RadioGroup } from '../../../../../../core';
3+
import { useGukModulesContext } from '../../../../gukModulesProvider';
4+
import type { IProposalActionInputDataParameter } from '../../proposalActionsDefinitions';
5+
import type { IProposalActionsDecoderProps } from '../proposalActionsDecoder.api';
6+
import { type ProposalActionsFieldValue, proposalActionsDecoderUtils } from '../proposalActionsDecoderUtils';
7+
8+
export interface IProposalActionsDecoderBooleanFieldProps extends Pick<IProposalActionsDecoderProps, 'formPrefix'> {
9+
/**
10+
* Action parameter to be rendered.
11+
*/
12+
parameter: IProposalActionInputDataParameter;
13+
/**
14+
* Name of the input field.
15+
*/
16+
fieldName: string;
17+
/**
18+
* Hides the default labels when set to true.
19+
*/
20+
hideLabels?: boolean;
21+
}
22+
23+
export const ProposalActionsDecoderBooleanField: React.FC<IProposalActionsDecoderBooleanFieldProps> = (props) => {
24+
const { parameter, fieldName, formPrefix, hideLabels } = props;
25+
const { name, notice, type } = parameter;
26+
27+
const { copy } = useGukModulesContext();
28+
29+
const errorMessages = copy.proposalActionsDecoder.validation;
30+
const validateFunction = (value: ProposalActionsFieldValue) =>
31+
proposalActionsDecoderUtils.validateValue(value, { label: name, type, required: true, errorMessages });
32+
33+
const formFieldName = proposalActionsDecoderUtils.getFieldName(fieldName, formPrefix);
34+
const { fieldState, field } = useController<Record<string, ProposalActionsFieldValue>>({
35+
name: formFieldName,
36+
rules: { validate: validateFunction },
37+
});
38+
39+
const { error } = fieldState;
40+
const alert = error?.message == null ? undefined : { message: error.message, variant: 'critical' as const };
41+
42+
const label = hideLabels ? undefined : (
43+
<>
44+
{name} <span className="text-neutral-500">({type})</span>
45+
</>
46+
);
47+
48+
const handleValueChange = (value: string) => field.onChange(value === 'true');
49+
50+
return (
51+
<RadioGroup
52+
alert={alert}
53+
helpText={hideLabels ? undefined : notice}
54+
label={label}
55+
name={formFieldName}
56+
onBlur={field.onBlur}
57+
onValueChange={handleValueChange}
58+
ref={field.ref}
59+
value={field.value?.toString() ?? ''}
60+
>
61+
<Radio label={copy.proposalActionsDecoder.booleanTrue} value="true" />
62+
<Radio label={copy.proposalActionsDecoder.booleanFalse} value="false" />
63+
</RadioGroup>
64+
);
65+
};

‎src/modules/components/proposal/proposalActions/proposalActionsDecoder/proposalActionsDecoderField/proposalActionsDecoderField.test.tsx‎

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,15 @@ describe('<ProposalActionsDecoderField /> component', () => {
4646
it('returns a simple text field when parameter type is not a nested type', () => {
4747
const parameter = { name: 'boolParam', type: 'bool', value: undefined };
4848
render(createTestComponent({ parameter }));
49-
expect(screen.getByRole('textbox', { name: parameter.name })).toBeInTheDocument();
49+
expect(screen.getByRole('textbox', { name: `${parameter.name} (${parameter.type})` })).toBeInTheDocument();
50+
});
51+
52+
it('renders a radio group instead of a text field for boolean parameters when mode is edit', () => {
53+
const parameter = { name: 'boolParam', type: 'bool', value: undefined };
54+
const mode = ProposalActionsDecoderMode.EDIT;
55+
render(createTestComponent({ parameter, mode }));
56+
expect(screen.getAllByRole('radio')).toHaveLength(2);
57+
expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
5058
});
5159

5260
it('renders a hidden text-field for nested types', () => {
@@ -66,7 +74,7 @@ describe('<ProposalActionsDecoderField /> component', () => {
6674
const parameter = { name: 'tupleType', type: 'tuple', value: ['0x00', true, '2231'], components };
6775
render(createTestComponent({ parameter }));
6876
components.forEach((component, index) => {
69-
const textField = screen.getByRole('textbox', { name: component.name });
77+
const textField = screen.getByRole('textbox', { name: `${component.name} (${component.type})` });
7078
expect(textField).toBeInTheDocument();
7179
expect(textField).toHaveDisplayValue(parameter.value[index].toString());
7280
});
@@ -76,8 +84,15 @@ describe('<ProposalActionsDecoderField /> component', () => {
7684
const parameter = { name: 'arrayType', type: 'uint[]', value: ['12', '777', '465413', '0'] };
7785
render(createTestComponent({ parameter }));
7886
expect(screen.getAllByText(parameter.name)).toHaveLength(2); // 2 because of the hidden array input
87+
parameter.value.forEach((_, index) =>
88+
expect(screen.getByText(`[${index.toString()}]:`)).toHaveClass('text-neutral-500'),
89+
);
7990
const [, ...textInputs] = screen.getAllByRole('textbox');
8091
expect(textInputs).toHaveLength(parameter.value.length);
92+
// eslint-disable-next-line testing-library/no-node-access
93+
expect(screen.getByText('[0]:').parentElement).toContainElement(textInputs[0]);
94+
// eslint-disable-next-line testing-library/no-node-access
95+
expect(screen.getByText('[0]:').parentElement).toHaveClass('items-center');
8196
textInputs.forEach((input, index) => expect(input).toHaveDisplayValue(parameter.value[index]));
8297
});
8398

@@ -110,6 +125,7 @@ describe('<ProposalActionsDecoderField /> component', () => {
110125
.getAllByRole('button')
111126
.filter((button) => within(button).queryByTestId(IconType.CLOSE) != null);
112127
expect(removeButtons).toHaveLength(parameter.value.length);
128+
removeButtons.forEach((button) => expect(button).toHaveClass('h-8', 'w-8'));
113129
await userEvent.click(removeButtons[0]);
114130

115131
expect(screen.getAllByRole('textbox')).toHaveLength(parameter.value.length);
@@ -145,11 +161,25 @@ describe('<ProposalActionsDecoderField /> component', () => {
145161
.getAllByRole('button')
146162
.filter((button) => within(button).queryByTestId(IconType.CLOSE) != null);
147163
expect(removeButtons).toHaveLength(parameter.value.length);
164+
removeButtons.forEach((button) => expect(button).toHaveClass('h-8', 'w-8'));
165+
parameter.value.forEach((_, index) =>
166+
expect(screen.getByText(`[${index.toString()}]:`)).toHaveClass('text-neutral-500'),
167+
);
168+
// eslint-disable-next-line testing-library/no-node-access
169+
expect(screen.getByText('[0]:').parentElement).toContainElement(
170+
screen.getAllByRole('textbox', {
171+
name: `${parameterComponents[0].name} (${parameterComponents[0].type})`,
172+
})[0],
173+
);
174+
// eslint-disable-next-line testing-library/no-node-access
175+
expect(screen.getByText('[0]:').parentElement).toHaveClass('items-start');
148176
await userEvent.click(removeButtons[1]);
149177

150-
expect(screen.getAllByRole('textbox', { name: parameterComponents[0].name })).toHaveLength(
151-
parameter.value.length - 1,
152-
);
178+
expect(
179+
screen.getAllByRole('textbox', {
180+
name: `${parameterComponents[0].name} (${parameterComponents[0].type})`,
181+
}),
182+
).toHaveLength(parameter.value.length - 1);
153183
expect(unregister).toHaveBeenCalledWith(`${formPrefix}.${fieldName}.2`);
154184
expect(setValue).toHaveBeenCalledWith(`${formPrefix}.${fieldName}`, [
155185
['1', false],

0 commit comments

Comments
 (0)