primitives icon indicating copy to clipboard operation
primitives copied to clipboard

fix: [One-Time Password Field] truncate values ​​that exceed the length when pasting

Open nayounsang opened this issue 9 months ago • 4 comments

Description

close #3550

  • truncate values exceed the length of collections when pasting
  • There is a concern that SRP may be violated when inserting the corresponding logic into sanitizeValue, and therefore, side effects are a concern, so verification is performed in the paste logic.
  • Test code is skipped because the pasted values ​​of @testing-library/user-event are not assigned to input.

Test with Storybook Case: paste 124868392

<input autocomplete="off" autocapitalize="off" autocorrect="off" autosave="off" spellcheck="false" readonly="" type="hidden" 
value="124868" 
name="code">

image

nayounsang avatar May 19 '25 08:05 nayounsang

🦋 Changeset detected

Latest commit: 6981e19b0b7271e8ee5d444ecb7c99ebd4befe33

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@radix-ui/react-one-time-password-field Patch
radix-ui Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar May 19 '25 08:05 changeset-bot[bot]

Thanks! Mind adding a unit test to cover this case?

chaance avatar May 20 '25 16:05 chaance

@chaance I add todo test because user.paste not linked to input data. This is same to the paste test above.

nayounsang avatar May 23 '25 07:05 nayounsang

@chaance

  it('should truncate pasted characters to the number of inputs', async () => {
    const inputs = screen.getAllByRole<HTMLInputElement>('textbox', {
      hidden: false,
    });
    const firstInput = inputs[0]!;
    fireEvent.click(firstInput);
    
    const pasteData = '123456789';
    fireEvent.paste(firstInput, {
      clipboardData: {
        getData: () => pasteData
      }
    });
    
    expect(getInputValues(inputs)).toBe('1,2,3,4,5,6');
  });

I found a way to fix the paste test so that it can simulate fireEvent that are not userEvent. However, in my philosophy, it seems better to avoid using fireEvent. What should I do?

nayounsang avatar May 23 '25 07:05 nayounsang