feat(Formbot): support for yup via validationSchema; Add Context provider and EasyInput consumer #20

choochootrain · 2019-02-22T23:51:05Z

supports yup schemas for validation:
pulled from EMR Formbot - validationSchema takes precedence over validations
manages state of Inputs within Formbot context:
see Add Context to Formbot with EasyInput consumer and HOC #21 for details
minor addition of Fieldset component:
pulled from EMR

codecov-io · 2019-02-22T23:51:09Z

Codecov Report

Merging #20 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #20   +/-   ##
=======================================
  Coverage   71.91%   71.91%           
=======================================
  Files          12       12           
  Lines         146      146           
  Branches       19       19           
=======================================
  Hits          105      105           
  Misses         31       31           
  Partials       10       10

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 e783265...6725c09. Read the comment docs.

codecov-io · 2019-02-22T23:51:09Z

Codecov Report

Merging #20 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #20   +/-   ##
=======================================
  Coverage   71.91%   71.91%           
=======================================
  Files          12       12           
  Lines         146      146           
  Branches       19       19           
=======================================
  Hits          105      105           
  Misses         31       31           
  Partials       10       10

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 e783265...e32d5d8. Read the comment docs.

kylealwyn · 2019-02-23T00:11:05Z

Let's add docz example using yup

src/Form/Formbot.js

src/Form/FormGroup.js

kylealwyn · 2019-02-23T01:07:34Z

After clicking submit in new yup example

cehsu

I'm getting an error:
"TypeError: this.validatable.map is not a function[Learn More]"
Do we need to add something like this? Or am I doing something wrong?

  constructor(props) {
    this.validatable = [];
  }

src/Form/Formbot.example.js

kylealwyn · 2019-02-25T19:03:26Z

src/index.js

@@ -16,6 +16,7 @@ export Flex from './Flex';
 export Formbot from './Form/Formbot';
 export FormError from './Form/FormError';
 export FormGroup from './Form/FormGroup';
+export FieldSet from './Form/FormGroup';


Fieldset should be an entirely new component. FormGroups refer to https://getbootstrap.com/docs/4.0/components/forms/#form-groups which adds bottom margin to adjacent inputs

added but FieldSet does something similar in the presence of adjacent elements - unclear how one would mix usage of FieldSet and FormGroup

hm also web-app has a different FieldSet which has a sublegend and is styled differently - should we leave them as is or theme FieldSets?

This reverts commit 622f235.

choochootrain · 2019-02-25T20:00:19Z

theres also PhoneInput, DateInput in web-app so this PR will get pretty big

i can ship this as is and update the EMR and we can do a second pass to clean up the web-app next hack day

kylealwyn · 2019-02-25T20:27:59Z

@choochootrain

Supernit: Can we rename FieldSet to Fieldset? HTML tag reads fieldset so I'd prefer sticking to convention and start casing?

added but FieldSet does something similar in the presence of adjacent elements - unclear how one would mix usage of FieldSet and FormGroup

Our FormGroup and Field are very similar. Most (should be all) of our input components are wrapped in <Field />, which adds top margin to siblings. FormGroup, on the other hand, always applies a bottom margin.

Fieldset is used for optionally grouping related fields if multiple sets exist and conventionally has the legend element as first descendant. See edit-profile.js in web app, where we group personal and address information.

The css class form-group in Bootstrap always applies a margin bottom, and we use it to wrap components in a form which may not have a margin applied, or most commonly, a row of inputs within columns where the css sibling selector breaks. This is more a sugar component for consistency, as we could do <Box mb={3}> and get the same results

<Form>
  <Fieldset legend="Some group of fields">
    <Input />
    <Input /> {/* Here we have adjacent Fields so this one has a top margin applied */}
  </Fieldset>

  <Fieldset legend="Some other group of fields">
    <FormGroup>
      <Row>
         <Col>
            <Input />
         </Col>
         <Col>
            <Input />
         </Col>
      </Row>
    </FormGroup>
  </Fieldset>

  <Button type="submit" />
</Form>

theres also PhoneInput, DateInput in web-app so this PR will get pretty big

i can ship this as is and update the EMR and we can do a second pass to clean up the web-app next hack day

This is the right call.

choochootrain · 2019-02-25T20:34:37Z

@kylealwyn looks like FormGroup uses Box in favor of bootstrap's form-group class

const FormGroup = ({ children, padding = 3 }) => <Box mb={padding}>{children}</Box>;

kylealwyn · 2019-02-25T21:09:28Z

@choochootrain sorry, only using bootstrap class as reference for why it exists

src/index.js

choochootrain · 2019-02-25T22:59:23Z

@sappira-inc/engineering take a look at the last change, i'm open to suggestions

Feat(formbot): support validation via yup schema

6725c09

choochootrain requested a review from a team February 22, 2019 23:51

Hurshal Patel added 3 commits February 22, 2019 16:12

Fix validatable = false case

79eaf73

Add yup example

334f3a9

Actually fix validatable

4e0c021