Feature: TimePicker's minutes/seconds list multiplicity#2836
Open
Realetive wants to merge 7 commits into
Open
Conversation
…rid and TimePicker
… and roundToMultiplicity
Realetive
requested review from
TitanKuzmich,
Yakutoc,
Yeti-or,
neretin-trike and
shuga2704
as code owners
📝 WalkthroughWalkthroughThis PR adds step-based multiplicity configuration to TimePicker and TimePickerGrid, allowing minute and second selection increments to be controlled via ChangesTimePicker multiplicity feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
|
Theme Builder app deployed! https://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-2836/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/plasma-new-hope/src/components/TimePickerGrid/TimePickerGrid.tsx`:
- Around line 61-64: The memo for columnsConfig is missing multiplicityMinutes
and multiplicitySeconds so getColumnsFromFormat(actualFormat,
multiplicityMinutes, multiplicitySeconds) can return stale values; update the
useMemo dependency array for the columnsConfig declaration to include
multiplicityMinutes and multiplicitySeconds (in addition to actualFormat) so
columnsConfig is recalculated when those props change.
In `@packages/plasma-new-hope/src/components/TimePickerGrid/utils/index.ts`:
- Around line 140-147: In range and roundToMultiplicity guard against
invalid/non-positive step and ensure rounded values stay within the selectable
set: in range(...) ensure step is normalized to at least 1 (e.g., if step <= 0
set step = 1) and compute length with Math.max(1, Math.ceil(number / step)) so
Array.from never gets an invalid length; in roundToMultiplicity(value, step)
return value unchanged if !step or step <= 1, otherwise compute rounded =
Math.round(value / step) * step and if rounded >= number clamp it down to number
- step (and ensure it never becomes negative) so you never return a value
outside the list produced by range.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d604be46-6498-47d1-8044-c159c6096aa4
📒 Files selected for processing (8)
packages/plasma-new-hope/src/components/TimePicker/TimePicker.tsxpackages/plasma-new-hope/src/components/TimePicker/TimePicker.types.tspackages/plasma-new-hope/src/components/TimePickerGrid/TimePickerGrid.component-test.tsxpackages/plasma-new-hope/src/components/TimePickerGrid/TimePickerGrid.tsxpackages/plasma-new-hope/src/components/TimePickerGrid/TimePickerGrid.types.tspackages/plasma-new-hope/src/components/TimePickerGrid/utils/index.tspackages/plasma-new-hope/src/examples/components/TimePicker/TimePicker.stories.tsxutils/api-tests/src/components/TimePicker/TimePicker.api.test.tsx
Comment on lines
+61
to
+64
| const columnsConfig = useMemo( | ||
| () => getColumnsFromFormat(actualFormat, multiplicityMinutes, multiplicitySeconds), | ||
| [actualFormat], | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/plasma-new-hope/src/components/TimePickerGrid/TimePickerGrid.tsx"
echo "== File head/context =="
sed -n '1,200p' "$FILE" | nl -ba
echo
echo "== Lines 40-90 (around useMemo) =="
sed -n '40,90p' "$FILE" | nl -ba
echo
echo "== Find definition/usages of multiplicityMinutes/multiplicitySeconds in this file =="
rg -n "multiplicity(Minutes|Seconds)" "$FILE" || true
echo
echo "== Find component props/type where these come from =="
rg -n "TimePickerGrid" "$FILE" || true
rg -n "interface|type|props|Props" "$FILE" || true
echo
echo "== Search repo for getColumnsFromFormat signature =="
rg -n "function getColumnsFromFormat|getColumnsFromFormat\\(" packages/plasma-new-hope/src/components/TimePickerGrid -S || true
# If not found in that directory, search broader within packages/plasma-new-hope/src
rg -n "getColumnsFromFormat\\(" packages/plasma-new-hope/src -S || trueRepository: salute-developers/plasma
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/plasma-new-hope/src/components/TimePickerGrid/TimePickerGrid.tsx"
python3 - <<'PY'
from pathlib import Path
p = Path("packages/plasma-new-hope/src/components/TimePickerGrid/TimePickerGrid.tsx")
lines = p.read_text(encoding="utf-8").splitlines()
for i in range(1, min(len(lines), 140)+1):
if 35 <= i <= 90 or i in (1,2,3) or i in (60,61,62,63,64,65,66):
pass
PY
echo "== Lines 50-80 (with line numbers via python) =="
python3 - <<'PY'
from pathlib import Path
p = Path("packages/plasma-new-hope/src/components/TimePickerGrid/TimePickerGrid.tsx")
lines = p.read_text(encoding="utf-8").splitlines()
start, end = 45, 85
for i in range(start, end+1):
if i <= len(lines):
print(f"{i:4d}: {lines[i-1]}")
PY
echo
echo "== Lines containing useMemo =="
rg -n "useMemo\\(" "$FILE" || true
echo
echo "== Find definitions/usages of multiplicityMinutes/multiplicitySeconds in this file =="
rg -n "multiplicity(Minutes|Seconds)" "$FILE" || true
echo
echo "== Search for getColumnsFromFormat definition/signature (repo-local) =="
rg -n "getColumnsFromFormat" packages/plasma-new-hope/src/components/TimePickerGrid -S || true
rg -n "getColumnsFromFormat" packages/plasma-new-hope/src -S || trueRepository: salute-developers/plasma
Length of output: 3634
🏁 Script executed:
#!/bin/bash
set -euo pipefail
UTILS="packages/plasma-new-hope/src/components/TimePickerGrid/utils/index.ts"
echo "== Lines 1-120 of utils/index.ts =="
python3 - <<'PY'
from pathlib import Path
p=Path("packages/plasma-new-hope/src/components/TimePickerGrid/utils/index.ts")
lines=p.read_text(encoding="utf-8").splitlines()
for i in range(1, min(len(lines), 130)+1):
if i<=120:
print(f"{i:4d}: {lines[i-1]}")
PY
echo
echo "== Search within TimePickerGrid for roundToMultiplicity and columnsConfig usage =="
FILE="packages/plasma-new-hope/src/components/TimePickerGrid/TimePickerGrid.tsx"
rg -n "roundToMultiplicity|getColumnsFromFormat|columnsConfig" "$FILE" -n || trueRepository: salute-developers/plasma
Length of output: 5081
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/plasma-new-hope/src/components/TimePickerGrid/TimePickerGrid.tsx"
echo "== Lines 1-120 of TimePickerGrid.tsx =="
python3 - <<'PY'
from pathlib import Path
p=Path("packages/plasma-new-hope/src/components/TimePickerGrid/TimePickerGrid.tsx")
lines=p.read_text(encoding="utf-8").splitlines()
for i in range(1, min(len(lines), 130)+1):
print(f"{i:4d}: {lines[i-1]}")
PYRepository: salute-developers/plasma
Length of output: 5824
Fix stale columnsConfig memoization by adding multiplicity props to useMemo dependencies
getColumnsFromFormat(actualFormat, multiplicityMinutes, multiplicitySeconds) uses multiplicityMinutes/multiplicitySeconds to build the minutes/seconds ranges, but useMemo only depends on actualFormat, so columnsConfig can be stale when those props change.
Proposed fix
const columnsConfig = useMemo(
() => getColumnsFromFormat(actualFormat, multiplicityMinutes, multiplicitySeconds),
- [actualFormat],
+ [actualFormat, multiplicityMinutes, multiplicitySeconds],
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const columnsConfig = useMemo( | |
| () => getColumnsFromFormat(actualFormat, multiplicityMinutes, multiplicitySeconds), | |
| [actualFormat], | |
| ); | |
| const columnsConfig = useMemo( | |
| () => getColumnsFromFormat(actualFormat, multiplicityMinutes, multiplicitySeconds), | |
| [actualFormat, multiplicityMinutes, multiplicitySeconds], | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/plasma-new-hope/src/components/TimePickerGrid/TimePickerGrid.tsx`
around lines 61 - 64, The memo for columnsConfig is missing multiplicityMinutes
and multiplicitySeconds so getColumnsFromFormat(actualFormat,
multiplicityMinutes, multiplicitySeconds) can return stale values; update the
useMemo dependency array for the columnsConfig declaration to include
multiplicityMinutes and multiplicitySeconds (in addition to actualFormat) so
columnsConfig is recalculated when those props change.
Comment on lines
+140
to
+147
| export const range = (number: number, padLength = 2, step = 1): string[] => { | ||
| if (step > number) step = number; | ||
| return Array.from({ length: Math.ceil(number / step) }, (_, i) => (i * step).toString().padStart(padLength, '0')); | ||
| }; | ||
|
|
||
| export const roundToMultiplicity = (value: number, step?: number): number => { | ||
| if (!step || step <= 1) return value; | ||
| return Math.round(value / step) * step; |
There was a problem hiding this comment.
Guard invalid step values and clamp rounded output to selectable values.
Line 142 can throw on step <= 0 (Array.from with invalid length), and Line 147 can produce 60 for minutes/seconds (e.g. 59 with step 20), which is not present in the rendered list.
Proposed fix
+const ALLOWED_MULTIPLICITIES = [1, 5, 10, 15, 20, 30] as const;
+
+const normalizeStep = (step?: number): number => {
+ if (!Number.isFinite(step) || !step || step <= 0) return 1;
+ if (ALLOWED_MULTIPLICITIES.includes(step as (typeof ALLOWED_MULTIPLICITIES)[number])) return step;
+
+ return ALLOWED_MULTIPLICITIES.reduce((nearest, current) =>
+ Math.abs(current - step) < Math.abs(nearest - step) ? current : nearest,
+ );
+};
+
export const range = (number: number, padLength = 2, step = 1): string[] => {
- if (step > number) step = number;
- return Array.from({ length: Math.ceil(number / step) }, (_, i) => (i * step).toString().padStart(padLength, '0'));
+ const safeStep = Math.min(normalizeStep(step), number);
+ return Array.from(
+ { length: Math.ceil(number / safeStep) },
+ (_, i) => (i * safeStep).toString().padStart(padLength, '0'),
+ );
};
export const roundToMultiplicity = (value: number, step?: number): number => {
- if (!step || step <= 1) return value;
- return Math.round(value / step) * step;
+ const safeStep = normalizeStep(step);
+ if (safeStep <= 1) return value;
+
+ const maxValid = Math.floor(59 / safeStep) * safeStep;
+ return Math.max(0, Math.min(maxValid, Math.round(value / safeStep) * safeStep));
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const range = (number: number, padLength = 2, step = 1): string[] => { | |
| if (step > number) step = number; | |
| return Array.from({ length: Math.ceil(number / step) }, (_, i) => (i * step).toString().padStart(padLength, '0')); | |
| }; | |
| export const roundToMultiplicity = (value: number, step?: number): number => { | |
| if (!step || step <= 1) return value; | |
| return Math.round(value / step) * step; | |
| const ALLOWED_MULTIPLICITIES = [1, 5, 10, 15, 20, 30] as const; | |
| const normalizeStep = (step?: number): number => { | |
| if (!Number.isFinite(step) || !step || step <= 0) return 1; | |
| if (ALLOWED_MULTIPLICITIES.includes(step as (typeof ALLOWED_MULTIPLICITIES)[number])) return step; | |
| return ALLOWED_MULTIPLICITIES.reduce((nearest, current) => | |
| Math.abs(current - step) < Math.abs(nearest - step) ? current : nearest, | |
| ); | |
| }; | |
| export const range = (number: number, padLength = 2, step = 1): string[] => { | |
| const safeStep = Math.min(normalizeStep(step), number); | |
| return Array.from( | |
| { length: Math.ceil(number / safeStep) }, | |
| (_, i) => (i * safeStep).toString().padStart(padLength, '0'), | |
| ); | |
| }; | |
| export const roundToMultiplicity = (value: number, step?: number): number => { | |
| const safeStep = normalizeStep(step); | |
| if (safeStep <= 1) return value; | |
| const maxValid = Math.floor(59 / safeStep) * safeStep; | |
| return Math.max(0, Math.min(maxValid, Math.round(value / safeStep) * safeStep)); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/plasma-new-hope/src/components/TimePickerGrid/utils/index.ts` around
lines 140 - 147, In range and roundToMultiplicity guard against
invalid/non-positive step and ensure rounded values stay within the selectable
set: in range(...) ensure step is normalized to at least 1 (e.g., if step <= 0
set step = 1) and compute length with Math.max(1, Math.ceil(number / step)) so
Array.from never gets an invalid length; in roundToMultiplicity(value, step)
return value unchanged if !step or step <= 1, otherwise compute rounded =
Math.round(value / step) * step and if rounded >= number clamp it down to number
- step (and ensure it never becomes negative) so you never return a value
outside the list produced by range.
Contributor
TitanKuzmich
reviewed
Jun 5, 2026
Contributor
There was a problem hiding this comment.
Не вижу тут валидации при вводе, если задана кратность. То есть multiplicityMinutes=5, а в инпут введут 13:13. Скорее всего нужно поведение, как при вводе некорректного значения для NumberInput https://plasma.sberdevices.ru/pr/pr-2836/new-hope-storybook/?path=/story/data-entry-numberinput--default&args=isManualInput:!true
TitanKuzmich
reviewed
Jun 5, 2026
Comment on lines
+24
to
+37
| export const WithMultiplicityMinutes: StoryObj<TimePickerProps> = { | ||
| args: { | ||
| value: '00:00', | ||
| multiplicityMinutes: 5, | ||
| }, | ||
| }; | ||
|
|
||
| export const WithMultiplicitySeconds: StoryObj<TimePickerProps> = { | ||
| args: { | ||
| value: '00:00:00', | ||
| columnsQuantity: 3, | ||
| multiplicitySeconds: 20, | ||
| }, | ||
| }; |
Contributor
There was a problem hiding this comment.
Эти стори дотюнить нужно, так как сейчас не работают
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PLASMA-WEB
TimePicker
TimePickerGrid
What/why changed
Реализована возможность управления отображаемым списком минут и секунд в TimePicker'е, кратным 1 (поведение по умолчанию, т. е. все 60 минут/секунд), 5, 10, 15, 20, 30. Добавлена валидация вводимого значения с округлением к ближайшему валидному числу. Добавлен фолбэк при попытке передать «кратность» вне разрешённого списка чисел (1, 5, 10, 15, 20, 30). На новый функционал добавлены тесты и обновлён сторибук.
Summary by CodeRabbit
multiplicityMinutesandmultiplicitySecondsproperties to control minute and second increments in time selection (1, 5, 10, 15, 20, or 30 increments).📦 Published PR as canary version:
Canary Versions✨ Test out this PR locally via:
npm install @salutejs/plasma-asdk@0.378.0-canary.2836.26952968753.0 npm install @salutejs/plasma-b2c@1.620.0-canary.2836.26952968753.0 npm install @salutejs/plasma-core@1.228.0-canary.2836.26952968753.0 npm install @salutejs/plasma-giga@0.347.0-canary.2836.26952968753.0 npm install @salutejs/plasma-homeds@0.347.0-canary.2836.26952968753.0 npm install @salutejs/plasma-hope@1.374.0-canary.2836.26952968753.0 npm install @salutejs/plasma-icons@1.239.0-canary.2836.26952968753.0 npm install @salutejs/plasma-new-hope@0.364.0-canary.2836.26952968753.0 npm install @salutejs/plasma-tokens-b2b@1.56.0-canary.2836.26952968753.0 npm install @salutejs/plasma-tokens-b2c@0.67.0-canary.2836.26952968753.0 npm install @salutejs/plasma-tokens-web@1.71.0-canary.2836.26952968753.0 npm install @salutejs/plasma-tokens@1.140.0-canary.2836.26952968753.0 npm install @salutejs/plasma-typo@0.44.0-canary.2836.26952968753.0 npm install @salutejs/plasma-ui@1.350.0-canary.2836.26952968753.0 npm install @salutejs/plasma-web@1.622.0-canary.2836.26952968753.0 npm install @salutejs/sdds-bizcom@0.352.0-canary.2836.26952968753.0 npm install @salutejs/sdds-cs@0.356.0-canary.2836.26952968753.0 npm install @salutejs/sdds-dfa@0.350.0-canary.2836.26952968753.0 npm install @salutejs/sdds-finai@0.343.0-canary.2836.26952968753.0 npm install @salutejs/sdds-insol@0.347.0-canary.2836.26952968753.0 npm install @salutejs/sdds-netology@0.351.0-canary.2836.26952968753.0 npm install @salutejs/sdds-os@0.22.0-canary.2836.26952968753.0 npm install @salutejs/sdds-platform-ai@0.351.0-canary.2836.26952968753.0 npm install @salutejs/sdds-sbcom@0.352.0-canary.2836.26952968753.0 npm install @salutejs/sdds-scan@0.350.0-canary.2836.26952968753.0 npm install @salutejs/sdds-serv@0.351.0-canary.2836.26952968753.0 npm install @salutejs/core-themes@0.31.0-canary.2836.26952968753.0 npm install @salutejs/plasma-themes@0.52.0-canary.2836.26952968753.0 npm install @salutejs/sdds-themes@0.67.0-canary.2836.26952968753.0 npm install @salutejs/sdds-api-tests@0.9.0-canary.2836.26952968753.0 npm install @salutejs/plasma-cy-utils@0.158.0-canary.2836.26952968753.0 npm install @salutejs/plasma-sb-utils@0.228.0-canary.2836.26952968753.0 npm install @salutejs/plasma-tokens-utils@0.52.0-canary.2836.26952968753.0 # or yarn add @salutejs/plasma-asdk@0.378.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-b2c@1.620.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-core@1.228.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-giga@0.347.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-homeds@0.347.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-hope@1.374.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-icons@1.239.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-new-hope@0.364.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-tokens-b2b@1.56.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-tokens-b2c@0.67.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-tokens-web@1.71.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-tokens@1.140.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-typo@0.44.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-ui@1.350.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-web@1.622.0-canary.2836.26952968753.0 yarn add @salutejs/sdds-bizcom@0.352.0-canary.2836.26952968753.0 yarn add @salutejs/sdds-cs@0.356.0-canary.2836.26952968753.0 yarn add @salutejs/sdds-dfa@0.350.0-canary.2836.26952968753.0 yarn add @salutejs/sdds-finai@0.343.0-canary.2836.26952968753.0 yarn add @salutejs/sdds-insol@0.347.0-canary.2836.26952968753.0 yarn add @salutejs/sdds-netology@0.351.0-canary.2836.26952968753.0 yarn add @salutejs/sdds-os@0.22.0-canary.2836.26952968753.0 yarn add @salutejs/sdds-platform-ai@0.351.0-canary.2836.26952968753.0 yarn add @salutejs/sdds-sbcom@0.352.0-canary.2836.26952968753.0 yarn add @salutejs/sdds-scan@0.350.0-canary.2836.26952968753.0 yarn add @salutejs/sdds-serv@0.351.0-canary.2836.26952968753.0 yarn add @salutejs/core-themes@0.31.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-themes@0.52.0-canary.2836.26952968753.0 yarn add @salutejs/sdds-themes@0.67.0-canary.2836.26952968753.0 yarn add @salutejs/sdds-api-tests@0.9.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-cy-utils@0.158.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-sb-utils@0.228.0-canary.2836.26952968753.0 yarn add @salutejs/plasma-tokens-utils@0.52.0-canary.2836.26952968753.0