Skip to content
This repository was archived by the owner on Aug 7, 2020. It is now read-only.

feat(field): add custom error message and ng-pattern support #109

Merged
merged 2 commits into from
Mar 5, 2018
Merged

feat(field): add custom error message and ng-pattern support #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
064d0ef
:sparkles: feat(field): add custom error message and ng-pattern support
JDownloader Feb 27, 2018
482a8fa
chore(field): add more test and review doc
JDownloader Mar 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions packages/oui-field/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@
name="email"
ng-model="$ctrl.user.email">
</oui-field>

<oui-field label="{{'Username'}}"
error-messages="{pattern: 'Username must be alphabetic with a length between 3 and 8.'}">
Copy link
Contributor

@tremblaymath tremblaymath Mar 2, 2018

Choose a reason for hiding this comment

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

You should add a helper text "Username must be alphabetic with a length between 3 and 8.", it will be helpful to trigger error. It could be interesting for other examples too.

Copy link
Contributor

@neolitec neolitec Mar 5, 2018

Choose a reason for hiding this comment

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

An helper text or an error, not both of them...
Maybe something like "This input is valid only if..." for helper text.

Copy link
Contributor Author

@JDownloader JDownloader Mar 5, 2018

Choose a reason for hiding this comment

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

I would avoid also doing both, I think the error is easy to trigger like the lastname field.

Copy link
Contributor

@tremblaymath tremblaymath Mar 5, 2018

Choose a reason for hiding this comment

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

easy to trigger yes!
But the user doesn't know the validations without clicking on "Show the example" and reading the code.
"I think the error is easy to trigger like the lastname field." There is already a help text for lastname "At least 3 chars" and it's a good thing that we should reproduce

Copy link
Contributor

@tremblaymath tremblaymath Mar 2, 2018

Choose a reason for hiding this comment

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

I would like to see parameters in this example

<input
class="oui-input"
type="text"
id="username"
name="username"
ng-model="$ctrl.user.username"
ng-pattern="/^[a-zA-Z]{3,8}$/">
</oui-field>
```

### Checkbox
Expand Down Expand Up @@ -147,9 +158,20 @@

## API

### oui-datagrid

| Attribute | Type | Binding | One-time binding | Values | Default | Description |
| ---- | ---- | ---- | ---- | ---- | ---- | ---- |
| `help-text` | string | @? | yes | | | the field label |
| `label` | function | @? | yes | | | a text to help fill the form field |
| Attribute | Type | Binding | One-time binding | Values | Default | Description |
| ---- | ---- | ---- | ---- | ---- | ---- | ---- |
| `error-messages` | object | <? | no | | | a dictionary to override default messages |
| `help-text` | string | @? | yes | | | the field label |
| `label` | function | @? | yes | | | a text to help fill the form field |

`error-messages` object can override error messages for validation.

| Attribute | Type | Default |
| ---- | ---- | ---- |
| `required` | string | Mandatory. |
| `email` | string | Invalid email. |
| `min` | string | Too low (1 min). |
| `max` | string | Too high (100 max). |
| `minlength` | string | Too short (6 characters min). |
| `maxlength` | string | Too high (12 characters max). |
| `pattern` | string | Invalid format. |
3 changes: 2 additions & 1 deletion packages/oui-field/src/field.component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import template from "./field.html";
export default {
bindings: {
label: "@?",
helpText: "@?"
helpText: "@?",
errorMessages: "<?"
},
controller,
require: {
Expand Down
13 changes: 9 additions & 4 deletions packages/oui-field/src/field.controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ const VALIDATION_PARAMETERS = {
min: ["min", "ng-min"],
max: ["max", "ng-max"],
minlength: ["minlength", "ng-minlength"],
maxlength: ["maxlength", "ng-maxlength"]
maxlength: ["maxlength", "ng-maxlength"],
pattern: ["pattern", "ng-pattern"]
};

export default class FieldController {
Expand Down Expand Up @@ -124,7 +125,7 @@ export default class FieldController {

showErrors (controlElement, name) {
this.$timeout(() => {
if (this.form[name].$invalid) {
if (this.form[name] && this.form[name].$invalid) {
angular.element(controlElement).addClass(FieldController.getErrorClass(controlElement));
this.$ouiFieldElement.addClass("oui-field_error");
this.isErrorVisible = true;
Expand All @@ -149,16 +150,20 @@ export default class FieldController {
getFirstError () {
const names = Object.keys(this.controls);
for (let i = 0; i < names.length; ++i) {
if (this.form[names[i]].$invalid) {
if (this.form[names[i]] && this.form[names[i]].$invalid) {
return this.form[names[i]].$error;
}
}

return null;
}

getMessageString (errorName) {
return (this.errorMessages && this.errorMessages[errorName]) || this.ouiFieldConfiguration.translations.errors[errorName];
}

getErrorMessage (errorName) {
const message = this.ouiFieldConfiguration.translations.errors[errorName];
const message = this.getMessageString(errorName);
const parameterValue = this.validationParameters[this.currentErrorField][errorName];
return message.replace(`{{${errorName}}}`, parameterValue);
}
Expand Down
2 changes: 2 additions & 0 deletions packages/oui-field/src/field.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
ng-bind="$ctrl.getErrorMessage('min')"></span>
<span ng-message="max"
ng-bind="$ctrl.getErrorMessage('max')"></span>
<span ng-message="pattern"
ng-bind="$ctrl.getErrorMessage('pattern')"></span>
</span>
</div>
<div class="oui-field__helper"
Expand Down
3 changes: 2 additions & 1 deletion packages/oui-field/src/field.provider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ export default class {
min: "Too low ({{min}} min).",
max: "Too high ({{max}} max).",
minlength: "Too short ({{minlength}} characters min).",
maxlength: "Too high ({{maxlength}} characters max)."
maxlength: "Too high ({{maxlength}} characters max).",
pattern: "Invalid format."
}
};
}
Expand Down
130 changes: 125 additions & 5 deletions packages/oui-field/src/index.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe("ouiField", () => {
expect(getLabel(element)).toBeNull();
});

it("should set the for attribute on label", () => {
it("should set the 'for' attribute on label", () => {
const id = "lastname";

const element = TestUtils.compileTemplate(`
Expand All @@ -90,6 +90,60 @@ describe("ouiField", () => {
expect(getLabel(element).getAttribute("for")).toEqual(id);
});

