Show errors inline and prevent form submit when errors present.

[jeffstieler]

Requires https://github.com/Automattic/woocommerce-connect-server/pull/273 to test.

Fixes #206 and incorporates some error UI design from #306.

To test:

• Erase a required field value (like Title or Origin)
• Verify the error message is displayed
• Verify the "Save changes" button is disabled
• Enter an invalid zip code for Origin and
• Verify the error message is displayed
• Verify the "Save changes" button is disabled
• Add a package with only a name
• Verify that the package list item shows an error state
• Verify the "Save changes" button is disabled
• Force a bad value for a shipping service adjustment in the database by changing the serialized i:0 value to s:1:"a"
• Reload form
• Verify that the group containing that service is expanded, and the service adjustment is highlighted red with an icon

[jeffstieler]

I'm opening this up for review even though it's missing one piece that I originally considered as part of this effort, which is showing errors from the server inline on the form.

[allendav]

Optional. No need to assign to classes, just call classNames here

[jkudish]

!! is kinda sketchy as far as good practices go

also, why not just pass in hasError here directly since we never need the errors themselves in this component. I think the least info the components require, the better

[jkudish]

Do you think Redux has a part to play in error handling? Would it simplify passing error objects all over the different components?

[jeffstieler]

Do you think Redux has a part to play in error handling? Would it simplify passing error objects all over the different components?

I initially started with that, but then landed on this documentation (by way of reduxjs/redux#749) that made me rethink it.

[allendav]

I deleted all my instances and added a fresh instance of Canada Post. I see this in dev console when editing it for the first time:

[allendav]

Immediately after the above, if I click on Add a Package, I get the following:

[jeffstieler]

@allendav I can't reproduce your console errors. Can you try again now that I've rebased?

[allendav]

@jeffstieler i don't get those errors anymore since the rebase. i do get this now:

[jeffstieler]

@allendav as we discussed, the key warning in NoticesList occurs on master as well. Is this good to go?

[nabsul]

Since we don't allow the user to submit invalid form data, maybe we should relax the adjustment value field so that it allows the user to input invalid characters.

The way it works now is inconsistent with other text field above (zip, title, etc.). This would also hopefully resolve this issue: #188

When I tried editing the DB so that the value is a string, the field was highlighted, but there was no way to fix it because it wouldn't let me edit the field. Try adding a two-char string there.

[allendav]

Since we don't allow the user to submit invalid form data, maybe we should relax the adjustment value field so that it allows the user to input invalid characters.

I'd rather we didn't

The way it works now is inconsistent with other text field above (zip, title, etc.).

And I think that's appropriate for this field, since only numerics are allowed. Conceivably we could do the same for the ZIP code field as well, at least for USPS. But let's not make it easier for the user to enter bad data.

But if we can't figure out a way to allow only certain characters in a field (like decimals) I guess we may have no choice but to relax this.

[allendav]

@allendav as we discussed, the key warning in NoticesList occurs on master as well. Is this good to go?

@jeffstieler - I think so - captured key issue as a new issue here : #360

[jeffstieler]

But if we can't figure out a way to allow only certain characters in a field (like decimals) I guess we may have no choice but to relax this.

I think this is the key problem with our current implementation. I agree with @nabsul on this one, we should relax the onChange handler, and let the schema do the validation.

EDIT: Let's just hash this out as part of #188.