Skip to content

fix: holding left click breaks mask #959

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
claytonlin1110 wants to merge 3 commits into
base: master
Choose a base branch
from
+20 −2
Open

fix: holding left click breaks mask #959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
663937f
fix: holding left click breaks mask
claytonlin1110 Mar 4, 2026
fde8fad
fix: align practice
claytonlin1110 Mar 4, 2026
5b1a920
fix: add test
claytonlin1110 Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/PickerInput/Selector/Input.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,16 +334,21 @@ const Input = React.forwardRef<InputRef, InputProps>((props, ref) => {
const rafRef = React.useRef<number>();

useLayoutEffect(() => {
if (!focused || !format || mouseDownRef.current) {
if (!focused || !format) {
return;
}

// Reset with format if not match
// Reset with format if not match (always apply when focused so mask works when focusing by mousedown)
if (!maskFormat.match(inputValue)) {
triggerInputChange(format);
return;
}

// When mousedown get focus, defer selection to mouseUp so click position is used
if (mouseDownRef.current) {
return;
}

// Match the selection range
inputRef.current.setSelectionRange(selectionStart, selectionEnd);

Expand Down
13 changes: 13 additions & 0 deletions tests/new-range.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1010,6 +1010,19 @@ describe('NewPicker.Range', () => {
expect(startInput.selectionEnd).toEqual(6);
});

it('focus by mousedown defers selection sync to mouseUp', () => {
const { container } = render(<Demo />);

const startInput = container.querySelectorAll<HTMLInputElement>('input')[0];

fireEvent.mouseDown(startInput);
fireEvent.focus(startInput);

fireEvent.mouseUp(startInput);
expect(startInput.selectionStart).toBeDefined();
expect(startInput.selectionEnd).toBeDefined();
});
Comment on lines +1013 to +1024
Copy link

@coderabbitai coderabbitai bot Mar 5, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

这个用例的断言过弱，无法真正覆盖“延迟到 mouseUp”行为

当前只在 mouseUp 后用 toBeDefined() 断言，无法验证“mouseDown/focus 阶段不应同步选区”，而且该断言对 null 也会通过。建议补充 mouseUp 前后对比断言，明确验证延迟同步语义。

建议修改示例 
 it('focus by mousedown defers selection sync to mouseUp', () => {
   const { container } = render(<Demo />);

   const startInput = container.querySelectorAll<HTMLInputElement>('input')[0];

   fireEvent.mouseDown(startInput);
   fireEvent.focus(startInput);
+
+  // 模拟按下时的点击位置（尚未 mouseUp）
+  startInput.selectionStart = 5;
+  startInput.selectionEnd = 5;
+
+  // mouseDown 期间不应被掩码同步覆盖
+  expect(startInput.selectionStart).toEqual(5);
+  expect(startInput.selectionEnd).toEqual(5);

   fireEvent.mouseUp(startInput);
-  expect(startInput.selectionStart).toBeDefined();
-  expect(startInput.selectionEnd).toBeDefined();
+  // mouseUp 后才同步到对应 mask cell（5 -> month cell: 4~6）
+  expect(startInput.selectionStart).toEqual(4);
+  expect(startInput.selectionEnd).toEqual(6);
 });
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/new-range.spec.tsx` around lines 1013 - 1024, The test "focus by
mousedown defers selection sync to mouseUp" only asserts selectionStart/End are
defined after mouseUp, which is too weak; update the test around Demo/startInput
to assert selectionStart and selectionEnd are null (or unchanged) immediately
after fireEvent.mouseDown and fireEvent.focus to prove selection is not
synchronized yet, then assert they become numeric (not null/undefined) after
fireEvent.mouseUp; reference the existing calls fireEvent.mouseDown(startInput),
fireEvent.focus(startInput), fireEvent.mouseUp(startInput) and the
selectionStart/selectionEnd checks when adding these pre- and post-mouseUp
assertions.


it('blur to reset back text', async () => {
const { container } = render(<Demo />);

Expand Down