update FormGroup and InputLabel to support fieldset and legend

[jasonbasuil]

closes #631

Continuing from my initial PR-616, I think this could possibly be one / easiest? path forward to supporting <fieldset> and <legend> with our existing FormGroup with an initial cutover in order to be screen reader accessible on some key flows that need it.

• Adds inputType prop on <FormGroup> that is set to a string of 'radio' or 'checkbox'. If inputType == 'radio' or 'checkbox', the <FormGroup> is rendered as a <fieldset> otherwise it renders our normal <div>
• That inputType gets passed down to the <InputLabel> which will determine whether the default <label> is changed to a <legend>.

I tested it out locally on RS and it seems to work okay if given the correct inputType on a given <FormGroup>. Also checked after running our tests and there doesn't seem to be a difference / any breaking tests as it relates to getByLabelText ... at least what I saw when label gets switched out for legend (or the tests were not relevant to these changes currently). My thinking is that when the switch does happen (only when we decide to set inputType) and it does happen to break a test, I think we could just change getByLabelText to something like getByRole("group", { name: "Some legend that acts as label for radio or checkbox groups" }));

Example on app/javascript/researcher/surveys/survey_form/survey_question_form_group.jsx when applied:

fieldset.legend.example.mov

Possible iteration? -> Updating RadioButtonGroup and CheckboxButtonGroup to handle the setting legend instead of the FormGroup?

[rroppolo]

Suggested change

	const renderFormGroupChildren = () => (
	const formGroupChildren = (

Since the functional component is executed on every render there's not really a need for this to be inside a function. It could just be assigned to a const. Or if it starts to get more complicated, broken out into a child component

[rroppolo]

I though we might have this in the react style guide but I can't find it :( I think maybe it was just raised as a proposal but not officially adopted yet by the team. But the proposal was the use of renderFoo functions inside a component be deprecated in favor of splitting things out into child components to break up a large render method. But wondering here if we can remove some of the duplication anyway through with something like

   return createElement(
     elementType,
      {
        className: classNames(
          'FormGroup',
          props.className, {
            'FormGroup--is-invalid': hasErrors,
            'FormGroup--bordered': props.bordered,
            'FormGroup--inline': props.inline,
          },
        ),
        id: props.id
      },
      formGroupChildren,
    )

We do this in some other places like Card where we take in an elementType prop to make the root element type dynamic.

[rsaris]

I generally agree w/ this proposal (and it could be used in InputLabel as well though I do wonder if that would do better w/ a Boolean (or maybe we make an InputLabel / InputLegend which we use well here.

[rroppolo]

I'm a little worried there may be a lot of other input types not represented here, like selects or datepickers. Wondering if we would want to make this prop a boolean and more indicative that it is controlling the fieldset? Like elementType="fieldset"? We could always make a RadioFormGroup component that acts as a wrapper that passes the correct elementType through to FormGroup like

const RadioFormGroup = (props) => (
  <FormGroup 
    elementType="fieldset"
    inputElementType="legend"
    {...props} 
  />
);

if we wanted something a bit more semantic that is still easy to selectively roll out. Then the consumer just needs to switch FormGroup to RadioFormGroup/CheckboxFormGroup and everything else stays the same? Assuming we set a defaultValue for those two props to the non-fieldset case (ie. divs)

[rsaris]

Do we need to scope this to the specific element we're working on or is this something that should be in a generic style override?

As this is written now -- it will apply to every legend in the app, which may not be correct.

[rsaris]

I generally agree w/ this proposal (and it could be used in InputLabel as well though I do wonder if that would do better w/ a Boolean (or maybe we make an InputLabel / InputLegend which we use well here.

[rroppolo]

LGTM! Thanks for making that change! 💯 🌮 🙌