kubevirt: VMWizard - redesign and import the first step (General)

[atiratree]

depends on:

• shared: PopoverStatus - support active icon and showing popover without header #2453

• bump patternfly/react-core #2677

• adds form helpers like FormFieldRow

• uses immutables and memoization for performance

• adds EnhancedK8sMethods

• move wizard logic to actions

@mareklibra @rawagner please review

according to the new design
http://openshift.github.io/openshift-origin-design/web-console/knikubevirt/knikubevirt

@matthewcarleton

[atiratree]

/hold
for 4.3

[atiratree]

The wizard is not complete yet, but the features and steps will be added incrementally in the future

[matthewcarleton]

@rawagner this is looking great! Thank you for doing this :)
Here's a few design tweaks I think we could do to better align things with PatternFly:
There should be a next button on the first step.
Neither the Create and Review button or the Next button should be disabled as they should alert the user of the needed fields before they proceed.
The tooltip icon should be pf-icon-help and #72767B unless active then it should be #0066CC
We've dropped the tooltip from the Operating System (we only have it on Source, Flavor, and Workload profile)
We've renamed Provision source to just Source to align with the storage step.
The alert you are showing should be updated to use the inline variation see here
Is this alert shown after they have tried to proceed? If so does it scroll the user back to the top so they can see it?
If possible, I think we should use input help text(red) rather than larger inline ones if it's referring to a specific input.
Does the container image field need to be a text area? I've also wondered with the fields where they just paste in a path if we should provide helper text below to give some guidance.
Sorry this got so long :)

[atiratree]

@rawagner this is looking great! Thank you for doing this :)
Here's a few design tweaks I think we could do to better align things with PatternFly:
There should be a next button on the first step.

There will be in the future, but I put Review and Create just for now because this PR adds just the first step.

Neither the Create and Review button or the Next button should be disabled as they should alert the user of the needed fields before they proceed.

We are doing real-time validation of input, so the user doesn't have click on the button to see the error messages. This way it is more natural to keep in disabled until all required fields are valid.

The tooltip icon should be pf-icon-help and #72767B unless active then it should be #0066CC
We've dropped the tooltip from the Operating System (we only have it on Source, Flavor, and Workload profile)
We've renamed Provision source to just Source to align with the storage step.
The alert you are showing should be updated to use the inline variation see here

All done!

Is this alert shown after they have tried to proceed? If so does it scroll the user back to the top so they can see it?
If possible, I think we should use input help text(red) rather than larger inline ones if it's referring to a specific input.

User will see these alerts when they open the dialog. These are alerts for the resources which could not load correctly. And these might be used by multiple fields or none.

Does the container image field need to be a text area? I've also wondered with the fields where they just paste in a path if we should provide helper text below to give some guidance.

switched to text area

Sorry this got so long :)

np, thanks for the comments :)

current version:

[matthewcarleton]

@suomiy
+1 on your changes :)
disabled buttons:
Having fields dynamically validate is awesome but I still think these buttons shouldn't be disabled.
My reasoning is mostly because of accessibility. There's lots of scenarios where a user (keyboard only, screen reader, high zoom) will have problems and not understand why. They might miss the announcement of the error, they might be zoomed in and not see it or their vision could be poor and they don't notice it.
With a disabled button we don't have the opportunity to help them when these scenarios take place. We just assume they will figure it out on their own (which likely they will eventually, or just give up).
Having the button enabled helps them understand what's wrong with the form via an alert when they've tried to submit without success. This puts them in a place where they don't have to problem solve only act on the guidance they're given.
What if we did something like this:

User will see these alerts when they open the dialog. These are alerts for the resources which could not load correctly. And these might be used by multiple fields or none.

Is this a real scenario or just a placeholder? I'm assuming in this scenario they've tried to create this VM and ran into this error and are now back in the wizard trying to correct it? If that's the case wouldn't they be on the results page? From there they would go to the storage step to correct the issue with the storage class so they could see this error on the page. What have I missed :) ?

[atiratree]

...
What if we did something like this:

I agree, you have good points. I am not tackling the VMWizard navigation & logic much in this PR.
I will try to do address these issues in the next PR, if that's okay?

Is this a real scenario or just a placeholder? I'm assuming in this scenario they've tried to create this VM and ran into this error and are now back in the wizard trying to correct it? If that's the case wouldn't they be on the results page? From there they would go to the storage step to correct the issue with the storage class so they could see this error on the page. What have I missed :) ?

Real scenario. No, they just opened the dialog without filling anything and are presented with these errors; usually permission issues which result in an inability of using the whole wizard.

[atiratree]

rebased

[matthewcarleton]

I agree, you have good points. I am not tackling the VMWizard navigation & logic much in this PR.
I will try to do address these issues in the next PR, if that's okay?

that would be great, thanks!

Real scenario. No, they just opened the dialog without filling anything and are presented with these errors; usually permission issues which result in an inability of using the whole wizard.

Ah ok, I wonder then if we should just present them with this error and guidance on how to fix it. I suppose there isn't much value in providing all of the fields if they can't proceed until the resolve the issue. WDYT?

[atiratree]

Ah ok, I wonder then if we should just present them with this error and guidance on how to fix it. I suppose there isn't much value in providing all of the fields if they can't proceed until the resolve the issue. WDYT?

Not sure, because there are some errors which will disable the dialog only partially (e.g. datavolumes). You can still create some configurations without them.

[atiratree]

/retest

[atiratree]

/retest

[atiratree]

fixed issues mentioned in offline review

[atiratree]

/hold cancel

[atiratree]

/retest

[mareklibra]

?

[atiratree]

removed unnecessary prop

note: the full feature footer is implemented in the followup #2565

[mareklibra]

lgtm
waiting for the branch to merge this to

[rawagner]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rawagner, suomiy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• frontend/packages/kubevirt-plugin/OWNERS [rawagner,suomiy]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment