Redesigned and refactored new import flow

[rohitkrai03]

Fixes

• https://issues.redhat.com/browse/HAC-3660

Description

• Updated the import flow to new design.
• Refactored the components based on new flow and removed FormikWizard.

Type of change

• Feature

Screen shots / Gifs for design review

Screen.Recording.2023-04-28.at.7.09.18.PM.mov

TODO

• Fix e2e tests.

Browser conformance:

• Chrome
• Firefox
• Safari
• Edge

[codecov]

Codecov Report

Merging #592 (9f402b5) into main (cdbf9fe) will decrease coverage by 0.09%.
The diff coverage is 88.40%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #592      +/-   ##
==========================================
- Coverage   81.20%   81.12%   -0.09%     
==========================================
  Files         464      468       +4     
  Lines       11175    11212      +37     
  Branches     3176     3186      +10     
==========================================
+ Hits         9075     9096      +21     
- Misses       1976     1992      +16     
  Partials      124      124              

Impacted Files	Coverage Δ
src/components/ImportForm/utils/transform-utils.ts	100.00% <ø> (ø)
src/components/ImportForm/utils/types.ts	100.00% <ø> (ø)
.../components/ImportForm/utils/useDevfileSamples.tsx	100.00% <ø> (ø)
...rc/components/ImportForm/utils/validation-utils.ts	100.00% <ø> (ø)
src/shared/components/formik-fields/field-types.ts	100.00% <ø> (ø)
...ponents/name-value-editor/BasicNameValueEditor.tsx	62.00% <ø> (ø)
...ponents/ImportForm/ReviewSection/ReviewSection.tsx	78.57% <66.66%> (-0.38%)	⬇️
src/components/ImportForm/GitImportForm.tsx	76.71% <76.71%> (ø)
...omponents/ImportForm/SourceSection/AuthOptions.tsx	96.00% <83.33%> (-0.16%)	⬇️
src/components/ImportForm/SampleImportForm.tsx	85.36% <85.36%> (ø)
... and 17 more

... and 2 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cdbf9fe...9f402b5. Read the comment docs.

[rottencandy]

If I change app name, runtime detection gets triggered

recording-1682692369.mp4

If I go back when runtime detection is in progress, the import button stays disabled

recording-1682692800.mp4

[jrichter1]

After I input a valid repo and make a change to the URL, it lets me proceed even if the repo is no longer valid

Screencast.from.05-02-2023.09.28.09.AM.webm

[jrichter1]

also the help/docs in the Learn more section will need updates

[jrichter1]

There is also no way to name a new application when creating it with a sample component, let alone the component itself.

[jrichter1]

Some shenanigans with runtime selection:

• manually selecting dockerfile prevents from going back to the recommended runtime
• selecting something else and going back to the recommended runtime just hangs the runtime detection

Screencast.from.05-02-2023.11.43.06.AM.webm

[rohitkrai03]

If I change app name, runtime detection gets triggered

recording-1682692369.mp4
If I go back when runtime detection is in progress, the import button stays disabled

recording-1682692800.mp4

@rottencandy Fixed the issues.

[rohitkrai03]

After I input a valid repo and make a change to the URL, it lets me proceed even if the repo is no longer valid

Screencast.from.05-02-2023.09.28.09.AM.webm

@jrichter1 Fixed it.

[rohitkrai03]

also the help/docs in the Learn more section will need updates

Yeah, docs team is working on updating the learn more topics.

[rohitkrai03]

There is also no way to name a new application when creating it with a sample component, let alone the component itself.

Yeah, that's what the UX recommended.

[rohitkrai03]

Some shenanigans with runtime selection:

• manually selecting dockerfile prevents from going back to the recommended runtime
• selecting something else and going back to the recommended runtime just hangs the runtime detection

Screencast.from.05-02-2023.11.43.06.AM.webm

Fixed it.

[Misjohns]

Spacing should be consistent within the containers. I'm using 64px spacing so it's not so tight.

[Misjohns]

For the secret section:

• Secret remove is too far away from the field; use same placement as the environment variable field placement.
• Secret fields are very long. Let's align those with the variable name field.

[Misjohns]

Import screen:

• Git repository URL* and Context directory should both have a question mark icon that accesses a pop-over
• "Context dir" should be spelled out "Context directory"

Unable to access repo screen:

