Skip to content

fix(timepicker): prevent extra digits when editing incomplete time (#DS-3985) - #1854

Open
KamilEmeleev wants to merge 2 commits into
mainfrom
fix/DS-3985
Open

fix(timepicker): prevent extra digits when editing incomplete time (#DS-3985)#1854
KamilEmeleev wants to merge 2 commits into
mainfrom
fix/DS-3985

Conversation

@KamilEmeleev

Copy link
Copy Markdown

Summary

You could type endless digits into an incomplete value: 11:1 plus digits at the start grew to 911:1, 9911:1. Nothing limited how long a time part could be.

List of notable changes:

  • added normalizeTimePart because the old check only caught big numbers, not long ones like 001
  • updated replaceNumbers to cut every part down to two digits
  • updated onInput to write the fixed value back while the time is still incomplete, and move the caret to the next part
  • added unit tests and a Playwright test

What should reviewers focus on?

  • Typing digit by digit still works — 1, 12:, 12:3 are left alone
  • The caret now jumps on the incomplete path too
  • The bad value still hits the DOM for a moment: this fixes the value, it does not block the key

@KamilEmeleev KamilEmeleev added the bug Something isn't working label Aug 6, 2026
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

🚨 E2E tests failed

Review the report for details.


💡 Comment /approve-snapshots to approve snapshot changes.

this.selectionStart = selectionStart;
this.selectionEnd = newTimeObj ? selectionEnd : selectionStart;

this.createSelectionOfTimeComponentInInput((selectionStart as number) + 1);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selectionStart может быть null, нигде выше это не проверяется, надо добавить проверку

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an input-masking edge case in KbqTimepicker where repeatedly typing digits into an incomplete time (e.g. 11:1) could grow a segment unbounded (e.g. 911:1, 9911:1). The change introduces normalization that limits each time segment to two digits and updates the input view while the value is still incomplete, keeping the caret behavior consistent.

Changes:

  • Add normalizeTimePart and update replaceNumbers to clamp and trim each time segment to a max of 2 digits.
  • Update onInput to rewrite normalized incomplete values back to the input and maintain caret movement/selection behavior.
  • Add unit + Playwright coverage for the “growing digits” scenario, and add stable data-testid hooks for E2E selection.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
packages/components/timepicker/timepicker.directive.ts Normalizes oversized time parts and rewrites incomplete values to prevent unbounded growth while preserving caret behavior.
packages/components/timepicker/timepicker.spec.ts Adds unit tests for trimming/clamping behavior and ensuring intermediate typing states remain unchanged.
packages/components/timepicker/e2e.ts Adds data-testid attributes to timepicker inputs for reliable E2E targeting.
packages/components/timepicker/e2e.playwright-spec.ts Adds an E2E regression test covering repeated digit insertion on an incomplete value.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@artembelik

Copy link
Copy Markdown
Contributor

/redeploy-preview

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

🔄 Redeploying documentation preview.

@artembelik

Copy link
Copy Markdown
Contributor

редеплой сообщение с ссылкой на доку не показал, вот адрес

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants