Skip to content

Commit 18c046a

Browse files
Snowfox0618Snowfox0618
authored
[FE] Fix number of partitions field validation issue (#3400)
* Now user can only input valid digits, and '-' is not allowed in positive-only inputs. * Fix ISSUE#3319 Now user can only input valid digits, and '-' is not allowed in positive-only inputs. * Revert "Fix ISSUE#3319" This reverts commit a4e34f5. * Fix ISSUE#3319 Created a helper function, and added a unit test to cover it. * Fix ISSUE#3319 Located the helper function outside the component, and renamed some unit tests to make their meaning more clear. * Fix ISSUE#3319 - Added an attribute 'integerOnly' to component 'Input', to represent whether this input component instance will accept decimal. - Improved input-check function and paste-check function, to avoid invalid number format (like '3-3', '3.3.3'). - Added new unit tests to test new input-check and paste-check functions. - Added attribute 'integerOnly' to Input instances in the TopicForm component.
1 parent b5e3d1f commit 18c046a

File tree

4 files changed

+269
-25
lines changed

4 files changed

+269
-25
lines changed

‎kafka-ui-react-app/src/components/Topics/shared/Form/TopicForm.tsx‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ const TopicForm: React.FC<Props> = ({
116116
placeholder="Number of partitions"
117117
min="1"
118118
name="partitions"
119+
positiveOnly
120+
integerOnly
119121
/>
120122
<FormError>
121123
<ErrorMessage errors={errors} name="partitions" />
@@ -161,6 +163,8 @@ const TopicForm: React.FC<Props> = ({
161163
placeholder="Min In Sync Replicas"
162164
min="1"
163165
name="minInSyncReplicas"
166+
positiveOnly
167+
integerOnly
164168
/>
165169
<FormError>
166170
<ErrorMessage errors={errors} name="minInSyncReplicas" />
@@ -177,6 +181,8 @@ const TopicForm: React.FC<Props> = ({
177181
placeholder="Replication Factor"
178182
min="1"
179183
name="replicationFactor"
184+
positiveOnly
185+
integerOnly
180186
/>
181187
<FormError>
182188
<ErrorMessage errors={errors} name="replicationFactor" />
@@ -227,6 +233,8 @@ const TopicForm: React.FC<Props> = ({
227233
placeholder="Maximum message size"
228234
min="1"
229235
name="maxMessageBytes"
236+
positiveOnly
237+
integerOnly
230238
/>
231239
<FormError>
232240
<ErrorMessage errors={errors} name="maxMessageBytes" />

‎kafka-ui-react-app/src/components/common/Input/Input.tsx‎

Lines changed: 102 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,87 @@ export interface InputProps
1111
hookFormOptions?: RegisterOptions;
1212
search?: boolean;
1313
positiveOnly?: boolean;
14+
15+
// Some may only accept integer, like `Number of Partitions`
16+
// some may accept decimal
17+
integerOnly?: boolean;
18+
}
19+
20+
function inputNumberCheck(
21+
key: string,
22+
positiveOnly: boolean,
23+
integerOnly: boolean,
24+
getValues: (name: string) => string,
25+
componentName: string
26+
) {
27+
let isValid = true;
28+
if (!((key >= '0' && key <= '9') || key === '-' || key === '.')) {
29+
// If not a valid digit char.
30+
isValid = false;
31+
} else {
32+
// If there is any restriction.
33+
if (positiveOnly) {
34+
isValid = !(key === '-');
35+
}
36+
if (isValid && integerOnly) {
37+
isValid = !(key === '.');
38+
}
39+
40+
// Check invalid format
41+
const value = getValues(componentName);
42+
43+
if (isValid && (key === '-' || key === '.')) {
44+
if (!positiveOnly) {
45+
if (key === '-') {
46+
if (value !== '') {
47+
// '-' should not appear anywhere except the start of the string
48+
isValid = false;
49+
}
50+
}
51+
}
52+
if (!integerOnly) {
53+
if (key === '.') {
54+
if (value === '' || value.indexOf('.') !== -1) {
55+
// '.' should not appear at the start of the string or appear twice
56+
isValid = false;
57+
}
58+
}
59+
}
60+
}
61+
}
62+
return isValid;
63+
}
64+
65+
function pasteNumberCheck(
66+
text: string,
67+
positiveOnly: boolean,
68+
integerOnly: boolean
69+
) {
70+
let value: string;
71+
value = text;
72+
let sign = '';
73+
if (!positiveOnly) {
74+
if (value.charAt(0) === '-') {
75+
sign = '-';
76+
}
77+
}
78+
if (integerOnly) {
79+
value = value.replace(/\D/g, '');
80+
} else {
81+
value = value.replace(/[^\d.]/g, '');
82+
if (value.indexOf('.') !== value.lastIndexOf('.')) {
83+
const strs = value.split('.');
84+
value = '';
85+
for (let i = 0; i < strs.length; i += 1) {
86+
value += strs[i];
87+
if (i === 0) {
88+
value += '.';
89+
}
90+
}
91+
}
92+
}
93+
value = sign + value;
94+
return value;
1495
}
1596

1697
const Input: React.FC<InputProps> = ({
@@ -20,35 +101,42 @@ const Input: React.FC<InputProps> = ({
20101
inputSize = 'L',
21102
type,
22103
positiveOnly,
104+
integerOnly,
23105
...rest
24106
}) => {
25107
const methods = useFormContext();
108+
26109
const keyPressEventHandler = (
27110
event: React.KeyboardEvent<HTMLInputElement>
28111
) => {
29-
const { key, code } = event;
112+
const { key } = event;
30113
if (type === 'number') {
31-
// Manualy prevent input of 'e' character for all number inputs
114+
// Manually prevent input of non-digit and non-minus for all number inputs
32115
// and prevent input of negative numbers for positiveOnly inputs
33-
if (key === 'e' || (positiveOnly && (key === '-' || code === 'Minus'))) {
116+
if (
117+
!inputNumberCheck(
118+
key,
119+
typeof positiveOnly === 'boolean' ? positiveOnly : false,
120+
typeof integerOnly === 'boolean' ? integerOnly : false,
121+
methods.getValues,
122+
typeof name === 'string' ? name : ''
123+
)
124+
) {
34125
event.preventDefault();
35126
}
36127
}
37128
};
38129
const pasteEventHandler = (event: React.ClipboardEvent<HTMLInputElement>) => {
39130
if (type === 'number') {
40131
const { clipboardData } = event;
41-
const text = clipboardData.getData('Text');
42-
// replace all non-digit characters with empty string
43-
let value = text.replace(/[^\d.]/g, '');
44-
if (positiveOnly) {
45-
// check if value is negative
46-
const parsedData = parseFloat(value);
47-
if (parsedData < 0) {
48-
// remove minus sign
49-
value = String(Math.abs(parsedData));
50-
}
51-
}
132+
// The 'clipboardData' does not have key 'Text', but has key 'text' instead.
133+
const text = clipboardData.getData('text');
134+
// Check the format of pasted text.
135+
const value = pasteNumberCheck(
136+
text,
137+
typeof positiveOnly === 'boolean' ? positiveOnly : false,
138+
typeof integerOnly === 'boolean' ? integerOnly : false
139+
);
52140
// if paste value contains non-numeric characters or
53141
// negative for positiveOnly fields then prevent paste
54142
if (value !== text) {

‎kafka-ui-react-app/src/components/common/Input/__tests__/Input.spec.tsx‎

Lines changed: 148 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,23 @@ import { screen } from '@testing-library/react';
44
import { render } from 'lib/testHelpers';
55
import userEvent from '@testing-library/user-event';
66

7+
// Mock useFormContext
8+
let component: HTMLInputElement;
9+
710
const setupWrapper = (props?: Partial<InputProps>) => (
811
<Input name="test" {...props} />
912
);
1013
jest.mock('react-hook-form', () => ({
1114
useFormContext: () => ({
1215
register: jest.fn(),
16+
17+
// Mock methods.getValues and methods.setValue
18+
getValues: jest.fn(() => {
19+
return component.value;
20+
}),
21+
setValue: jest.fn((key, val) => {
22+
component.value = val;
23+
}),
1324
}),
1425
}));
1526

@@ -23,20 +34,146 @@ describe('Custom Input', () => {
2334
});
2435
});
2536
describe('number', () => {
26-
const getInput = () => screen.getByRole('spinbutton');
37+
const getInput = () => screen.getByRole<HTMLInputElement>('spinbutton');
38+
39+
describe('input', () => {
40+
it('allows user to type numbers only', async () => {
41+
render(setupWrapper({ type: 'number' }));
42+
const input = getInput();
43+
component = input;
44+
await userEvent.type(input, 'abc131');
45+
expect(input).toHaveValue(131);
46+
});
47+
48+
it('allows user to type negative values', async () => {
49+
render(setupWrapper({ type: 'number' }));
50+
const input = getInput();
51+
component = input;
52+
await userEvent.type(input, '-2');
53+
expect(input).toHaveValue(-2);
54+
});
55+
56+
it('allows user to type positive values only', async () => {
57+
render(setupWrapper({ type: 'number', positiveOnly: true }));
58+
const input = getInput();
59+
component = input;
60+
await userEvent.type(input, '-2');
61+
expect(input).toHaveValue(2);
62+
});
63+
64+
it('allows user to type decimal', async () => {
65+
render(setupWrapper({ type: 'number' }));
66+
const input = getInput();
67+
component = input;
68+
await userEvent.type(input, '2.3');
69+
expect(input).toHaveValue(2.3);
70+
});
71+
72+
it('allows user to type integer only', async () => {
73+
render(setupWrapper({ type: 'number', integerOnly: true }));
74+
const input = getInput();
75+
component = input;
76+
await userEvent.type(input, '2.3');
77+
expect(input).toHaveValue(23);
78+
});
79+
80+
it("not allow '-' appear at any position of the string except the start", async () => {
81+
render(setupWrapper({ type: 'number' }));
82+
const input = getInput();
83+
component = input;
84+
await userEvent.type(input, '2-3');
85+
expect(input).toHaveValue(23);
86+
});
2787

28-
it('allows user to type only numbers', async () => {
29-
render(setupWrapper({ type: 'number' }));
30-
const input = getInput();
31-
await userEvent.type(input, 'abc131');
32-
expect(input).toHaveValue(131);
88+
it("not allow '.' appear at the start of the string", async () => {
89+
render(setupWrapper({ type: 'number' }));
90+
const input = getInput();
91+
component = input;
92+
await userEvent.type(input, '.33');
93+
expect(input).toHaveValue(33);
94+
});
95+
96+
it("not allow '.' appear twice in the string", async () => {
97+
render(setupWrapper({ type: 'number' }));
98+
const input = getInput();
99+
component = input;
100+
await userEvent.type(input, '3.3.3');
101+
expect(input).toHaveValue(3.33);
102+
});
33103
});
34104

35-
it('allows negative values', async () => {
36-
render(setupWrapper({ type: 'number' }));
37-
const input = getInput();
38-
await userEvent.type(input, '-2');
39-
expect(input).toHaveValue(-2);
105+
describe('paste', () => {
106+
it('allows user to paste numbers only', async () => {
107+
render(setupWrapper({ type: 'number' }));
108+
const input = getInput();
109+
component = input;
110+
await userEvent.click(input);
111+
await userEvent.paste('abc131');
112+
expect(input).toHaveValue(131);
113+
});
114+
115+
it('allows user to paste negative values', async () => {
116+
render(setupWrapper({ type: 'number' }));
117+
const input = getInput();
118+
component = input;
119+
await userEvent.click(input);
120+
await userEvent.paste('-2');
121+
expect(input).toHaveValue(-2);
122+
});
123+
124+
it('allows user to paste positive values only', async () => {
125+
render(setupWrapper({ type: 'number', positiveOnly: true }));
126+
const input = getInput();
127+
component = input;
128+
await userEvent.click(input);
129+
await userEvent.paste('-2');
130+
expect(input).toHaveValue(2);
131+
});
132+
133+
it('allows user to paste decimal', async () => {
134+
render(setupWrapper({ type: 'number' }));
135+
const input = getInput();
136+
component = input;
137+
await userEvent.click(input);
138+
await userEvent.paste('2.3');
139+
expect(input).toHaveValue(2.3);
140+
});
141+
142+
it('allows user to paste integer only', async () => {
143+
render(setupWrapper({ type: 'number', integerOnly: true }));
144+
const input = getInput();
145+
component = input;
146+
await userEvent.click(input);
147+
await userEvent.paste('2.3');
148+
expect(input).toHaveValue(23);
149+
});
150+
151+
it("not allow '-' appear at any position of the pasted string except the start", async () => {
152+
render(setupWrapper({ type: 'number' }));
153+
const input = getInput();
154+
component = input;
155+
await userEvent.click(input);
156+
await userEvent.paste('2-3');
157+
expect(input).toHaveValue(23);
158+
});
159+
160+
it("not allow '.' appear at the start of the pasted string", async () => {
161+
render(setupWrapper({ type: 'number' }));
162+
const input = getInput();
163+
component = input;
164+
await userEvent.click(input);
165+
await userEvent.paste('.33');
166+
expect(input).toHaveValue(0.33);
167+
});
168+
169+
it("not allow '.' appear twice in the pasted string", async () => {
170+
render(setupWrapper({ type: 'number' }));
171+
const input = getInput();
172+
component = input;
173+
await userEvent.click(input);
174+
await userEvent.paste('3.3.3');
175+
expect(input).toHaveValue(3.33);
176+
});
40177
});
41178
});
42179
});

‎kafka-ui-react-app/src/components/common/NewTable/__test__/Table.spec.tsx‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,17 @@ jest.mock('react-router-dom', () => ({
2020
useNavigate: () => mockedUsedNavigate,
2121
}));
2222

23+
// This is needed by ESLint.
24+
jest.mock('react-hook-form', () => ({
25+
useFormContext: () => ({
26+
register: jest.fn(),
27+
28+
// Mock methods.getValues and methods.setValue
29+
getValues: jest.fn(),
30+
setValue: jest.fn(),
31+
}),
32+
}));
33+
2334
type Datum = typeof data[0];
2435

2536
const data = [

0 commit comments

Comments
 (0)