feat(showcase): add the ngx-form-errors to the showcase #1721

christophercr · 2020-03-16T09:16:49Z

ISSUES CLOSED: #1195

Supersedes: #1361

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/NationalBankBelgium/stark/blob/master/CONTRIBUTING.md#-commit-message-guidelines
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[X] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: Incomplete PR #1361

What is the new behavior?

Took over the same changes from #1361 and made the necessary fixes based on the pending remarks:

update to latest version of ngx-form-errors
translations
replace grid layout by flex layout to make it compatible with IE
update example files to be in sync with the actual implementation

Does this PR introduce a breaking change?

[ ] Yes
[X] No

coveralls · 2020-03-16T09:35:00Z

Coverage remained the same at 88.154% when pulling 4a98d3c on christophercr:feature/ngx-forms into bee0d72 on NationalBankBelgium:master.

showcase/src/assets/translations/nl.json

showcase/src/assets/translations/fr.json

SuperITMan

Small changes for the translations.
Also a small issue in the demo page.

It's almost OK 😊

showcase/src/assets/translations/en.json

showcase/src/assets/translations/fr.json

showcase/src/assets/translations/en.json

showcase/src/assets/translations/fr.json

SuperITMan · 2020-03-24T14:41:59Z

showcase/src/app/welcome/pages/reactive-form-errors/reactive-form-errors-page.component.html

+										type="password"
+										[placeholder]="'SHOWCASE.NGX_FORM_ERRORS.FIELDS.CONFIRM_PASSWORD' | translate"
+										formControlName="confirmPassword"
+										[errorStateMatcher]="parentErrorStateMatcher"


I did some tests locally and I have the feeling this line should not be there. Indeed, if I remove it, I don't have the following issue:

(password fields contain the same values and respect the rules)

Could you please explain why this "parentErrorStateMatcher" is necessary ? 😊

Another point, if the passwords are not equal, the "Submit" button is disabled but no error is displayed 😕

Yeah, I was a bit lazy in the original implementation and I didn't check properly that it was working as expected.

The reason of the StateMatcher is to programmatically tell the mat-input when to toggle to the "error" state because we use a form group validator instead of a control validator, so we must tell the field when there is an error in the group. However our state matcher implementation is indeed buggy.

So I reworked this and the StateMatcher is not needed anymore 👍 I now use simple control validators and I even use the literal SHOWCASE.NGX_FORM_ERRORS.FORM.VALIDATION.CONFIRM_PASSWORD.ARE_EQUAL which was not used at all before 😉

Now it works like a charm 💪

christophercr · 2020-03-25T12:20:43Z

Updated the PR with the fixes for the translations and refactored the validation of the "confirmPassword" field.

Could you guys have a look?

christophercr · 2020-08-24T12:43:18Z

@SuperITMan I've rebased this PR and fixed all conflicts. It's ready to be merged 😉

nicanac

LGTM, thanks

carlo-nomes

A couple of thoughts about the translations. 😜

carlo-nomes · 2020-08-25T14:05:42Z

showcase/src/assets/translations/nl.json

+          "REQUIRED": "{{fieldName}} is verplicht",
+          "PASSWORD_REQUIRED": "{{fieldName}} moet worden gegeven",
+          "USER_NAME": {
+            "UNIQUE": "Dit gebruikersnaam is al in gebruik"


*Deze gebruikersnaam is al in gebruik

Actually I think you are not using this translation.

carlo-nomes · 2020-08-25T14:32:10Z

showcase/src/assets/translations/nl.json

+          "PASSWORD": {
+            "MAX_LENGTH": "Wachtwoord mag niet meer dan {{requiredLength}} tekens lang zijn",
+            "MIN_LENGTH": "Wachtwoord moet minimaal {{requiredLength}} tekens lang zijn",
+            "PATTERN": "Het wachtwoord moet minimaal één hoofdletter, één kleine letter en één cijfer bevatten"


I would keep the sentence structure similar here. Eg: "Wachtwoord moet minimaal één hoofdletter, één kleine letter en één cijfer bevatten"

carlo-nomes · 2020-08-25T14:33:31Z

showcase/src/assets/translations/en.json

+          "PASSWORD": {
+            "MAX_LENGTH": "Password cannot be more than {{requiredLength}} characters long",
+            "MIN_LENGTH": "Password must be at least {{requiredLength}} characters long",
+            "PATTERN": "The password must contain at least one uppercase, one lowercase, and one number"


I would keep the sentence structure similar here. Eg: "Password must contain at least one uppercase, one lowercase, and one number"

christophercr · 2021-04-01T14:21:12Z

Hey guys, fyi I'll continue with this PR in the next days so it can finally be merged 😉 (god! it's been open for a year now!!)

ISSUES CLOSED: NationalBankBelgium#1195

sonarcloud · 2021-05-12T07:56:40Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

christophercr · 2021-05-12T08:37:30Z

Finally the day has come! PR rebased and updated with the latest remarks! 👍

Can you guys have a look?

SuperITMan · 2021-05-16T14:27:33Z

LGTM! Thanks for your great job on this! 👍
Let's merge it, finally 🥳 🎉

christophercr force-pushed the feature/ngx-forms branch from 33e76be to 4dee132 Compare March 16, 2020 09:17

christophercr changed the title ~~feat(showcase): add the ngx form error to the showcase~~ feat(showcase): add the ngx-form-errorr to the showcase Mar 16, 2020

christophercr changed the title ~~feat(showcase): add the ngx-form-errorr to the showcase~~ feat(showcase): add the ngx-form-errors to the showcase Mar 16, 2020

christophercr added comp: stark-showcase type: feature labels Mar 16, 2020

christophercr added this to the 10.0.1 milestone Mar 16, 2020

christophercr assigned SuperITMan and nicanac Mar 16, 2020

christophercr unassigned SuperITMan and nicanac Mar 16, 2020

christophercr requested review from SuperITMan and nicanac March 16, 2020 09:37

christophercr commented Mar 16, 2020

View reviewed changes

showcase/src/assets/translations/nl.json Outdated Show resolved Hide resolved

christophercr commented Mar 16, 2020

View reviewed changes

showcase/src/assets/translations/fr.json Outdated Show resolved Hide resolved

christophercr modified the milestones: 10.0.1, 10.1.0 Mar 19, 2020

christophercr force-pushed the feature/ngx-forms branch from 4dee132 to f34f5a8 Compare March 20, 2020 10:19

SuperITMan requested changes Mar 24, 2020

View reviewed changes

SuperITMan mentioned this pull request Mar 24, 2020

feat(stark-demo): add the ngx form error to the showcase #1361

Closed

3 tasks

christophercr force-pushed the feature/ngx-forms branch from f34f5a8 to 33c572a Compare March 25, 2020 12:14

christophercr requested a review from SuperITMan March 25, 2020 12:21

SuperITMan modified the milestones: 10.1.0, 10.2.0 Jul 24, 2020

christophercr force-pushed the feature/ngx-forms branch 2 times, most recently from 3442e87 to 7e8e5b8 Compare August 23, 2020 12:17

nicanac approved these changes Aug 25, 2020

View reviewed changes

carlo-nomes suggested changes Aug 25, 2020

View reviewed changes

SuperITMan modified the milestones: 10.2.0, 11.0.0 Mar 10, 2021

christophercr force-pushed the feature/ngx-forms branch 3 times, most recently from a839e85 to b119bc4 Compare May 12, 2021 07:42

feat(showcase): add the ngx-form-errors demo to the showcase

4a98d3c

ISSUES CLOSED: NationalBankBelgium#1195

christophercr force-pushed the feature/ngx-forms branch from b119bc4 to 4a98d3c Compare May 12, 2021 07:55

SuperITMan approved these changes May 16, 2021

View reviewed changes

SuperITMan merged commit 0f05c34 into NationalBankBelgium:master May 16, 2021

christophercr deleted the feature/ngx-forms branch September 8, 2021 07:03

SuperITMan modified the milestones: 12.0.0, 12.0.0-beta.0 Jan 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(showcase): add the ngx-form-errors to the showcase #1721

feat(showcase): add the ngx-form-errors to the showcase #1721

christophercr commented Mar 16, 2020

coveralls commented Mar 16, 2020 •

edited

Loading

SuperITMan left a comment

SuperITMan Mar 24, 2020

christophercr Mar 25, 2020

christophercr commented Mar 25, 2020 •

edited

Loading

christophercr commented Aug 24, 2020

nicanac left a comment

carlo-nomes left a comment

carlo-nomes Aug 25, 2020

carlo-nomes Aug 25, 2020

carlo-nomes Aug 25, 2020

carlo-nomes Aug 25, 2020

christophercr commented Apr 1, 2021

sonarcloud bot commented May 12, 2021

christophercr commented May 12, 2021

SuperITMan commented May 16, 2021

feat(showcase): add the ngx-form-errors to the showcase #1721

feat(showcase): add the ngx-form-errors to the showcase #1721

Conversation

christophercr commented Mar 16, 2020

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

coveralls commented Mar 16, 2020 • edited Loading

SuperITMan left a comment

Choose a reason for hiding this comment

SuperITMan Mar 24, 2020

Choose a reason for hiding this comment

christophercr Mar 25, 2020

Choose a reason for hiding this comment

christophercr commented Mar 25, 2020 • edited Loading

christophercr commented Aug 24, 2020

nicanac left a comment

Choose a reason for hiding this comment

carlo-nomes left a comment

Choose a reason for hiding this comment

carlo-nomes Aug 25, 2020

Choose a reason for hiding this comment

carlo-nomes Aug 25, 2020

Choose a reason for hiding this comment

carlo-nomes Aug 25, 2020

Choose a reason for hiding this comment

carlo-nomes Aug 25, 2020

Choose a reason for hiding this comment

christophercr commented Apr 1, 2021

sonarcloud bot commented May 12, 2021

christophercr commented May 12, 2021

SuperITMan commented May 16, 2021

coveralls commented Mar 16, 2020 •

edited

Loading

christophercr commented Mar 25, 2020 •

edited

Loading