• We should not show the Git reference, Context directory or Import code button if the repo can't be accessed.

[Misjohns]

Detecting values:
We don't need 2 "detecting…" screens. Let's keep the background overlay with "Detecting values…" message on card until we can load the components.

[Misjohns]

Samples:
Need to align the link buttons so cards are easier to scan

[rohitkrai03]

For the secret section:

• Secret remove is too far away from the field; use same placement as the environment variable field placement.
• Secret fields are very long. Let's align those with the variable name field.

@Misjohns This PR doesn't have ability to add secrets. #595 adds that.

[Misjohns]

Component card:

• Runtime value does not need to be bold
• GH link does not need to be bold

[rohitkrai03]

Detecting values: We don't need 2 "detecting…" screens. Let's keep the background overlay with "Detecting values…" message on card until we can load the components.

@Misjohns We cannot do this. Importing sample and importing git repos is two different forms put together now. We need to detect values for git repo on the review page because we have a runtime selector that can be used to change the detected values. Changing the runtime from selector triggers the same CDQ detection. That's why we cannot detect the values on the source step and move to review only after detection completes. We would need a consistent progress mechanism for when user first land and CDQ happens and also when user changes the runtime and CDQ happens.
We could add the backdrop version on review page though but I feel that could be too in your face to disable everything and show spinner when user just changed the runtime.

Also, the spinner on samples flow says Importing sample... and not Detecting value... because user doesn't need to know what we're doing under the hood (running detection and importing sample). He just knows that he clicked on Import sample so the progress bar should convey that his action is in progress.

Also, we don't show two detecting screens in one flow. For samples, we have the backdrop version because we want to disable all the action buttons available while a sample is importing. For git repo, we have the inline version because of the reasons explained above (here user can press back and go back to source screen without any issues).

[Misjohns]

@rohitkrai03 Based on your comment "We would need a consistent progress mechanism for when user first land and CDQ happens and also when user changes the runtime and CDQ happens." Can you please share video of changing the runtime and CDQ happens, so I can see the progress message?

[rohitkrai03]

@rohitkrai03 Based on your comment "We would need a consistent progress mechanism for when user first land and CDQ happens and also when user changes the runtime and CDQ happens." Can you please share video of changing the runtime and CDQ happens, so I can see the progress message?

@Misjohns This how it looks right now.
https://user-images.githubusercontent.com/6041994/235757121-9ac69fe7-3c0e-478a-9c6d-1455e72bf685.mov

[rohitkrai03]

@rohitkrai03 Based on your comment "We would need a consistent progress mechanism for when user first land and CDQ happens and also when user changes the runtime and CDQ happens." Can you please share video of changing the runtime and CDQ happens, so I can see the progress message?

@Misjohns This is how it looks if we use the backdrop version on the review page as well. This seems like a reasonable way of keeping things consistent between samples and git repo flows. Detection while changing the runtime happens inline within the dropdown anyways.

Although we do loose out on extra information which explains what we are doing when we say Detecting values...

Screen.Recording.2023-05-03.at.12.20.36.AM.mov

[Misjohns]

@rohitkrai03 The new card version is much better. The initial version looked like an empty state with that extra copy which doesn't add much value if the user doesn't have time to read it.

[rohitkrai03]

Spacing should be consistent within the containers. I'm using 64px spacing so it's not so tight.

@Misjohns Added consistent spacing within the container. Based on PF's internal padding and margin within components, I've set the padding top for the icon and padding bottom for the CTA buttons within the container to pf-u-pt-lg and pf-u-pb-lg.

[rohitkrai03]

Import screen:

• Git repository URL* and Context directory should both have a question mark icon that accesses a pop-over
• "Context dir" should be spelled out "Context directory"

Unable to access repo screen:

• We should not show the Git reference, Context directory or Import code button if the repo can't be accessed.

@Misjohns Done.

Screen.Recording.2023-05-03.at.1.05.09.AM.mov

[rohitkrai03]

Samples: Need to align the link buttons so cards are easier to scan

@Misjohns Done.

[rohitkrai03]

Component card:

• Runtime value does not need to be bold
• GH link does not need to be bold

@Misjohns Done.

[jrichter1]

/retest

[openshift-ci]

@rohitkrai03: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[karthikjeeyar]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: karthikjeeyar, rohitkrai03

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [karthikjeeyar,rohitkrai03]

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