feat(Form): Add <PhoneInput /> and <DateInput />; Clean up <Select /> #34

kylealwyn · 2019-03-22T23:26:37Z

Adds new <PhoneInput /> and <DateInput /> components for easy input masking
Fixes some existing styling issues with <Select />

codecov-io · 2019-03-22T23:28:29Z

Codecov Report

Merging #34 into master will decrease coverage by 21.19%.
The diff coverage is 82.22%.

@@            Coverage Diff             @@
##           master      #34      +/-   ##
==========================================
- Coverage   88.88%   67.69%   -21.2%     
==========================================
  Files          13       21       +8     
  Lines         198      421     +223     
  Branches       31       88      +57     
==========================================
+ Hits          176      285     +109     
- Misses         15      106      +91     
- Partials        7       30      +23

Impacted Files	Coverage Δ
src/utils.js	`94.59% <100%> (+1.04%)`	⬆️
src/Form/Input.js	`46.55% <100%> (ø)`
src/Form/EasyInput.js	`66.66% <66.66%> (ø)`
src/Form/PhoneInput.js	`75% <75%> (ø)`
src/Form/DateInput.js	`87.5% <87.5%> (ø)`
src/Form/Label.js	`33.33% <0%> (ø)`
src/Form/FormError.js	`50% <0%> (ø)`
src/Form/Field.js	`100% <0%> (ø)`
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6b0d9b...c606806. Read the comment docs.

kylealwyn · 2019-03-27T17:51:49Z

@heydoctor/engineering

erikshestopal · 2019-03-27T18:33:17Z

EDIT: removing node_modules and rerunning npm did the trick.
~~Running yarn docs crashes with the following trace:~~

