Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add error text for duplicate label and value #3151

Merged
merged 2 commits into from
Sep 4, 2024
Merged

Add error text for duplicate label and value #3151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0ccb5a5
Add error text for duplicate label and value
emilys314 Sep 3, 2024
f5d9eea
Update error text
emilys314 Sep 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 58 additions & 8 deletions frontend/src/pages/connectionTypes/manage/advanced/DropdownAdvancedPropertiesForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
import * as React from 'react';
import { Button, Flex, FlexItem, FormGroup, Radio, TextInput } from '@patternfly/react-core';
import { MinusCircleIcon, PlusCircleIcon } from '@patternfly/react-icons';
import {
Button,
Flex,
FlexItem,
FormGroup,
FormHelperText,
HelperText,
HelperTextItem,
Radio,
TextInput,
} from '@patternfly/react-core';
import { ExclamationCircleIcon, MinusCircleIcon, PlusCircleIcon } from '@patternfly/react-icons';
import { Table, Tbody, Td, Th, Thead, Tr } from '@patternfly/react-table';
import text from '@patternfly/react-styles/css/utilities/Text/text';
import useDraggableTableControlled from '~/utilities/useDraggableTableControlled';
Expand All @@ -23,8 +33,9 @@ const DropdownAdvancedPropertiesForm: React.FC<AdvancedFieldProps<DropdownField>
// filter out empty rows for validation (they will be removed on save)
const itemsWithoutEmptyRows =
properties.items?.filter((item) => item.label || item.value) ?? [];

// TODO: warn user of duplicate values in table (and labels?)
const duplicateLabels = itemsWithoutEmptyRows.find((item1, index1) =>
properties.items?.find((item2, index2) => item1.label === item2.label && index1 !== index2),
);
const duplicateValues = itemsWithoutEmptyRows.find((item1, index1) =>
properties.items?.find((item2, index2) => item1.value === item2.value && index1 !== index2),
);
Expand All @@ -34,6 +45,7 @@ const DropdownAdvancedPropertiesForm: React.FC<AdvancedFieldProps<DropdownField>
!!properties.variant &&
itemsWithoutEmptyRows.length > 0 &&
noMissingValues &&
!duplicateLabels &&
!duplicateValues,
);
}, [properties.variant, properties.items, onValidate]);
Expand All @@ -43,11 +55,25 @@ const DropdownAdvancedPropertiesForm: React.FC<AdvancedFieldProps<DropdownField>
[properties],
);
const setDropdownItems = React.useCallback(
(newItems: { label: string; value: string }[]) => {
(items: { label: string; value: string; labelError?: string; valueError?: string }[]) => {
// if a default value no longer exists as a possible option, remove it
const newDefaults = properties.defaultValue?.filter((defaultOption) =>
newItems.find((newOption) => defaultOption === newOption.value),
items.find((newOption) => defaultOption === newOption.value),
);
const newItems = items.map((item, index) => {
const duplicateLabel = items.find(
(dup, dupIndex) => item.label && item.label === dup.label && dupIndex < index,
);
const duplicateValue = items.find(
(dup, dupIndex) => item.value && item.value === dup.value && dupIndex < index,
);
return {
label: item.label,
value: item.value,
...(duplicateLabel && { labelError: `${item.label} already exists.` }),
...(duplicateValue && { valueError: `${item.value} already exists.` }),
};
});
onChange({
...properties,
defaultValue: newDefaults,
Expand All @@ -60,6 +86,8 @@ const DropdownAdvancedPropertiesForm: React.FC<AdvancedFieldProps<DropdownField>
const { tableProps, rowsToRender } = useDraggableTableControlled<{
label: string;
value: string;
labelError?: string;
valueError?: string;
}>(dropdownItems, setDropdownItems);

return (
Expand Down Expand Up @@ -106,7 +134,7 @@ const DropdownAdvancedPropertiesForm: React.FC<AdvancedFieldProps<DropdownField>
</FormGroup>
<FormGroup>
<Table data-testid="connection-type-fields-table" className={tableProps.className}>
<Thead>
<Thead noWrap>
<Tr>
<Th screenReaderText="Drag and drop" />
<Th
Expand All @@ -117,6 +145,7 @@ const DropdownAdvancedPropertiesForm: React.FC<AdvancedFieldProps<DropdownField>
headerContent: 'Dropdown item label',
},
}}
width={45}
>
Dropdown item labels
</Th>
Expand All @@ -127,6 +156,7 @@ const DropdownAdvancedPropertiesForm: React.FC<AdvancedFieldProps<DropdownField>
headerContent: 'Dropdown item value',
},
}}
width={45}
>
<div>
Dropdown item values
Expand All @@ -135,7 +165,7 @@ const DropdownAdvancedPropertiesForm: React.FC<AdvancedFieldProps<DropdownField>
</span>
</div>
</Th>
<Th screenReaderText="Actions" width={10} />
<Th screenReaderText="Actions" />
</Tr>
</Thead>
<Tbody {...tableProps.tbodyProps}>
Expand All @@ -161,8 +191,18 @@ const DropdownAdvancedPropertiesForm: React.FC<AdvancedFieldProps<DropdownField>
...dropdownItems.slice(i + 1),
]);
}}
validated={r.data.labelError ? 'error' : undefined}
aria-label="dropdown item label"
/>
{r.data.labelError && (
<FormHelperText>
<HelperText>
<HelperTextItem icon={<ExclamationCircleIcon />} variant="error">
{r.data.labelError}
</HelperTextItem>
</HelperText>
</FormHelperText>
)}
</Td>
<Td dataLabel="Dropdown item values">
<TextInput
Expand All @@ -179,8 +219,18 @@ const DropdownAdvancedPropertiesForm: React.FC<AdvancedFieldProps<DropdownField>
...dropdownItems.slice(i + 1),
]);
}}
validated={r.data.valueError ? 'error' : undefined}
aria-label="dropdown item value"
/>
{r.data.valueError && (
<FormHelperText>
<HelperText>
<HelperTextItem icon={<ExclamationCircleIcon />} variant="error">
{r.data.valueError}
</HelperTextItem>
</HelperText>
</FormHelperText>
)}
</Td>
<Td>
<Button
Expand Down
61 changes: 61 additions & 0 deletions ...c/pages/connectionTypes/manage/advanced/__tests__/DropdownAdvancedPropertiesForm.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,65 @@ describe('DropdownFieldAdvancedPropertiesForm', () => {
items: [{ label: 'b', value: 'b' }],
});
});

it('should show duplicate error message', async () => {
let renderResult = render(
<DropdownAdvancedPropertiesForm
properties={{
variant: 'single',
items: [
{ label: 'a', value: 'b' },
{ label: '', value: '' },
],
}}
field={field}
onChange={onChange}
onValidate={onValidate}
/>,
);

const itemLabel = screen.getByTestId('dropdown-item-row-label-1');
act(() => fireEvent.change(itemLabel, { target: { value: 'a' } }));
expect(onChange).toHaveBeenLastCalledWith({
variant: 'single',
items: [
{ label: 'a', value: 'b' },
{ label: 'a', value: '', labelError: 'a already exists.' },
],
});

const itemValue = screen.getByTestId('dropdown-item-row-value-1');
act(() => fireEvent.change(itemValue, { target: { value: 'b' } }));
expect(onChange).toHaveBeenLastCalledWith({
variant: 'single',
items: [
{ label: 'a', value: 'b' },
{ label: '', value: 'b', valueError: 'b already exists.' },
],
});

renderResult.unmount();
renderResult = render(
<DropdownAdvancedPropertiesForm
properties={{
variant: 'single',
items: [
{ label: 'a', value: 'b' },
{
label: 'a',
value: 'b',
labelError: 'a already exists.',
valueError: 'b already exists.',
},
] as { label: string; value: string }[],
}}
field={field}
onChange={onChange}
onValidate={onValidate}
/>,
);

expect(screen.getByText('a already exists.')).toBeVisible();
expect(screen.getByText('b already exists.')).toBeVisible();
});
});
Loading