Add onChange functionality to our DDF select fields

[skateman]

This is a generalization of the ProviderSelectField that is being used in the provider forms to load the provider schema based on the user's selection in a dropdown. I adjusted the component and the provider form to solely rely on attributes passed to the schema instead of using a react context. We are using the PF3 select with a custom placeholder which has been defined directly in the mapper. In order to avoid circular dependencies, I exported this into a separate file and. Then I renamed the component and moved it out to the global mapper so we can utilize it in other forms as well.

The dynamic loadin' is sorted out via the onChange function that gets called on any change in the dropdown with the new value as a single argument. Ideally this can be used together with the state to alter the schema as we do it in the provider forms.

const [fields, setFields] = useState([]);

const dynamicSelect = {
  component: 'select',
  name: 'type',
  label: 'Type',
  options: [{label: 'a', value: 'a'}, {label: 'b', value: 'b'}],
  onChange: value => {
    API.get(`/foo/bar&value=${value}`).then(data => setFields(data.fields)),
  },
};

<MiqFormRenderer ... schema={{ fields: [dynamicSelect, ...fields] }} />

@DavidResende0 this is how we're going to load the fields from the API if @himdel agrees

[himdel]

How does a select have a "schema"? It should have options, but it already has static options in your example,
so what does the schema actually do in the select?

Reading the example, that "schema" is not actually related to the select at all, in fact it just changes the rest of the form.

IMO that's architecturally wrong, the form itself (our react component wrapping MiqFormRenderer), should be doing that kind of logic.

A form field should not be affecting other form fields from inside itself.

[skateman]

@himdel there's already an example of this in the provider forms: when you select what kind of provider you want to create, we send an API request to retrieve the schema related to the given provider. If you don't like the name of the component, I'm open for better ideas, this is again the problem of starting a metal band.

[himdel]

No, my problem is not with the name, it's with one component changing the whole form schema, instead of the form changing the schema depending on the value from that component.

EDIT: or a select-with-a-subform where that select only affects the subform schema

But If you're just refactoring that for some other use, go for it 👍.
IMHO it should not exist in the first place.

[skateman]

No, my problem is not with the name, it's with one component changing the whole form schema, instead of the form changing the schema depending on the value from that component.

If you're just refactoring that for some other use, go for it.
But it should not exist in the first place.

The component is not changing anything, the function is defined in the form and passed to the component for calling it when its value changes. This is just an uglier onChange function, I can even rename it to onChange if it doesn't drive the renderer crazy (but I'm worried it would). We could listen to changes from the outside using the debug attribute on the MiqFormRenderer but IMO that's something we should avoid. We could also create a FormSpy but that needs to live inside the form as well and it's less effective as it cannot just subscribe to the changes of a single field. So AFAIK this is the most effective way to deal with API-provided schema changes in DDF which we really need and not just for the provider forms.

[himdel]

Oooh.. ok, then select-with-onchange probably makes more sense, yeah 👍

[skateman]

@himdel updated

[skateman]

I'm thinking that maybe we can just simply enhance the existing Select component (as it is already customized) with a conditional useEffect (not sure if possible) that would be called when the onChange attribute is set. This way we don't need the exotic component name. WDYT @himdel

[himdel]

LGTM :) reimplements the provider select field on top of a new select with an onchange handler exposed.

I'm thinking that maybe we can just simply enhance the existing Select component (as it is already customized) with [onChange]

Up to you :) Then we would have a select that supports onChange, while none of the other standard components do, but then again, it makes sense to have it in the select. OTOH one more custom thing to remember.

[miq-bot]

Checked commits skateman/manageiq-ui-classic@4dc7048~...564596d with ruby 2.6.3, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
0 files checked, 0 offenses detected
Everything looks fine. ⭐

[skateman]

@himdel changed, I guess it's ready for the final round