[candidate_parameters] Making the Proband Info Tab extensible

[gluneau]

This pull request allows the Proband tab to be extensible just like the Candidate Info tab.

The magic happens because of parameter_type_category.Name='Candidate Parameters Proband'

[driusan]

@jacobpenny Can you review the javascript in this?

[jacobpenny]

I'm getting some errors:
Uncaught ReferenceError: Tabs is not defined
Uncaught ReferenceError: TabPane is not defined

This is because these components are no longer in the global scope. This change was made in this PR. This module should have been updated at that time but wasn't.

So, you'll need to import these components where they are needed, i.e. add the following line to the rest of the import statements in CandidateParameters.js and recompile:
import {Tabs, TabPane} from 'Tabs';

Because we are now bundling the Tab components we shouldn't try to fetch it, and if we do try we just get an error. The file doesn't exist anymore:
GET http://localhost/js/components/Tabs.js 404 (Not Found)

So you need to remove this line

[jacobpenny]

SelectElement expects an object for options, not an array:
Warning: Failed prop type: Invalid prop `options` of type `array` supplied to `SelectElement`, expected `object`.

Should be:
var selectOptions = {};

[jacobpenny]

Rather than use for ... in and doing this hasOwnProperty check you could use forEach, which won't iterate inherited properties.
extraParameters.forEach(function(extraParam) { ... })

Better yet would be to use map. It looks like you are creating a component for each extraParameter, i.e. a 1-to-1 mapping.

I'd write it like:

var extraParameters = this.state.Data.extra_parameters;
var extraParametersFields = extraParameters.map(function(extraParam) { ... });

If you decide to use forEach or map, you'll need to change extraParameters[key2] to extraParam in the body of the function. If you use map, instead of pushing, just return the component.

[jacobpenny]

Side note: this render function is getting pretty long. I'd break this logic out into another function:

var extraParametersFields = extraParameters.map(this._convertExtraParamToInputComponent);

[jacobpenny]

Same as above: using forEach would be cleaner IMO.

types.forEach(function(type) {
  selectOptions[type] = type;
});

[jacobpenny]

Alignment issue here. I'm wondering why eslint didn't catch this.

[jacobpenny]

I'm not really sure what extraParameters[key2].Type.substring(0, 3) is.
I can infer it from the case statements but IMO it would be easier to read/understand if this were a named variable:

var extraParamType = extraParameters[key2].Type.substring(0, 3)
switch (extraParamType) { ... }

(I'm just guessing that it is the extra parameter type)

[jacobpenny]

Looks good!

[driusan]

@jacobpenny did you test it or just review it?

[jacobpenny]

@driusan Did basic testing but lack the data to fully test.