Skip to content
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.

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

fix: handle undefined item in select noEmpty validation and remove phone input default value #275

Merged
merged 5 commits into from
Aug 22, 2024

Conversation

matyasjay
Copy link
Member

@matyasjay matyasjay commented Aug 22, 2024

Description

  • Remove defaultValue prop from Phone input to make the input controlled explicitly
  • Add null check to item.value in the Select inputs noEmpty check
  • Add validation for required form errors to disable submit button if not valid

Motivation and Context

We ran into an unhandled error when the registerConfig was set to { required: false } or empty object and tried validating the select input.

There are console warnings about controlled/uncontrolled input behaviour when using Phone question.

Missing validation of required questions are allowing the user to submit the form even if the required fields are missing. This behaviour forces the developer to handle the error validation externally even when it happens inside the library already.

How Has This Been Tested?

Unit tests.

Screenshots (if appropriate):

 PASS  src/Questions/Country/__tests__/country.test.js
  ✓ check the placeholder text (38 ms)
  ✓ Country label (9 ms)
  ✓ change value of select (112 ms)
  ✓ sort country list by default (79 ms)
  ✓ handle country priority order (72 ms)
  ✓ check all the countries are rendered (13 ms)
  ✓ label tag is not displayed when label value is null (4 ms)
  ✓ renders a country list in spanish (71 ms)
  ✓ renders a country list in french (68 ms)
  ✓ renders a country list in deusche (67 ms)
  ✓ renders a fallback country list when the language is not supported (66 ms)
  ✓ show an error message (6 ms)
  ✓ Option values are country names (instead of country codes) if "returnCountryCode" is true (73 ms)

 PASS  src/__tests__/builder.test.js
  form builder without custom errors
    ✓ check if component is rendered (118 ms)
    ✓ check if questions are rendered (63 ms)
    ✓ check if it calls submit eventhandler once only when required fields are filled in (197 ms)
  form builder with custom errors
    ✓ check if component renders custom formErrors (79 ms)
  form builder with reCAPTCHA
    ✓ check it does not submit forms when they have a reCAPTCHA (18 ms)

 PASS  src/Questions/Autocomplete/__tests__/autocomplete.test.js
  ✓ check if component is rendered (19 ms)
  ✓ check if placeholder exists (5 ms)
  ✓ check if label exists (8 ms)
  ✓ label tag is not displayed when label value is null (5 ms)
  ✓ Change value of autocomplete select (384 ms)
  ✓ check if error exists (11 ms)

 PASS  src/Questions/County/_tests_/county.test.js
  ✓ check the placeholder text (22 ms)
  ✓ County label (8 ms)
  ✓ label tag is not displayed when label value is null (3 ms)
  ✓ County select appears when country question is clicked and GE value (89 ms)
  ✓ County select is opened on the right country region (84 ms)
  ✓ renders a fallback country list when country condition is not supported (36 ms)
  ✓ show an error message (5 ms)

 PASS  src/Questions/Phone/__tests__/phone.test.js
  ✓ label is displayed (27 ms)
  ✓ placeholder is displayed (16 ms)
  ✓ error is displayed (19 ms)
  ✓ phone can be filled (33 ms)
  ✓ phone ES country is displayed (22 ms)
  ✓ default country code is displayed (23 ms)
  ✓ pattern error is displayed (16 ms)
  ✓ country code is visible (13 ms)
  ✓ default value is displayed (14 ms)
  ✓ default value can be changed (23 ms)

 PASS  src/Questions/Date/__tests__/date.test.js
  ✓ required error is displayed (31 ms)
  ✓ under-age error error is displayed (14 ms)
  ✓ Select day dropdown is opened in the right date (12 ms)
  ✓ Select month dropdown is opened in the right date (13 ms)
  ✓ Select year dropdown is opened in the right date (11 ms)
  ✓ calendar is opened minAge years ago (12 ms)
  ✓ calendar is opened in new Date (14 ms)
  ✓ dateformat is applied (34 ms)
  ✓ calendar is in spanish (9 ms)
  ✓ calendar is in french (11 ms)
  ✓ calendar is in german (9 ms)
  ✓ calendar sending no-valid language (12 ms)

 PASS  src/Questions/Genre/_tests_/gender.test.js
  ✓ check the placeholder text (13 ms)
  ✓ Gender label (4 ms)
  ✓ label tag is not displayed when label value is null (7 ms)
  ✓ Change value of gender select (20 ms)
  ✓ Check custom options are rendered (17 ms)
  ✓ renders a country list in spanish (15 ms)
  ✓ renders a country list in french (13 ms)
  ✓ renders a country list in deusch (14 ms)
  ✓ renders a country list in swedish (12 ms)
  ✓ show an error message (4 ms)

 PASS  src/Questions/SelectImage/_tests_/selectImages.test.js
  ✓ check if component is rendered (31 ms)
  ✓ check if label exists (19 ms)
  ✓ can first option be checked/unchecked (18 ms)
  ✓ all options are rendered (16 ms)
  ✓ shows an error message (17 ms)

 PASS  src/Questions/Input/__tests__/input.test.js
  ✓ renders label with markdown (15 ms)
  ✓ handles default markdown link (16 ms)
  ✓ handles custom markdown link callback (11 ms)
  ✓ icon is displayed (4 ms)
  ✓ icon default is displayed (3 ms)
  ✓ icon tooltip is displayed (8 ms)
  ✓ icon tooltip is not displayed when there is no text (3 ms)
  ✓ icon is not displayed (2 ms)
  ✓ placeholder is displayed (5 ms)
  ✓ descriptions are displayed (6 ms)
  ✓ error is displayed (4 ms)
  ✓ input can be filled (5 ms)
  ✓ patern error is displayed (5 ms)

 PASS  src/Questions/Radio/__tests__/radio.test.js
  ✓ markdown is displayed (14 ms)
  ✓ handles default markdown link (19 ms)
  ✓ radio labels are displayed (10 ms)
  ✓ radio values are assigned (7 ms)
  ✓ error is displayed (9 ms)
  ✓ radio can be selected (8 ms)

 PASS  src/Questions/Select/__tests__/select.test.js
  ✓ check if component is rendered (18 ms)
  ✓ check if placeholder exists (6 ms)
  ✓ renders label with markdown (9 ms)
  ✓ handles default markdown link (14 ms)
  ✓ handles custom markdown link callback (9 ms)
  ✓ check if error exists (10 ms)
  ✓ check if option labels exist (20 ms)
  ✓ check if it preselects an option from the config (5 ms)

 PASS  src/Questions/MultipleCheckboxes/__tests__/multipleCheckboxes.test.js
  ✓ check if component is rendered (23 ms)
  ✓ check if label exists (13 ms)
  ✓ can first option be checked/unchecked (13 ms)
  ✓ all options are rendered (10 ms)
  ✓ shows an error message (9 ms)
  ✓ shows a max option error message (8 ms)
  ✓ shows a minimun option error message (9 ms)

 PASS  src/Questions/Age/_tests_/age.test.js
  ✓ check the placeholder text (12 ms)
  ✓ Age label (5 ms)
  ✓ label tag is not displayed when label value is null (3 ms)
  ✓ Change value of age select (21 ms)
  ✓ Check custom options are rendered (16 ms)
  ✓ show an error message (5 ms)

 PASS  src/Questions/Checkbox/__tests__/checkbox.test.js
  ✓ can checked/unchecked (16 ms)
  ✓ default checked false (3 ms)
  ✓ default checked true (3 ms)
  ✓ renders markdown (4 ms)
  ✓ shows an error message (8 ms)
  ✓ handles default markdown link (10 ms)
  ✓ handles custom markdown link callback (11 ms)

 PASS  src/Questions/CountrySubdivision/__tests__/countrySubdivision.test.js
  ✓ shows an error message if the field is required but doesn't have a value (21 ms)

 PASS  src/Questions/Recaptcha/__tests__/recaptcha.test.js
  ✓ renders a reCAPTCHA (2 ms)

 PASS  src/Questions/Markdown/__tests__/markdown.test.js
  Without links included
    ✓ Check if component is rendered (6 ms)
    ✓ Check if label exists (2 ms)
  With links included
    ✓ Check if component is rendered (5 ms)
    ✓ Check if hyperlink href was created (7 ms)

 PASS  src/Questions/CountrySubdivision/__tests__/buildCountrySubdivisonOptions.test.js
  ✓ buildCountrySubdivisonOptions returns all the regions of a country in a {label, value} format
  with the region iso code as value (6 ms)
  ✓ buildCountrySubdivisonOptions returns all the regions of a country in a {label, value} format
  with the region name as value (6 ms)
  ✓ buildCountrySubdivisonOptions returns all the regions of a country in a {label, value} format
  with the region full iso code as value (4 ms)
  ✓ buildCountrySubdivisonOptions returns all the regions of a country in a {label, value} format
  with the region full iso code as value and the region in the 'priorityOptions' array at the begining (6 ms)
  ✓ buildCountrySubdivisonOptions returns only the regions of a country in the whitelist in a {label, value} format
  with the region iso code as value
  ✓ buildCountrySubdivisonOptions returns all the regions of a country in a {label, value} format
  with the region iso code as value except the regions in the blacklist

Test Suites: 18 passed, 18 total
Tests:       127 passed, 127 total
Snapshots:   0 total
Time:        4.756 s, estimated 5 s
Ran all test suites.

Watch Usage: Press w to show more.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@matyasjay matyasjay changed the title Fix select config fix: handle undefined item in select noEmpty validation and remove phone input default value Aug 22, 2024
@matyasjay matyasjay marked this pull request as ready for review August 22, 2024 10:11
@matyasjay
Copy link
Member Author

@inigomarquinez @NuriaExtremadouro FYI can you guys give a review please?

@NuriaExtremadouro NuriaExtremadouro merged commit 39fa184 into onebeyond:main Aug 22, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants