[BD-46] feat: added FormControl input mask #2626

PKulkoRaccoonGang · 2023-09-15T09:36:18Z

Description

added FormControl input mask.

Issue: #2458

Deploy Preview

Form.Control component

Merge Checklist

If your update includes visual changes, have they been reviewed by a designer? Send them a link to the Netlify deploy preview, if applicable.
Does your change adhere to the documented style conventions?
Do any prop types have missing descriptions in the Props API tables in the documentation site (check deploy preview)?
Were your changes tested using all available themes (see theme switcher in the header of the deploy preview, under the "Settings" icon)?
Were your changes tested in the example app?
Is there adequate test coverage for your changes?
Consider whether this change needs to reviewed/QA'ed for accessibility (a11y). If so, please add wittjeff and adamstankiewicz as reviewers on this PR.

Post-merge Checklist

Verify your changes were released to NPM at the expected version.
If you'd like, share your contribution in #show-and-tell.
🎉 🙌 Celebrate! Thanks for your contribution.

netlify · 2023-09-15T09:36:23Z

✅ Deploy Preview for paragon-openedx ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`d6dc0e3`
🔍 Latest deploy log	https://app.netlify.com/sites/paragon-openedx/deploys/655de87e89bb540008252f8a
😎 Deploy Preview	https://deploy-preview-2626--paragon-openedx.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

openedx-webhooks · 2023-09-15T09:36:24Z

Thanks for the pull request, @PKulkoRaccoonGang!

When this pull request is ready, tag your edX technical lead.

codecov · 2023-09-18T10:00:33Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (484a30e) 92.82% compared to head (d6dc0e3) 92.83%.
Report is 57 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2626   +/-   ##
=======================================
  Coverage   92.82%   92.83%           
=======================================
  Files         235      235           
  Lines        4237     4241    +4     
  Branches     1029     1030    +1     
=======================================
+ Hits         3933     3937    +4     
  Misses        300      300           
  Partials        4        4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/Form/FormControl.jsx

src/Form/tests/FormControl.test.jsx

src/Form/FormControl.jsx

adamstankiewicz · 2023-11-20T13:47:36Z

src/Form/form-control.mdx