Uncaught TypeError: Cannot set property 'jsx' of undefined
    at module.exports (webpack-internal:///./node_modules/acorn-jsx/inject.js:362)
    at eval (webpack-internal:///./node_modules/acorn-jsx/index.js:3)
    at Object../node_modules/acorn-jsx/index.js (:1235/static/js/vendors.js:227)
    at __webpack_require__ (:1235/static/js/runtime~app.js:801)
    at fn (:1235/static/js/runtime~app.js:164)
    at eval (webpack-internal:///./node_modules/buble/dist/buble-browser.es.js:9)
    at Module../node_modules/buble/dist/buble-browser.es.js (:1235/static/js/vendors.js:532)
    at __webpack_require__ (:1235/static/js/runtime~app.js:801)
    at fn (:1235/static/js/runtime~app.js:164)
    at eval (webpack-internal:///./node_modules/react-live/dist/react-live.es.js:26)
    at Module../node_modules/react-live/dist/react-live.es.js (:1235/static/js/vendors.js:10903)
    at __webpack_require__ (:1235/static/js/runtime~app.js:801)
    at fn (:1235/static/js/runtime~app.js:164)
    at eval (webpack-internal:///./node_modules/docz-theme-default/dist/index.m.js:48)
    at Module../node_modules/docz-theme-default/dist/index.m.js (:1235/static/js/vendors.js:5128)
    at __webpack_require__ (:1235/static/js/runtime~app.js:801)
    at fn (:1235/static/js/runtime~app.js:164)
    at Module.eval (webpack-internal:///./.docz/app/root.jsx:6)
    at eval (webpack-internal:///./.docz/app/root.jsx:59)
    at Module../.docz/app/root.jsx (:1235/static/js/app.js:34)
    at __webpack_require__ (:1235/static/js/runtime~app.js:801)
    at fn (:1235/static/js/runtime~app.js:164)
    at Module.eval (webpack-internal:///./.docz/app/index.jsx:6)
    at eval (webpack-internal:///./.docz/app/index.jsx:70)
    at Module../.docz/app/index.jsx (:1235/static/js/app.js:22)
    at __webpack_require__ (:1235/static/js/runtime~app.js:801)
    at fn (:1235/static/js/runtime~app.js:164)
    at Object.0 (:1235/static/js/app.js:791)
    at __webpack_require__ (:1235/static/js/runtime~app.js:801)
    at checkDeferredModules (:1235/static/js/runtime~app.js:60)
    at Array.webpackJsonpCallback [as push] (:1235/static/js/runtime~app.js:47)
    at :1235/static/js/app.js:1

erikshestopal · 2019-03-27T18:49:59Z

@kylealwyn Played around with the inputs to try and break them. Let me know if these are valid bugs:

Trying to delete a backslash makes the cursor jump to the end:

Trying to delete a parenthesis makes the cursor jump to the end:

erikshestopal · 2019-03-27T18:44:02Z

src/Form/PhoneInput.js

+    }
+  };
+
+  return <Input forwardedRef={ref} value={currentValue} onChange={handleChange} {...inputProps} />;


Should we set a maxLength of 10 numbers (or 11 with country code)?

Can we make this input type="tel" by default so we can have a numpad on mobile devices?

Should we specify the expected format as a parenthetical aspect in the label (e.g. (123) 123-1234). Should we also allow the character required for the format? For example, if a user types in "(" they are currently blocked from doing so, which might be unintuitive if the format is specified in the label. Or maybe not. Perhaps we could consider masking (https://www.uxbooth.com/articles/the-new-rules-of-form-design/, https://uxplanet.org/designing-perfect-text-field-clarity-accessibility-and-user-effort-d03c1e26004b).

That behavior is dictated by Google's libphonenumber to disallow the leading paren so I'd rather not get in the way. It's also more difficult to type a paren on mobile when the numpad is presented.

kylealwyn · 2019-03-29T18:21:13Z

@erikshestopal Country code support is in

cehsu

These are really slick. I agree with Erik's comments.

cehsu · 2019-03-29T00:02:01Z

src/Form/Select.js

-            <option value="">{placeholder || 'Select an option...'}</option>
+        <SelectContainer value={value} size={size}>
+          <SelectInput name={name} value={value} onChange={this.handleChange} onBlur={this.handleBlur}>
+            <SelectOption value="">{placeholder || 'Select an option...'}</SelectOption>


These guidelines advise against using an option to replace a label (which is not exactly what is happening here): https://designsystem.digital.gov/components/form-controls/

Instead, we could add any additional guidance as a parenthetical portion of the label: States (please select a state), as it seems like the label is typically used to convey instructions to users.

Additional examples:
https://www.w3.org/TR/wai-aria-practices-1.1/examples/listbox/listbox-collapsible.html
https://webaim.org/techniques/forms/controls
http://www.webaxe.org/accessible-custom-select-dropdowns/

src/Form/Select.js

src/Form/Input.js

src/Form/DateInput.js

cehsu · 2019-03-29T18:23:38Z

src/Form/PhoneInput.js

+    }
+  };
+
+  return <Input forwardedRef={ref} value={currentValue} onChange={handleChange} {...inputProps} />;


Should we specify the expected format as a parenthetical aspect in the label (e.g. (123) 123-1234). Should we also allow the character required for the format? For example, if a user types in "(" they are currently blocked from doing so, which might be unintuitive if the format is specified in the label. Or maybe not. Perhaps we could consider masking (https://www.uxbooth.com/articles/the-new-rules-of-form-design/, https://uxplanet.org/designing-perfect-text-field-clarity-accessibility-and-user-effort-d03c1e26004b).

…p to DateInput (#36) ## Fix I forgot to include theses exports in #34 😞Seems like something a robot should disallow ## Add Also adds support to DateInput for an `initialValue` prop. This solves an issue where the database stores ISO dates (YYYY-MM-DD) but we want to display MM/DD/YYYY on the client. We can't format the `value` prop, otherwise we'll incorrectly format incomplete dates on every render.

feat(Form): Form and Date Inputs

d57edf1

kylealwyn added 4 commits March 22, 2019 16:33

Handle case where passed value changes

fc62575

Save date inputs

f97a037

Add DateInput and tests

427ab40

Clean up

88ca3ce

kylealwyn changed the title ~~feat(Form): Form and Date Inputs~~ feat(Form): Add <FormInput /> and <DateInput />; Clean up <Select /> Mar 23, 2019

Add some comments

d449ba8

kylealwyn requested a review from a team March 25, 2019 22:53

choochootrain changed the title ~~feat(Form): Add <FormInput /> and <DateInput />; Clean up <Select />~~ feat(Form): Add <PhoneInput /> and <DateInput />; Clean up <Select /> Mar 25, 2019

kylealwyn marked this pull request as ready for review March 27, 2019 17:51

erikshestopal reviewed Mar 27, 2019

View reviewed changes

kylealwyn added 3 commits March 29, 2019 11:11

Phone input handles country codes

0d2c50c

Comment how shouldAppendSlash is defined

103cecc

Define cursor position in DateInput like we do in PhoneInput

bb6c314

cehsu approved these changes Mar 29, 2019

View reviewed changes

kylealwyn added 5 commits March 29, 2019 11:41

Rename IconContainer; Forward props and ref to select input

045d19a

Add support for custom delimiter

b5696b3

Move country code support to formatPhoneNumber to format initial value

2b67ece

Move old value before new value in arguments

984bb21

Remove final slash reference

c794767

kylealwyn force-pushed the feat/phone-date-inputs branch from 5d1636f to c794767 Compare March 29, 2019 20:06

erikshestopal approved these changes Mar 29, 2019

View reviewed changes

kylealwyn added 2 commits March 31, 2019 21:18

Prevent value changes in onKeyDown

1ad0d4f

Check for length being shorter instead of looking at backspace

c606806

kylealwyn merged commit 801047a into master Apr 1, 2019

kylealwyn deleted the feat/phone-date-inputs branch April 1, 2019 18:24

kylealwyn mentioned this pull request Apr 5, 2019

fix(DateInput): Export PhoneInput and DateInput; Add initialValue prop to DateInput #36

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Form): Add <PhoneInput /> and <DateInput />; Clean up <Select /> #34

feat(Form): Add <PhoneInput /> and <DateInput />; Clean up <Select /> #34

kylealwyn commented Mar 22, 2019 •

edited

Loading

codecov-io commented Mar 22, 2019 •

edited by codecov bot

Loading

kylealwyn commented Mar 27, 2019

erikshestopal commented Mar 27, 2019 •

edited

Loading

erikshestopal commented Mar 27, 2019

erikshestopal Mar 27, 2019

cehsu Mar 29, 2019

kylealwyn Mar 29, 2019

kylealwyn commented Mar 29, 2019

cehsu left a comment

cehsu Mar 29, 2019

cehsu Mar 29, 2019

feat(Form): Add <PhoneInput /> and <DateInput />; Clean up <Select /> #34

feat(Form): Add <PhoneInput /> and <DateInput />; Clean up <Select /> #34

Conversation

kylealwyn commented Mar 22, 2019 • edited Loading

codecov-io commented Mar 22, 2019 • edited by codecov bot Loading

Codecov Report

kylealwyn commented Mar 27, 2019

erikshestopal commented Mar 27, 2019 • edited Loading

erikshestopal commented Mar 27, 2019

erikshestopal Mar 27, 2019

Choose a reason for hiding this comment

cehsu Mar 29, 2019

Choose a reason for hiding this comment

kylealwyn Mar 29, 2019

Choose a reason for hiding this comment

kylealwyn commented Mar 29, 2019

cehsu left a comment

Choose a reason for hiding this comment

cehsu Mar 29, 2019

Choose a reason for hiding this comment

cehsu Mar 29, 2019

Choose a reason for hiding this comment

kylealwyn commented Mar 22, 2019 •

edited

Loading

codecov-io commented Mar 22, 2019 •

edited by codecov bot

Loading

erikshestopal commented Mar 27, 2019 •

edited

Loading