Browse Source

Fix topic custom parameter duplication overriding (#1859)

* fix topic custom parameter multiple items of the same field bug

* move condition into the updateSelectedOption function
Arsen Simonyan 3 năm trước cách đây
mục cha
commit
85f095657c

+ 9 - 7
kafka-ui-react-app/src/components/Topics/shared/Form/CustomParams/CustomParamField.tsx

@@ -39,6 +39,14 @@ const CustomParamField: React.FC<Props> = ({
   const nameValue = watch(`customParams.${index}.name`);
   const prevName = useRef(nameValue);
 
+  const options = Object.keys(TOPIC_CUSTOM_PARAMS)
+    .sort()
+    .map((option) => ({
+      value: option,
+      label: option,
+      disabled: existingFields.includes(option),
+    }));
+
   React.useEffect(() => {
     if (nameValue !== prevName.current) {
       let newExistingFields = [...existingFields];
@@ -72,13 +80,7 @@ const CustomParamField: React.FC<Props> = ({
               minWidth="270px"
               onChange={onChange}
               value={value}
-              options={Object.keys(TOPIC_CUSTOM_PARAMS)
-                .sort()
-                .map((opt) => ({
-                  value: opt,
-                  label: opt,
-                  disabled: existingFields.includes(opt),
-                }))}
+              options={options}
             />
           )}
         />

+ 4 - 1
kafka-ui-react-app/src/components/common/Select/Select.styled.ts

@@ -94,9 +94,12 @@ export const Option = styled.li<OptionProps>`
   transition: all 0.2s ease-in-out;
   cursor: ${({ disabled }) => (disabled ? 'not-allowed' : 'pointer')};
   gap: 5px;
+  color: ${(props) =>
+    props.theme.select.color[props.disabled ? 'disabled' : 'normal']};
 
   &:hover {
-    background-color: ${(props) => props.theme.select.backgroundColor.hover};
+    background-color: ${(props) =>
+      props.theme.select.backgroundColor[props.disabled ? 'normal' : 'hover']};
   }
 
   &:active {

+ 9 - 4
kafka-ui-react-app/src/components/common/Select/Select.tsx

@@ -48,12 +48,17 @@ const Select: React.FC<SelectProps> = ({
   useClickOutside(selectContainerRef, clickOutsideHandler);
 
   const updateSelectedOption = (option: SelectOption) => {
-    if (disabled) return;
+    if (!option.disabled) {
+      setSelectedOption(option.value);
 
-    setSelectedOption(option.value);
-    if (onChange) onChange(option.value);
-    setShowOptions(false);
+      if (onChange) {
+        onChange(option.value);
+      }
+
+      setShowOptions(false);
+    }
   };
+
   React.useEffect(() => {
     setSelectedOption(value);
   }, [isLive, value]);

+ 44 - 21
kafka-ui-react-app/src/components/common/Select/__tests__/Select.spec.tsx

@@ -1,4 +1,7 @@
-import Select, { SelectProps } from 'components/common/Select/Select';
+import Select, {
+  SelectOption,
+  SelectProps,
+} from 'components/common/Select/Select';
 import React from 'react';
 import { render } from 'lib/testHelpers';
 import { screen } from '@testing-library/react';
@@ -10,34 +13,54 @@ jest.mock('react-hook-form', () => ({
   }),
 }));
 
-const options = [
-  { label: 'test-label1', value: 'test-value1' },
-  { label: 'test-label2', value: 'test-value2' },
+const options: Array<SelectOption> = [
+  { label: 'test-label1', value: 'test-value1', disabled: false },
+  { label: 'test-label2', value: 'test-value2', disabled: true },
 ];
 
 const renderComponent = (props?: Partial<SelectProps>) =>
   render(<Select name="test" {...props} />);
 
 describe('Custom Select', () => {
-  it('renders component', () => {
-    renderComponent();
-    expect(screen.getByRole('listbox')).toBeInTheDocument();
-  });
-  it('show select options when select is being clicked', () => {
-    renderComponent({
-      options,
+  describe('when isLive is not specified', () => {
+    beforeEach(() => {
+      renderComponent({
+        options,
+      });
     });
-    expect(screen.getByRole('option')).toBeInTheDocument();
-    userEvent.click(screen.getByRole('listbox'));
-    expect(screen.getAllByRole('option')).toHaveLength(3);
-  });
-  it('checking select option change', () => {
-    renderComponent({
-      options,
+
+    it('renders component', () => {
+      expect(screen.getByRole('listbox')).toBeInTheDocument();
+    });
+
+    it('show select options when select is being clicked', () => {
+      expect(screen.getByRole('option')).toBeInTheDocument();
+      userEvent.click(screen.getByRole('listbox'));
+      expect(screen.getAllByRole('option')).toHaveLength(3);
+    });
+
+    it('checking select option change', () => {
+      const listbox = screen.getByRole('listbox');
+      const optionLabel = 'test-label1';
+
+      userEvent.click(listbox);
+      userEvent.selectOptions(listbox, [optionLabel]);
+
+      expect(screen.getByRole('option')).toHaveTextContent(optionLabel);
+    });
+
+    it('trying to select disabled option does not trigger change', () => {
+      const listbox = screen.getByRole('listbox');
+      const normalOptionLabel = 'test-label1';
+      const disabledOptionLabel = 'test-label2';
+
+      userEvent.click(listbox);
+      userEvent.selectOptions(listbox, [normalOptionLabel]);
+      userEvent.click(listbox);
+      userEvent.selectOptions(listbox, [disabledOptionLabel]);
+
+      expect(screen.getByRole('option')).toHaveTextContent(normalOptionLabel);
     });
-    userEvent.click(screen.getByRole('listbox'));
-    userEvent.selectOptions(screen.getByRole('listbox'), ['test-label1']);
-    expect(screen.getByRole('option')).toHaveTextContent('test-label1');
   });
 
   describe('when non-live', () => {