it("should trigger error when invalid format", () => {

const element = TestUtils.compileTemplate(`
<form name="form">
<oui-field label="{{'username'}}">
<input type="text"
class="oui-input"
type="text"
id="username"
name="username"
ng-model="$ctrl.username"
ng-pattern="/^[a-zA-Z]{3,8}$/">
</oui-field>
</form>
`);
const controller = getField(element).controller("ouiField");

$timeout.flush();

const $control = getControl(controller, "username");
$control.val("ch@t12");
$control.triggerHandler("input");
$control.triggerHandler("blur");

expect(controller.getFirstError().pattern).toBeTruthy();
});

it("should trigger error when length too short", () => {

const element = TestUtils.compileTemplate(`
<form name="form">
<oui-field label="{{'username'}}">
<input type="text"
class="oui-input"
type="text"
id="username"
name="username"
minlength="6"
ng-model="$ctrl.username">
</oui-field>
</form>
`);
const controller = getField(element).controller("ouiField");

$timeout.flush();

const $control = getControl(controller, "username");
$control.val("abc");
$control.triggerHandler("input");
$control.triggerHandler("blur");

expect(controller.getFirstError().minlength).toBeTruthy();
});

it("should set the name of the form field in the controller", () => {
const name = "lastname";

Expand Down Expand Up @@ -128,16 +182,16 @@ describe("ouiField", () => {
id="age"
name="age"
ng-model="$ctrl.user.age"
min="{{$ctrl.validation.min}}"
max="{{$ctrl.validation.max}}">
ng-min="{{$ctrl.validation.min}}"
ng-max="{{$ctrl.validation.max}}">
<input
class="oui-input"
type="text"
id="name"
name="name"
ng-model="$ctrl.user.name"
minlength="{{$ctrl.validation.minlength}}"
maxlength="{{$ctrl.validation.maxlength}}">
ng-minlength="{{$ctrl.validation.minlength}}"
ng-maxlength="{{$ctrl.validation.maxlength}}">
</oui-field>
`, {
validation
Expand Down Expand Up @@ -466,6 +520,72 @@ describe("ouiField", () => {
});
});

describe("with validation", () => {
Copy link
Contributor

@tremblaymath tremblaymath Mar 2, 2018

Choose a reason for hiding this comment

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

Good test for custom message!
We should test default error messages too. Almost identical to this one

Copy link
Contributor Author

@JDownloader JDownloader Mar 5, 2018

Choose a reason for hiding this comment

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

Yeah, but it the test should not enforce a string but verify it's actually using the one from the provider

Copy link
Contributor

@tremblaymath tremblaymath Mar 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Yes I agree, provider should be mocked and we should only "verify it's actually using the one from the provider"
There was already missing tests before you started

it("should retrieve custom error messages", () => {
const message = "Username must be a least 6 characters.";
Copy link
Contributor

@neolitec neolitec Mar 2, 2018

Choose a reason for hiding this comment

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

(optional but should not be) I don't remember if it has been done before but I think you should also test that interpolation works in custom messages.

Copy link
Contributor Author

@JDownloader JDownloader Mar 5, 2018

Choose a reason for hiding this comment

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

This could be done in a new test, only for testing the interpolation.


const element = TestUtils.compileTemplate(`
<form name="form">
<oui-field label="{{'username'}}"
error-messages="{minlength: '${message}'}">
<input type="text"
class="oui-input"
type="text"
id="username"
name="username"
ng-minlength="6"
ng-model="$ctrl.username">
</oui-field>
</form>
`);

const controller = getField(element).controller("ouiField");

$timeout.flush();

const $control = getControl(controller, "username");
$control.val("abc");
$control.triggerHandler("input");
$control.triggerHandler("blur");

$timeout.flush();

expect(controller.getFirstError().minlength).toBeTruthy();
expect(controller.getErrorMessage("minlength")).toBe(message);
});

it("should give a message containing parameters", () => {
const messageMinlength = 5;

const element = TestUtils.compileTemplate(`
<form name="form">
<oui-field label="{{'username'}}">
<input type="text"
class="oui-input"
type="text"
id="username"
name="username"
ng-minlength="${messageMinlength}"
ng-model="$ctrl.username">
</oui-field>
</form>
`);

const controller = getField(element).controller("ouiField");

$timeout.flush();

const $control = getControl(controller, "username");
$control.val("abc");
$control.triggerHandler("input");
$control.triggerHandler("blur");

$timeout.flush();

expect(controller.getFirstError().minlength).toBeTruthy();
expect(controller.getErrorMessage("minlength")).toContain(messageMinlength);
});
});
});

});