Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add error formatting #178

Closed
wants to merge 1 commit into from

Conversation

ICEDLEE337
Copy link

Add a simple pipe with default error messages that display validation data for core Angular validators with configurable overriding via a provider token.

return displayName ? `${displayName || ''} ${(msg || '').toLowerCase()}` : msg;
}

export const errorDefaultMessages: IFormatters = {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these can be overridden by passing an argument to the subFormErrorProvider helper in utils

<mat-error data-input-image-url-error *ngIf="formGroupErrors?.imageUrl?.required">
Image url is required
<mat-error>
{{formGroupErrors?.imageUrl | formatError: 'Image Path'}}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the argument is optional. when provided, it is displayed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only one error at a time is shown based on the order of the FormControl's validators array

<mat-error data-input-price-error *ngIf="formGroupErrors?.price?.required">
Price is required
<mat-error>
{{formGroupErrors?.price | formatError}}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

example with no argument

@@ -59,8 +59,8 @@ export class ListingFormComponent extends NgxRootFormComponent<OneListing, OneLi
listingType: new FormControl(null, Validators.required),
id: new FormControl(null, Validators.required),
title: new FormControl(null, Validators.required),
imageUrl: new FormControl(null, Validators.required),
price: new FormControl(null, Validators.required),
imageUrl: new FormControl(null, {validators: [Validators.required, Validators.minLength(4), Validators.maxLength(8)]}),
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these configurable Validators.* parameter values are displayed in the error messages by default

describe('GIVEN error object has a member matching a defined error message key', () => {
beforeEach(() => {
definedErrorMessageKey = 'required';
errorObject = { [definedErrorMessageKey]: uuid.v4() };
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these may need cleaned up or fixed because i didn't run them locally

@ICEDLEE337
Copy link
Author

The goal is to provide DRYness and flexibility consistent with the standard set by this library. Error messages are formatted with Angular validator data according to the configurable message formatting functions. Other languages or different messages can be provided with a one-line addition to providers array consistent with ngx-sub-form's current manner of defining providers through util helper functions.

Please try the demo page to see the pipe in action!

I will fix build errors if there's a chance that this would be integrated. Thanks for your time and thanks for the great library!

@ICEDLEE337
Copy link
Author

closing bc there's obviously no interest in incorporating this into the repo

@ICEDLEE337 ICEDLEE337 closed this Oct 15, 2020
@maxime1992
Copy link
Contributor

I'm very sorry @ICEDLEE337 for not getting back to you.

We've been thinking a lot about #171 and spending most of the time we had for ngx-sub-form on actually implementing that (#188).

It's basically a (nearly) full rewrite internally of ngx-sub-form. As it was complicated enough, we've left on the side your MR to focus onto that but hopefully once that change is in we can reopen it and have another look 😊.

Sorry for being silent for so long and letting your without an answer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants