Validate pipeline form dialogs

[karlaspuldaro]

Fixes #658

Exporting or submitting a pipeline now requires the user to populate all required form fields.
Required fields in dialog body should contain attribute data-form-required.
While no selection is made or required inputs are empty, the OK button remains disabled.

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

[ajbozarth]

I like this as a POC, but if we're going to add this functionality I'd rather we "wrap" showDialog in our own "utility" function that can handle this validation more generally based on supplied params.

[lresende]

Can we also make sure Submit dialog does not have the same problem.

[vabarbosa]

I like this as a POC, but if we're going to add this functionality I'd rather we "wrap" showDialog in our own "utility" function that can handle this validation more generally based on supplied params.

i may be misunderstanding what you are asking, but we can't "wrap" showDialog in our own "utility" function that can handle this validation because showDialog returns a promise that get resolved only after the user has clicked on a dialog button. at which time it would be too late to prevent (or control) any action by the user.

what could perhaps be done is take the approach used here and create a more generic function that is reusable by some of the other dialogs prompts in Elyra (and would replace using JupyterLab's default showDialog).

would you know if this is a functionality JupyterLab would welcome? in which case it be added to JupyterLab's Dialog instead.

[ajbozarth]

what could perhaps be done is take the approach used here and create a more generic function that is reusable by some of the other dialogs prompts in Elyra (and would replace using JupyterLab's default showDialog).

Sorry for the poor clarity, this is exactly what I meant

would you know if this is a functionality JupyterLab would welcome? in which case it be added to JupyterLab's Dialog instead.

This is a good question, we should ask, this would be cleaner if done in lab.

[karlaspuldaro]

As per conversation with @vabarbosa we came up with a generic showFormDialog method that would live in packages/ui-compoments, that could be used by any elyra component.
The idea is to provide:

1. A generic validation, where form fields marked as required would be considered valid once populated
2. A way to handle more specific validations would to be passed as a function, so that the caller could do other things like checking for a proper string format in an input field.
   Below is a prototype of how the utility would look like (initially it would only have the validate-and-show-dialog function):

/*
 * Validate required form dialog fields upon display
 *
 * options - The dialog setup options
 * formValidationFunc - Optional validation function passed by the caller to provide their own logic
 *
 */
async showFormDialog(options, formValidationFunc) {
  const dialogBody = options.body;
  const dialog = new Dialog(options);
  if (formValidationFunc && typeof formValidationFunc === 'function') {

    // pass dialog widget to the caller for more specific logic and other dialog behaviors,
    // such as being able to disable other buttons that aren't the default one
    formValidationFunc(dialog);
  } else {

    // expect required fields to contain attribute: data-elyra-required
    const requiredFields = dialogBody.node.querySelectorAll('[data-elyra-required]');
    if (requiredFields.length > 0) {

      // disable defaultButton as default
      // add appropriate change event listener to each required field
      // that enables the defaultButton when field is populated
    }
  }
  return dialog.launch();
}

How would others feel about this approach?

[ajbozarth]

@karlaspuldaro that approach looks great to me, looking forward to seeing the detailed implementation

[lresende]

Maybe don't use 'elyraon thedata-elyra-required`, but otherwise look good

[karlaspuldaro]

NOTE: The Dialog widget will skip any validation upon pressing 'enter' key (needs to resolve in lab)

The generic validation implemented only checks if all required fields are populated. In the case of pipeline name input field, we check for a string not null, while a more detailed string validation is already handled by the backend.

[vabarbosa]

looks good. nice work. i just added a couple small comments

[vabarbosa]

NOTE: The Dialog widget will skip any validation upon pressing 'enter' key (needs to resolve in lab)

for this i think you can listen to the keydown event and prevent default behavior/propagation if Enter key is pressed and validation is not satisfied

[ajbozarth]

I'll be taking a look at this later today

[ajbozarth]

A couple of clarifying questions, but LGTM