feat(Form): Add form related components

[sharvit]

What:
Add form related components
closes #118, closes #117, closes #116

Link to Storybook

Discussion:

1. I created some Field level components (e.g. Stories/HorizontalFormField.js), do you think those components should get exported to consumers?
2. Do you think I should get rid of some InputGroups because they do not feel like patternfly?

Additional issues:

• This pr is based on feat(DropdownButton): DropdownButton and SplitButton components #130, please merge feat(DropdownButton): DropdownButton and SplitButton components #130 before

[jgiardino]

Thanks for adding these, @sharvit! These examples look really nice!!

To answer your second question, I think we should keep the Input Groups examples. We have these same examples on the patternfly test page for Forms

Here are my comments based on reviewing your examples...
@serenamarie125 - Please also review these comments and make sure they line up with your expectations too.

1. In the PatternFly styleguide for terminology and wording the recommendation is to use "Log In" as the button label for the Login form pattern that you show for the Inline Form.
2. The PatternFly design for validation does not include icons in the validation states. When I modify the KNOBS so that a validation state is selected and Show Feedback Icon is enabled (i.e. in an example like Horizontal Form), icons display in the input. This is visually an issue in the field labeled "My meeting URL" which includes an example of an input group with a button inside the input field. In this case, the icon and button overlap (see example below). Can we remove the KNOB "Show Feedback Icon" from these examples?
   • I realize this option would still be available in the component, but we are hoping to avoid confusion with developers who want to implement patternfly patterns by not including any storybook examples that are not consistent with the patternfly examples.
   • 
3. Some of the examples include a KNOB labeled "Show Loading" which will display a spinner on the form. I'm actually not familiar with any patternfly examples that show a spinner on a form, but there are a couple of things I noticed related to the spinner:
   • When using the inline spinner with a paragraph, the spinner-xs class should be used. Sizes per text elements are available on the patternfly spinner test page
   • The spacing between the spinner and text on the patternfly test page seems to created with just a space character. I would suggest following that, instead of applying the 10px padding to the text.
   • @serenamarie125 Are you aware of any examples that show a form layout with a spinner included, both for modal and non-modal forms? I'm not sure what the patternfly standard might be in this case.

[sharvit]

Thanks for your feedback @jgiardino !

To answer your second question, I think we should keep the Input Groups examples. We have these same examples on the patternfly test page for Forms

👍

1. Updated to Log in

2. Do you mean to remove the FormControlFeedback from all the stories?
   If so, I think I should remove this component from patternfly-react so it can be consumed from react-bootstrap or just add it as a normal markup. So we won't supply components which are not consistent with patternfly.

3. I found the spinner example here (at the bottom of this page): http://www.patternfly.org/pattern-library/forms-and-controls/buttons-on-forms/

   • Updated the size.
   • Updated the spacing and removed the padding.

[jgiardino]

(testing the waffle board integration with this comment...)
closes #116
closes #117
closes #118

---

actually, adding 'closes #n' is not supported in the comment of a PR, so I added this to the description

[waldenraines]

@sharvit can you rebase this now that #130 is merged please?

[waldenraines]

Bad copy/paste here I think.

[sharvit]

@waldenraines - rebased and updated the description.

[waldenraines]

LGTM

[jgiardino]

1. Updated to Log in

Doh, I'm sorry, I just realized that the Terminology and Wording documentation shows the action capitalized as "Log in" which is probably confusing. I failed to mention that the button label should be headline style capitalization with a capital "I" in "Log In". The capitalization recommendations are a little further down on this page.

2. Do you mean to remove the FormControlFeedback from all the stories?
   If so, I think I should remove this component from patternfly-react so it can be consumed from react-bootstrap or just add it as a normal markup. So we won't supply components which are not consistent with patternfly.

My concern with that approach is that we might not be able to consume many react-bootstrap components at all. :-) I don't think the patternfly-react components should necessarily prevent developers from accessing other variations that are available in react-bootstrap but are not supported in the patternfly design docs. I think we want to just not present these variations in the storybook so that the patternfly-react storybook always only shows patternfly-approved patterns.

So in this case, I would suggest that we remove the KNOB for "Show Feedback Icon" and not have this property enabled in the storybook examples.

3. I found the spinner example here (at the bottom of this page): http://www.patternfly.org/pattern-library/forms-and-controls/buttons-on-forms/

Thanks for sharing that example. I have totally overlooked it. :-) This one looks great to me!

[priley86]

@sharvit please rebase once more, apologies. #137 changes comma settings.

[sharvit]

@priley86 - rebased

Thanks for the feedback @jgiardino

Doh, I'm sorry, I just realized that the Terminology and Wording documentation shows the action capitalized as "Log in" which is probably confusing. I failed to mention that the button label should be headline style capitalization with a capital "I" in "Log In". The capitalization recommendations are a little further down on this page.

That's fine, updated 👍

My concern with that approach is that we might not be able to consume many react-bootstrap components at all. :-) I don't think the patternfly-react components should necessarily prevent developers from accessing other variations that are available in react-bootstrap but are not supported in the patternfly design docs. I think we want to just not present these variations in the storybook so that the patternfly-react storybook always only shows patternfly-approved patterns.

So in this case, I would suggest that we remove the KNOB for "Show Feedback Icon" and not have this property enabled in the storybook examples.

Agree, done.

What do you think about exporting the VerticalFormField, HorizontalFormField and the InlineFormField?
tbh, i got mixed feelings about it. It gives easier way for implementing forms, but it takes away flexibility.

[priley86]

@sharvit personally, I don't see the value of adding those iterators in the Vertical Form, Horizontal Form, and Inline Form examples just for the sake of making it look like a "real" form with all those fields. I would guess most devs will just be searching for the inner examples on how to use those elements/jsx. So maybe we can just simplify to one example of each variation w/o iterating them? Just personal opinion here...

[sharvit]

@priley86 I agree with you here, will wait to see how is the experience of using those components.

[jgiardino]

No objections from me, @priley86.
Thanks for the updates, @sharvit!

I just happened to click the link to the react-bootstrap documentation for forms, and the url must have recently changed from https://react-bootstrap.github.io/components.html#forms to https://react-bootstrap.github.io/components/forms/
:-(

Other than that, this LGTM!

[jeff-phillips-18]

LGTM

[sharvit]

@jgiardino - Nice catch, seems they updated the whole UI.

I have rebased, updated the URL and redeployed the storybook

I guess it also means we need to update the urls everywhere?
Would suggest writing some API around the documentation urls so it will be easier to upgrade next time.

[priley86]

@sharvit @jgiardino this should be easy enough. We can write a helper and update all links in a subsequent PR.

[priley86]

opened #154 to address the documentation links