@ahmadabdolsaheb I would appreciate your opinion on the wording/styling of the hint:

What about Please submit the link to the live version of your app?

@nhcarrigan It would need to be shorter than it already is, not longer; I do not want it wrapping on smaller screens.

Currently, this is at a point where I could see it being merged. All it would help with is warnings about invalid URLs, and I have set up the ground work I would expect for better error handling, and URL validation.

EDIT: I need to handle the tests...

I cannot seem to pass the Cypress test. Here is the failure:

 2) A certification,
       while viewing someone else's,
         "before all" hook for "should not render a LinkedIn button":
     AssertionError: Timed out retrying: Expected to find content: 'Sign me out of freeCodeCamp' but never did.

I am still busy trying to setup tests for GitPod, so cannot test this locally (laptop is struggling). Otherwise, I do not think I touched anything related in this PR, anyway.

Hey @SKY020, there's one issue stemming from the change to formatUrlValues. That's used in handleSubmitLegacy, so that also needs to handle the { values: ...} object.

@ahmadabdolsaheb I would appreciate your opinion on the wording/styling of the hint:

The message is descriptive. I would add a dot to the end of the message for consistency 👍
Although we are using different elements, we should unify the padding between the form and hint/error across our UI at some point.

Since only SolutionForm sets the DisplayError and only FormFields gets it and FormFields is the direct descendent of SolutionForm (SolutionForm > Form > FormFields), I think it would be better to have DisplayError as a state on SolutionForm and pass it down as props.

We could use Context to avoid passing it down one component at a time.

That way FormFields does not need to be a connected component and we can free up some space on the redux store.

Additional suggestions:
1)isOpenCompletionModal could be changed to something more descriptive such as 'isCompletionModalDisplayable'.
2) Some e2e tests are also appreciated.

Considerations:

Since we already have all the info we need for opening the completion modal in execute-challenge-saga, I cannot think of a good reason to have challenge-modal-epic, what do you think?

For the backend and and frontend shows, we could move the submit completely to solutionForm since it is a connected component and has the logic to handle executeChallenge. (That would simplify them a bit and make them good candidates for hookification

I am not fan of how we communicate validation information from FormFields --> Form --> SolutionForm --> Form (shows the error), but I see how it would help to keep form stateless.

We are setting isCompletionModalShouldOpen to the redux store with the executeChallenge. Then we would read isCompletionModalShouldOpen from store in execute-challenge-saga when executeChallenge is called.

Since isCompletionModalShouldOpen is supplied to executeChallenge as payload and it is only used in this context, It might make more sense to just use as payload directly in execute-challenge-saga and not set it in the store.

Also, what do you think of shouldCompletionModalOpen as an alternative name?

@ahmadabdolsaheb I have compromised on the name of the variable - now isShouldCompletionModalOpen

Also, I have passed it as an argument directly (bypassing the store). However, it is currently in the payload as true, and not { isShouldCompletionModalOpen: true }. I have mixed thoughts about the clarity of this, so what do you think?

Quick note (we don't have to fix it on this pr IIMHO) : unvalidated (localhost) submissions to the legacy cert section of the settings page would go through, and the hints would show up for all previous submissions.

I have tested submitting front/backend projects with this pr and it works as expected. Other than the nit-picks mentioned above it is LGMT.

Quick note (we don't have to fix it on this pr IIMHO) : unvalidated (localhost) submissions to the legacy cert section of the settings page would go through, and the hints would show up for all previous submissions.

For the most part, this looks like wanted behaviour. I think we do have an issue up which intends to add checks to submissions from the settings page. So, this would be resolved in that PR (perhaps, I can take a look).

@ahmadabdolsaheb Thanks a tonne, for the suggestion about removing the displayErrors state. This is simpler.