+## Input masks
+Paragon uses the [react-imask](https://www.npmjs.com/package/react-imask) library,
+which allows you to add masks of different types for inputs.
+To create your own mask, you need to pass the required mask pattern (`+{1}(000)000-00-00`) to the `inputMask` property. <br />


The US-based example phone number mask should likely be +{1} (000) 000-0000 so that phone numbers are formatted like +1 (555) 555-5555, which is typical for a US-based phone number.

Corrected, thanks

adamstankiewicz · 2023-11-20T13:47:38Z

src/Form/form-control.mdx

+        <h3>Phone</h3>
+        <Form.Group>
+          <Form.Control
+            inputMask="+{1}(000)000-00-00"


The US-based example phone number mask should likely be +{1} (000) 000-0000 so that phone numbers are formatted like +1 (555) 555-5555, which is typical for a US-based phone number.

adamstankiewicz · 2023-11-20T13:49:29Z

src/Form/form-control.mdx

+      <h3>Date</h3>
+      <Form.Group>
+        <Form.Control
+          inputMask={Date}


A bit curious about this one; it appears it doesn't let users input months without leading zeros? For example, when trying to type in June 10, I type 6 for the month, but it doesn't accept any input until I add a leading zero first.

Beyond this, I'm also wondering if we should keep the date input mask example of these examples given the Form.Control can be used specifically with a date picker input type.

It seems to me that this is a good solution if the mask requires entering a zero before the month.
But this example seems redundant since we have a better use case (Form.Control).
Removed this example

adamstankiewicz · 2023-11-20T13:50:38Z

src/Form/form-control.mdx

+    </>),
+    price: (
+        <>
+          <h3>Course prise</h3>


nit: "Price"

adamstankiewicz · 2023-11-20T13:51:08Z

src/Form/form-control.mdx

+      <>
+        <Form.Group>
+          <Form.Control
+            inputMask="+{1}(000)000-00-00"


The US-based example phone number mask should likely be +{1} (000) 000-0000 so that phone numbers are formatted like +1 (555) 555-5555, which is typical for a US-based phone number.

adamstankiewicz · 2023-11-20T13:53:25Z

src/Form/tests/FormControl.test.jsx

+
+    act(() => {
+      const input = screen.getByTestId('form-control-with-mask');
+      userEvent.type(input, '1234567890');


[curious] generally, userEvent.* functions are called outside of act since the userEvent.* functions are already wrapped with act (example source).

@adamstankiewicz thank you for bringing this issue to our attention 👍

At the moment we have a lot of similar examples of wrapping in act userEvent in Paragon. I've looked into several interesting sources (including the one you shared), this is really an unnecessary solution, here are some confirmations:

When should I use act() in react-testing-library?

Internal implementation

Is wrapping with act now required?

My solution is based on Is wrapping with act now required?. As it turned out, we have differences in the @testing-library/dom subdependency in the versions of the @testing-library/[email protected] and @testing-library/[email protected] libraries. A dependency conflict negatively affects the performance of the userEvent and we see a warning after running tests.

We have several options for further action:

Apply now the solution with an override of the @testing-library/dom package (with version 9.3.3). This way we will get rid of the problem of displaying warnings in the console and our tests will be cleaner. This solution can be developed in a separate task, where we remove the wrapping in act userEvent. I have verified this in the Tabs component and in my current case (FormControl).

"overrides": { "@testing-library/dom": "9.3.3" }

Make these changes and “synchronize” the @testing-library/dom subdependency in @testing-library/react and @testing-library/user-event in the pull request for updating to React 18. So It is quite likely that we will not have to do this since we will update the versions of @testing-library/react and @testing-library/user-event. But we cannot predict whether @testing-library/dom versions will change in the future. Therefore, the override option seems workable.

An article about dependency overriding, in which we can make sure that our intentions are correct:

There should only be a single copy of a given dependency in our tree, but npm's dependency resolution will result in a duplicate due to version conflicts.

Thanks for digging into, this @PKulkoRaccoonGang! TIL that @testing-library/dom has to be synchronized :) I agree with your synopsis/options; interesting that I'm not sure this has come up before in MFEs that use RTL. But I agree the overrides is sufficient to hold over until the React 18 upgrade.

adamstankiewicz · 2023-11-20T13:54:01Z

src/Form/tests/FormControl.test.jsx

+    render(<Component isClearValue />);
+
+    const input = screen.getByTestId('form-control-with-mask');
+    fireEvent.change(input, { target: { value: '1234567890' } });


[curious] Why use userEvent.type but fireEvent.change here? Ideally, we could rely on userEvent to more closely mimic real user behavior.

openedx-semantic-release-bot · 2023-12-08T13:57:33Z

🎉 This PR is included in version 21.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

openedx-semantic-release-bot · 2023-12-08T14:01:59Z

🎉 This PR is included in version 22.0.0-alpha.20 🎉

The release is available on:

Your semantic-release bot 📦🚀

openedx-webhooks added the blended PR is managed through 2U's blended developmnt program label Sep 15, 2023

PKulkoRaccoonGang marked this pull request as draft September 15, 2023 09:36

PKulkoRaccoonGang mentioned this pull request Sep 15, 2023

Extend Form.Control with input mask #2458

Closed

PKulkoRaccoonGang force-pushed the Peter_Kulko/form-control-mask branch 3 times, most recently from 3b88d2e to 029f3d9 Compare September 18, 2023 10:48

PKulkoRaccoonGang marked this pull request as ready for review September 18, 2023 10:49

PKulkoRaccoonGang requested review from monteri, viktorrusakov and adamstankiewicz September 18, 2023 10:50

monteri reviewed Sep 18, 2023

View reviewed changes

src/Form/FormControl.jsx Outdated Show resolved Hide resolved

src/Form/FormControl.jsx Outdated Show resolved Hide resolved

src/Form/tests/FormControl.test.jsx Outdated Show resolved Hide resolved

PKulkoRaccoonGang force-pushed the Peter_Kulko/form-control-mask branch from 62b9d12 to 4687fed Compare September 18, 2023 16:11

PKulkoRaccoonGang requested a review from monteri September 18, 2023 19:00

PKulkoRaccoonGang force-pushed the Peter_Kulko/form-control-mask branch from 4687fed to 9ef3e26 Compare September 18, 2023 19:18

viktorrusakov reviewed Sep 20, 2023

View reviewed changes

src/Form/FormControl.jsx Outdated Show resolved Hide resolved

src/Form/FormControl.jsx Outdated Show resolved Hide resolved

PKulkoRaccoonGang added 5 commits October 20, 2023 12:27

feat: added FormControl input mask

87699d6

feat: added code examples

662c5e6

feat: added tests

aa37c4f

refactor: refactorng after review

98726d2

refactor: refactoring after review

c1c1a1e

PKulkoRaccoonGang force-pushed the Peter_Kulko/form-control-mask branch from a236cc6 to c1c1a1e Compare October 20, 2023 09:29

refactor: refactoring after rebase

e565967

adamstankiewicz reviewed Nov 20, 2023

View reviewed changes

refactor: refactoring after review

e06aa95

PKulkoRaccoonGang marked this pull request as draft November 21, 2023 14:05

refactor: package override @testing-library/dom

d6dc0e3

PKulkoRaccoonGang marked this pull request as ready for review November 22, 2023 11:44

PKulkoRaccoonGang requested a review from adamstankiewicz November 22, 2023 11:44

adamstankiewicz approved these changes Dec 8, 2023

View reviewed changes

adamstankiewicz merged commit 8f2c06c into openedx:master Dec 8, 2023
10 checks passed

openedx-semantic-release-bot added the released label Dec 8, 2023

openedx-semantic-release-bot added the released on @alpha label Dec 8, 2023

khudym pushed a commit to khudym/paragon that referenced this pull request Jan 2, 2024

feat: added FormControl input mask (openedx#2626)

d6f6a7c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BD-46] feat: added FormControl input mask #2626

[BD-46] feat: added FormControl input mask #2626

PKulkoRaccoonGang commented Sep 15, 2023 •

edited

Loading

netlify bot commented Sep 15, 2023 •

edited

Loading

openedx-webhooks commented Sep 15, 2023 •

edited

Loading

codecov bot commented Sep 18, 2023 •

edited

Loading

adamstankiewicz Nov 20, 2023

PKulkoRaccoonGang Nov 22, 2023

adamstankiewicz Nov 20, 2023

adamstankiewicz Nov 20, 2023

PKulkoRaccoonGang Nov 22, 2023 •

edited

Loading

adamstankiewicz Nov 20, 2023

PKulkoRaccoonGang Nov 22, 2023

adamstankiewicz Nov 20, 2023

adamstankiewicz Nov 20, 2023

PKulkoRaccoonGang Nov 22, 2023 •

edited

Loading

adamstankiewicz Dec 8, 2023

adamstankiewicz Nov 20, 2023

openedx-semantic-release-bot commented Dec 8, 2023

openedx-semantic-release-bot commented Dec 8, 2023

[BD-46] feat: added FormControl input mask #2626

[BD-46] feat: added FormControl input mask #2626

Conversation

PKulkoRaccoonGang commented Sep 15, 2023 • edited Loading

Description

Deploy Preview

Merge Checklist

Post-merge Checklist

netlify bot commented Sep 15, 2023 • edited Loading

✅ Deploy Preview for paragon-openedx ready!

openedx-webhooks commented Sep 15, 2023 • edited Loading

codecov bot commented Sep 18, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PKulkoRaccoonGang Nov 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PKulkoRaccoonGang Nov 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openedx-semantic-release-bot commented Dec 8, 2023

openedx-semantic-release-bot commented Dec 8, 2023

PKulkoRaccoonGang commented Sep 15, 2023 •

edited

Loading

netlify bot commented Sep 15, 2023 •

edited

Loading

openedx-webhooks commented Sep 15, 2023 •

edited

Loading

codecov bot commented Sep 18, 2023 •

edited

Loading

PKulkoRaccoonGang Nov 22, 2023 •

edited

Loading

PKulkoRaccoonGang Nov 22, 2023 •

edited

Loading