Skip to content

Commit

Permalink
refactor: Code cleanup in code list editor (#14258)
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasEng authored Dec 12, 2024
1 parent 94be9b9 commit 0f7b868
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,13 @@ const codeList: CodeList = [
helpText: 'Test 3 help text',
},
];
const onBlurAny = jest.fn();
const onChange = jest.fn();
const onInvalid = jest.fn();
const defaultProps: StudioCodeListEditorProps = {
codeList,
texts,
onBlurAny,
onChange,
onInvalid,
};
Expand Down Expand Up @@ -119,8 +121,7 @@ describe('StudioCodeListEditor', () => {
const labelInput = screen.getByRole('textbox', { name: texts.itemLabel(1) });
const newValue = 'new text';
await user.type(labelInput, newValue);
await user.tab();
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledTimes(newValue.length);
expect(onChange).toHaveBeenLastCalledWith([
{ ...codeList[0], label: newValue },
codeList[1],
Expand All @@ -134,8 +135,7 @@ describe('StudioCodeListEditor', () => {
const valueInput = screen.getByRole('textbox', { name: texts.itemValue(1) });
const newValue = 'new text';
await user.type(valueInput, newValue);
await user.tab();
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledTimes(newValue.length);
expect(onChange).toHaveBeenLastCalledWith([
{ ...codeList[0], value: newValue },
codeList[1],
Expand All @@ -149,8 +149,7 @@ describe('StudioCodeListEditor', () => {
const descriptionInput = screen.getByRole('textbox', { name: texts.itemDescription(1) });
const newValue = 'new text';
await user.type(descriptionInput, newValue);
await user.tab();
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledTimes(newValue.length);
expect(onChange).toHaveBeenLastCalledWith([
{ ...codeList[0], description: newValue },
codeList[1],
Expand All @@ -164,8 +163,7 @@ describe('StudioCodeListEditor', () => {
const helpTextInput = screen.getByRole('textbox', { name: texts.itemHelpText(1) });
const newValue = 'new text';
await user.type(helpTextInput, newValue);
await user.tab();
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveBeenCalledTimes(newValue.length);
expect(onChange).toHaveBeenLastCalledWith([
{ ...codeList[0], helpText: newValue },
codeList[1],
Expand Down Expand Up @@ -196,6 +194,21 @@ describe('StudioCodeListEditor', () => {
]);
});

it('Calls the onBlurAny callback with the current code list when an item in the table is blurred', async () => {
const user = userEvent.setup();
renderCodeListEditor();
const valueInput = screen.getByRole('textbox', { name: texts.itemValue(1) });
const newValue = 'new text';
await user.type(valueInput, newValue);
await user.tab();
expect(onBlurAny).toHaveBeenCalledTimes(1);
expect(onBlurAny).toHaveBeenLastCalledWith([
{ ...codeList[0], value: newValue },
codeList[1],
codeList[2],
]);
});

it('Updates itself when the user changes something', async () => {
const user = userEvent.setup();
renderCodeListEditor();
Expand Down Expand Up @@ -267,8 +280,7 @@ describe('StudioCodeListEditor', () => {
const validValueInput = screen.getByRole('textbox', { name: texts.itemValue(3) });
const newValue = 'new value';
await user.type(validValueInput, newValue);
await user.tab();
expect(onInvalid).toHaveBeenCalledTimes(1);
expect(onInvalid).toHaveBeenCalledTimes(newValue.length);
});

it('Does not trigger onInvalid if an invalid code list is changed to a valid state', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,32 @@ import { areThereCodeListErrors, findCodeListErrors, isCodeListValid } from './v
import type { ValueErrorMap } from './types/ValueErrorMap';
import { StudioFieldset } from '../StudioFieldset';
import { StudioErrorMessage } from '../StudioErrorMessage';
import type { Override } from '../../types/Override';
import type { StudioInputTableProps } from '../StudioInputTable/StudioInputTable';

export type StudioCodeListEditorProps = {
codeList: CodeList;
onChange: (codeList: CodeList) => void;
onBlurAny?: (codeList: CodeList) => void;
onChange?: (codeList: CodeList) => void;
onInvalid?: () => void;
texts: CodeListEditorTexts;
};

export function StudioCodeListEditor({
codeList,
onBlurAny,
onChange,
onInvalid,
texts,
}: StudioCodeListEditorProps): ReactElement {
return (
<StudioCodeListEditorContext.Provider value={{ texts }}>
<StatefulCodeListEditor codeList={codeList} onChange={onChange} onInvalid={onInvalid} />
<StatefulCodeListEditor
codeList={codeList}
onBlurAny={onBlurAny}
onChange={onChange}
onInvalid={onInvalid}
/>
</StudioCodeListEditorContext.Provider>
);
}
Expand All @@ -48,6 +57,7 @@ type StatefulCodeListEditorProps = Omit<StudioCodeListEditorProps, 'texts'>;

function StatefulCodeListEditor({
codeList: defaultCodeList,
onBlurAny,
onChange,
onInvalid,
}: StatefulCodeListEditorProps): ReactElement {
Expand All @@ -60,18 +70,32 @@ function StatefulCodeListEditor({
const handleChange = useCallback(
(newCodeList: CodeList) => {
setCodeList(newCodeList);
isCodeListValid(newCodeList) ? onChange(newCodeList) : onInvalid?.();
isCodeListValid(newCodeList) ? onChange?.(newCodeList) : onInvalid?.();
},
[onChange, onInvalid],
);

return <ControlledCodeListEditor codeList={codeList} onChange={handleChange} />;
const handleBlurAny = useCallback(() => {
onBlurAny?.(codeList);
}, [onBlurAny, codeList]);

return (
<ControlledCodeListEditor
codeList={codeList}
onBlurAny={handleBlurAny}
onChange={handleChange}
/>
);
}

type InternalCodeListEditorProps = Omit<StatefulCodeListEditorProps, 'onInvalid'>;
type InternalCodeListEditorProps = Override<
Pick<StudioInputTableProps, 'onBlurAny'>,
Omit<StatefulCodeListEditorProps, 'onInvalid'>
>;

function ControlledCodeListEditor({
codeList,
onBlurAny,
onChange,
}: InternalCodeListEditorProps): ReactElement {
const { texts } = useStudioCodeListEditorContext();
Expand All @@ -86,12 +110,18 @@ function ControlledCodeListEditor({

return (
<StudioFieldset legend={texts.codeList} className={classes.codeListEditor} ref={fieldsetRef}>
<CodeListTable codeList={codeList} errorMap={errorMap} onChange={onChange} />
<CodeListTable
codeList={codeList}
errorMap={errorMap}
onBlurAny={onBlurAny}
onChange={onChange}
/>
<AddButton onClick={handleAddButtonClick} />
<Errors errorMap={errorMap} />
</StudioFieldset>
);
}

type InternalCodeListEditorWithErrorsProps = InternalCodeListEditorProps & ErrorsProps;

function CodeListTable(props: InternalCodeListEditorWithErrorsProps): ReactElement {
Expand All @@ -107,11 +137,14 @@ function EmptyCodeListTable(): ReactElement {
return <StudioParagraph size='small'>{texts.emptyCodeList}</StudioParagraph>;
}

function CodeListTableWithContent(props: InternalCodeListEditorWithErrorsProps): ReactElement {
function CodeListTableWithContent({
onBlurAny,
...rest
}: InternalCodeListEditorWithErrorsProps): ReactElement {
return (
<StudioInputTable>
<StudioInputTable onBlurAny={onBlurAny}>
<TableHeadings />
<TableBody {...props} />
<TableBody {...rest} />
</StudioInputTable>
);
}
Expand Down Expand Up @@ -145,7 +178,7 @@ function TableBody({
[codeList, onChange],
);

const handleBlur = useCallback(
const handleChange = useCallback(
(index: number, newItem: CodeListItem) => {
const updatedCodeList = changeCodeListItem(codeList, index, newItem);
onChange(updatedCodeList);
Expand All @@ -161,7 +194,7 @@ function TableBody({
item={item}
key={index}
number={index + 1}
onBlur={(newItem) => handleBlur(index, newItem)}
onChange={(newItem) => handleChange(index, newItem)}
onDeleteButtonClick={() => handleDeleteButtonClick(index)}
/>
))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,49 +13,49 @@ type StudioCodeListEditorRowProps = {
error: ValueError | null;
item: CodeListItem;
number: number;
onBlur: (newItem: CodeListItem) => void;
onChange: (newItem: CodeListItem) => void;
onDeleteButtonClick: () => void;
};

export function StudioCodeListEditorRow({
error,
item,
number,
onBlur,
onChange,
onDeleteButtonClick,
}: StudioCodeListEditorRowProps) {
const { texts } = useStudioCodeListEditorContext();

const handleLabelChange = useCallback(
(label: string) => {
const updatedItem = changeLabel(item, label);
onBlur(updatedItem);
onChange(updatedItem);
},
[item, onBlur],
[item, onChange],
);

const handleDescriptionChange = useCallback(
(description: string) => {
const updatedItem = changeDescription(item, description);
onBlur(updatedItem);
onChange(updatedItem);
},
[item, onBlur],
[item, onChange],
);

const handleValueChange = useCallback(
(value: string) => {
const updatedItem = changeValue(item, value);
onBlur(updatedItem);
onChange(updatedItem);
},
[item, onBlur],
[item, onChange],
);

const handleHelpTextChange = useCallback(
(helpText: string) => {
const updatedItem = changeHelpText(item, helpText);
onBlur(updatedItem);
onChange(updatedItem);
},
[item, onBlur],
[item, onChange],
);

return (
Expand All @@ -64,22 +64,22 @@ export function StudioCodeListEditorRow({
autoComplete='off'
error={error && texts.valueErrors[error]}
label={texts.itemValue(number)}
onBlur={handleValueChange}
onChange={handleValueChange}
value={item.value}
/>
<TextfieldCell
label={texts.itemLabel(number)}
onBlur={handleLabelChange}
onChange={handleLabelChange}
value={item.label}
/>
<TextfieldCell
label={texts.itemDescription(number)}
onBlur={handleDescriptionChange}
onChange={handleDescriptionChange}
value={item.description}
/>
<TextfieldCell
label={texts.itemHelpText(number)}
onBlur={handleHelpTextChange}
onChange={handleHelpTextChange}
value={item.helpText}
/>
<DeleteButtonCell onClick={onDeleteButtonClick} number={number} />
Expand All @@ -90,23 +90,23 @@ export function StudioCodeListEditorRow({
type TextfieldCellProps = {
error?: string;
label: string;
onBlur: (newString: string) => void;
onChange: (newString: string) => void;
value: CodeListItemValue;
autoComplete?: HTMLInputAutoCompleteAttribute;
};

function TextfieldCell({ error, label, value, onBlur, autoComplete }: TextfieldCellProps) {
function TextfieldCell({ error, label, value, onChange, autoComplete }: TextfieldCellProps) {
const ref = useRef<HTMLInputElement>(null);

useEffect((): void => {
ref.current?.setCustomValidity(error || '');
}, [error]);

const handleBlur = useCallback(
const handleChange = useCallback(
(event: React.ChangeEvent<HTMLInputElement>): void => {
onBlur(event.target.value);
onChange(event.target.value);
},
[onBlur],
[onChange],
);

const handleFocus = useCallback((event: FocusEvent<HTMLInputElement>): void => {
Expand All @@ -118,7 +118,7 @@ function TextfieldCell({ error, label, value, onBlur, autoComplete }: TextfieldC
aria-label={label}
autoComplete={autoComplete}
className={classes.textfieldCell}
onBlur={handleBlur}
onChange={handleChange}
onFocus={handleFocus}
ref={ref}
value={(value as string) ?? ''}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type CodeListProps = {
function CodeList({ codeList, onUpdateCodeList }: CodeListProps) {
const editorTexts: CodeListEditorTexts = useOptionListEditorTexts();

const handleUpdateCodeList = (updatedCodeList: StudioComponentsCodeList): void => {
const handleBlurAny = (updatedCodeList: StudioComponentsCodeList): void => {
const updatedCodeListWithMetadata = updateCodeListWithMetadata(codeList, updatedCodeList);
onUpdateCodeList(updatedCodeListWithMetadata);
};
Expand All @@ -36,7 +36,7 @@ function CodeList({ codeList, onUpdateCodeList }: CodeListProps) {
<Accordion.Content>
<StudioCodeListEditor
codeList={codeList.codeList}
onChange={handleUpdateCodeList}
onBlurAny={handleBlurAny}
texts={editorTexts}
/>
</Accordion.Content>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ function EditLibraryOptionListEditorModal({
const optionListHasChanged = (options: Option[]): boolean =>
JSON.stringify(options) !== JSON.stringify(localOptionList);

const handleOptionsChange = (options: Option[]) => {
const handleBlurAny = (options: Option[]) => {
if (optionListHasChanged(options)) {
updateOptionList({ optionListId: optionsId, optionsList: options });
setLocalOptionList(options);
Expand Down Expand Up @@ -126,7 +126,7 @@ function EditLibraryOptionListEditorModal({
>
<StudioCodeListEditor
codeList={localOptionList}
onChange={handleOptionsChange}
onBlurAny={handleBlurAny}
texts={editorTexts}
/>
</StudioModal.Dialog>
Expand All @@ -147,7 +147,7 @@ function EditManualOptionListEditorModal({
const modalRef = useRef<HTMLDialogElement>(null);
const editorTexts = useOptionListEditorTexts();

const handleOptionsChange = (options: Option[]) => {
const handleBlurAny = (options: Option[]) => {
if (component.optionsId) {
delete component.optionsId;
}
Expand Down Expand Up @@ -175,7 +175,7 @@ function EditManualOptionListEditorModal({
>
<StudioCodeListEditor
codeList={component.options ?? []}
onChange={handleOptionsChange}
onBlurAny={handleBlurAny}
texts={editorTexts}
/>
</StudioModal.Dialog>
Expand Down

0 comments on commit 0f7b868

Please sign in to comment.