Skip to content

Commit d845cd8

Browse files
arsen-afaunovarsen-afaunov
committed
Address review issues
1 parent bd2f64f commit d845cd8

File tree

4 files changed

+141
-77
lines changed

4 files changed

+141
-77
lines changed

‎src/ui/components/NumberFormatSettings/NumberInput/NumberInput.tsx‎

Lines changed: 65 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ interface NumberInputProps {
1717
}
1818

1919
const b = block('wizard-number-input');
20-
const DEFAULT_VALUE = 0;
2120

2221
const NumberInput: React.FC<NumberInputProps> = ({
2322
value,
@@ -29,36 +28,86 @@ const NumberInput: React.FC<NumberInputProps> = ({
2928
inputClassName,
3029
qa,
3130
}) => {
32-
const onBlur = React.useCallback(() => {
33-
if (Number.isNaN(value)) {
34-
onChange(DEFAULT_VALUE);
31+
const [internalValue, setInternalValue] = React.useState<string>(String(value));
32+
33+
React.useEffect(() => {
34+
setInternalValue(String(value));
35+
}, [value]);
36+
37+
const commitValue = React.useCallback(
38+
(newValue: number) => {
39+
setInternalValue(String(newValue));
40+
onChange(newValue);
41+
},
42+
[onChange],
43+
);
44+
45+
const clampAndCommit = React.useCallback(() => {
46+
const parsed = parseInt(internalValue, 10);
47+
if (Number.isNaN(parsed) || parsed < min) {
48+
commitValue(min);
49+
} else if (parsed > max) {
50+
commitValue(max);
51+
} else {
52+
commitValue(parsed);
3553
}
36-
}, [onChange, value]);
54+
}, [internalValue, min, max, commitValue]);
55+
56+
const onBlur = React.useCallback(() => {
57+
clampAndCommit();
58+
}, [clampAndCommit]);
59+
60+
const onKeyDown = React.useCallback(
61+
(e: React.KeyboardEvent<HTMLInputElement>) => {
62+
if (e.key === 'Enter') {
63+
clampAndCommit();
64+
}
65+
},
66+
[clampAndCommit],
67+
);
3768

38-
const onInput = React.useCallback((newValue) => onChange(parseInt(newValue, 10)), [onChange]);
69+
const onInput = React.useCallback((newValue: string) => {
70+
setInternalValue(newValue);
71+
}, []);
3972

4073
const onPlus = React.useCallback(() => {
41-
const newValue = Number.isNaN(value) ? DEFAULT_VALUE : Math.min(value + 1, max);
42-
onChange(newValue);
43-
}, [max, onChange, value]);
74+
const parsed = parseInt(internalValue, 10);
75+
if (Number.isNaN(parsed) || parsed < min) {
76+
commitValue(min);
77+
} else if (parsed >= max) {
78+
commitValue(max);
79+
} else {
80+
commitValue(parsed + 1);
81+
}
82+
}, [internalValue, min, max, commitValue]);
4483

4584
const onMinus = React.useCallback(() => {
46-
const newValue = Number.isNaN(value) ? DEFAULT_VALUE : Math.max(value - 1, min);
47-
onChange(newValue);
48-
}, [min, onChange, value]);
85+
const parsed = parseInt(internalValue, 10);
86+
if (Number.isNaN(parsed) || parsed > max) {
87+
commitValue(max);
88+
} else if (parsed <= min) {
89+
commitValue(min);
90+
} else {
91+
commitValue(parsed - 1);
92+
}
93+
}, [internalValue, min, max, commitValue]);
4994

5095
const memorizedInputAttrs: React.InputHTMLAttributes<HTMLInputElement> = React.useMemo(
5196
() => ({min, max, style: {textAlign: 'center'}}),
5297
[min, max],
5398
);
5499

100+
const parsedInternal = parseInt(internalValue, 10);
101+
const isMinusDisabled = !Number.isNaN(parsedInternal) && parsedInternal <= min;
102+
const isPlusDisabled = !Number.isNaN(parsedInternal) && parsedInternal >= max;
103+
55104
return (
56105
<div className={b({}, className)}>
57106
<Button
58107
className={b('button', buttonClassName)}
59108
view="outlined"
60109
pin="round-brick"
61-
disabled={value === min}
110+
disabled={isMinusDisabled}
62111
onClick={onMinus}
63112
>
64113
-
@@ -68,16 +117,17 @@ const NumberInput: React.FC<NumberInputProps> = ({
68117
type="number"
69118
pin="brick-brick"
70119
controlProps={memorizedInputAttrs}
71-
value={String(value)}
120+
value={internalValue}
72121
onBlur={onBlur}
73122
onUpdate={onInput}
123+
onKeyDown={onKeyDown}
74124
className={b('text-input', inputClassName)}
75125
/>
76126
<Button
77127
className={b('button', buttonClassName)}
78128
view="outlined"
79129
pin="brick-round"
80-
disabled={value === max}
130+
disabled={isPlusDisabled}
81131
onClick={onPlus}
82132
>
83133
+

‎src/ui/units/wizard/components/Dialogs/DialogShapes/DialogShapes.tsx‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {i18n} from 'i18n';
88
import type {DatasetOptions, Field, FilterField, ShapesConfig, Update} from 'shared';
99
import {LINE_WIDTH_DEFAULT_VALUE} from 'ui/units/wizard/constants/shapes';
1010

11-
import type {PaletteTypes} from '../../../constants';
11+
import {PaletteTypes} from '../../../constants';
1212

1313
import {DialogLineWidth} from './DialogLineWidth/DialogLineWidth';
1414
import DialogShapesPalette from './DialogShapesPalette/DialogShapesPalette';
@@ -67,11 +67,11 @@ const DialogShapes: React.FC<Props> = ({
6767
shapesConfig.mountedShapes && Object.keys(shapesConfig.mountedShapes)
6868
? shapesConfig.mountedShapes
6969
: {};
70-
const mountedShapesLineWidths = Object.keys(mountedShapes).reduce((acc, shapeKey) => {
71-
const shapeLineWidth = shapesConfig.mountedShapesLineWidths?.[shapeKey];
72-
73-
return {...acc, [shapeKey]: shapeLineWidth || LINE_WIDTH_DEFAULT_VALUE};
74-
}, {});
70+
const mountedShapesLineWidths =
71+
shapesConfig.mountedShapesLineWidths &&
72+
Object.keys(shapesConfig.mountedShapesLineWidths)
73+
? shapesConfig.mountedShapesLineWidths
74+
: {};
7575

7676
return {
7777
selectedValue: null,
@@ -177,10 +177,12 @@ const DialogShapes: React.FC<Props> = ({
177177
}
178178
/>
179179
<Flex direction="column" gap={4} spacing={{py: '5', px: '6'}}>
180-
<DialogLineWidth
181-
value={selectedShapeLineWidth}
182-
onChange={onLineWidthChange}
183-
/>
180+
{paletteType === PaletteTypes.Lines && (
181+
<DialogLineWidth
182+
value={selectedShapeLineWidth}
183+
onChange={onLineWidthChange}
184+
/>
185+
)}
184186
<DialogShapesPalette
185187
shapesState={shapesState}
186188
onPaletteItemClick={onPaletteItemClick}

‎tests/opensource-suites/wizard/visualizations/line/shapes.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ import {WizardPageQa, WizardVisualizationId} from '../../../../../src/shared';
66
import WizardPage from '../../../../page-objects/wizard/WizardPage';
77
import {PlaceholderName} from '../../../../page-objects/wizard/SectionVisualization';
88
import {ColorValue} from '../../../../page-objects/wizard/ColorDialog';
9+
import {DOMNamedAttributes} from '../../../../page-objects/wizard/ChartKit';
10+
import {
11+
LINE_WIDTH_DEFAULT_VALUE,
12+
LINE_WIDTH_MAX_VALUE,
13+
LINE_WIDTH_MIN_VALUE,
14+
LINE_WIDTH_VALUE_STEP,
15+
} from '../../../../../src/ui/units/wizard/constants/shapes';
916

1017
datalensTest.describe('Wizard', () => {
1118
datalensTest.describe('Line chart', () => {
@@ -70,5 +77,62 @@ datalensTest.describe('Wizard', () => {
7077
await expect(chartContainer).toHaveScreenshot();
7178
},
7279
);
80+
81+
datalensTest('User can change line width', async ({page}) => {
82+
const wizardPage = new WizardPage({page});
83+
84+
await wizardPage.sectionVisualization.addFieldByClick(PlaceholderName.X, 'Category');
85+
await wizardPage.sectionVisualization.addFieldByClick(PlaceholderName.Y, 'Sales');
86+
87+
const defaultLineWidth = LINE_WIDTH_DEFAULT_VALUE.toString();
88+
const newLineWidth = '5';
89+
90+
let lineWidths = await wizardPage.chartkit.getAttributeFromLines(
91+
DOMNamedAttributes.StrokeWidth,
92+
);
93+
94+
expect(lineWidths.length).toEqual(1);
95+
expect(lineWidths[0]).toEqual(defaultLineWidth);
96+
97+
await wizardPage.shapeDialog.open();
98+
99+
await wizardPage.shapeDialog.changeLineWidth(newLineWidth);
100+
101+
await wizardPage.shapeDialog.apply();
102+
103+
await wizardPage.chartkit.waitUntilLoaderExists();
104+
105+
lineWidths = await wizardPage.chartkit.getAttributeFromLines(
106+
DOMNamedAttributes.StrokeWidth,
107+
);
108+
109+
expect(lineWidths.length).toEqual(1);
110+
expect(lineWidths[0]).toEqual(String(newLineWidth));
111+
});
112+
113+
datalensTest('User can only select line widths 1–12', async ({page}) => {
114+
const wizardPage = new WizardPage({page});
115+
116+
await wizardPage.sectionVisualization.addFieldByClick(PlaceholderName.X, 'Category');
117+
await wizardPage.sectionVisualization.addFieldByClick(PlaceholderName.Y, 'Sales');
118+
119+
await wizardPage.shapeDialog.open();
120+
await wizardPage.shapeDialog.clickLineWidthSelectControl();
121+
122+
const optionElements = await wizardPage.shapeDialog.getLineWidthSelectOptions();
123+
124+
expect(await optionElements.count()).toEqual(LINE_WIDTH_MAX_VALUE);
125+
126+
for (
127+
let i = LINE_WIDTH_MIN_VALUE;
128+
i <= LINE_WIDTH_MAX_VALUE;
129+
i += LINE_WIDTH_VALUE_STEP
130+
) {
131+
const optionElement = optionElements.nth(i - 1);
132+
const optionValue = i.toString();
133+
134+
await expect(optionElement.locator(`[data-qa="${optionValue}"]`)).toBeVisible();
135+
}
136+
});
73137
});
74138
});

‎tests/suites/wizard/placeholders/shapesPlaceholder.test.ts‎

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,5 @@
11
import {Page} from '@playwright/test';
22
import {LineShapeType, SectionVisualizationAddItemQa} from '../../../../src/shared/constants';
3-
import {
4-
LINE_WIDTH_DEFAULT_VALUE,
5-
LINE_WIDTH_MAX_VALUE,
6-
LINE_WIDTH_MIN_VALUE,
7-
LINE_WIDTH_VALUE_STEP,
8-
} from '../../../../src/ui/units/wizard/constants/shapes';
93

104
import {WizardVisualizationId} from '../../../page-objects/common/Visualization';
115
import {DOMNamedAttributes} from '../../../page-objects/wizard/ChartKit';
@@ -53,52 +47,6 @@ datalensTest.describe('Wizard - section "Forms"', () => {
5347
expect(lines[0]).toEqual(DashArrayLineType[LineShapeType.Dash]);
5448
});
5549

56-
datalensTest('User can change line width', async ({page}: {page: Page}) => {
57-
const wizardPage = new WizardPage({page});
58-
const defaultLineWidth = LINE_WIDTH_DEFAULT_VALUE.toString();
59-
const newLineWidth = '5';
60-
61-
let lineWidths = await wizardPage.chartkit.getAttributeFromLines(
62-
DOMNamedAttributes.StrokeWidth,
63-
);
64-
65-
expect(lineWidths.length).toEqual(1);
66-
expect(lineWidths[0]).toEqual(defaultLineWidth);
67-
68-
await wizardPage.shapeDialog.open();
69-
70-
await wizardPage.shapeDialog.changeLineWidth(newLineWidth);
71-
72-
await wizardPage.shapeDialog.apply();
73-
74-
await wizardPage.chartkit.waitUntilLoaderExists();
75-
76-
lineWidths = await wizardPage.chartkit.getAttributeFromLines(
77-
DOMNamedAttributes.StrokeWidth,
78-
);
79-
80-
expect(lineWidths.length).toEqual(1);
81-
expect(lineWidths[0]).toEqual(String(newLineWidth));
82-
});
83-
84-
datalensTest('User can only select line widths 1–12', async ({page}: {page: Page}) => {
85-
const wizardPage = new WizardPage({page});
86-
87-
await wizardPage.shapeDialog.open();
88-
await wizardPage.shapeDialog.clickLineWidthSelectControl();
89-
90-
const optionElements = await wizardPage.shapeDialog.getLineWidthSelectOptions();
91-
92-
expect(await optionElements.count()).toEqual(LINE_WIDTH_MAX_VALUE);
93-
94-
for (let i = LINE_WIDTH_MIN_VALUE; i <= LINE_WIDTH_MAX_VALUE; i += LINE_WIDTH_VALUE_STEP) {
95-
const optionElement = optionElements.nth(i - 1);
96-
const optionValue = i.toString();
97-
98-
await expect(optionElement.locator(`[data-qa="${optionValue}"]`)).toBeVisible();
99-
}
100-
});
101-
10250
datalensTest(
10351
'The user can split the indicator by measurement into forms',
10452
async ({page}: {page: Page}) => {

0 commit comments

Comments
 (0)