feat(company data): improved white space handling and update identifier error messages #210
Conversation
Move trimming to onBlur and save. Refs: eclipse-tractusx#190
|
Hi @typecastcloud the pr is still set to draft, is that intended? |
Hi @evegufy, last time @oyo merged before all questions were resolved. I can set this to ready for review now. |
Changes to patterns introduced in PR eclipse-tractusx#196 were not yet included in error messages.
|
@typecastcloud could you please update the pr title to something like: |
typecastcloud
changed the title
typecastcloud
changed the title
| @@ -206,7 +206,7 @@ export const CompanyDataCax = () => { | |||
| } | |||
|
|
|||
| const onSearchChange = (expr: string) => { | |||
| if (isBPN(expr)) { | |||
| if (isBPN(expr?.trim())) { | |||
There was a problem hiding this comment.
if we follow the discussed convention to always trim any input value I think it's better to do the trim inside the isBPN (and all other similar) functions so the code here would be better readable.
There was a problem hiding this comment.
Would be unexpected behavior for isXYZ functions imho. You check a value against REGEX. Trimming here is only valid because we take care of white spaces somewhere else.
There was a problem hiding this comment.
okay - there are different ways to solve that. Anyway the trim has to go somewhere. Thinking about different input fields the question becomes more generic and tricky anyway: for example let's consider spaces inside some values like IBAN or phone numbers and how the system should deal with those. From a user perspective i guess it is less annoying if the system automatically parses and reformats values instead of showing an error like when pasting IBANs, phone numbers, etc. which are common to be written with spaces.
| @@ -222,7 +222,7 @@ export const CompanyDataCax = () => { | |||
| const validateLegalEntity = (value: string) => { | |||
| setLegalEntity(value) | |||
|
|
|||
| if (!Patterns.legalEntityPattern.test(value)) { | |||
| if (!Patterns.legalEntityPattern.test(value?.trim())) { | |||
There was a problem hiding this comment.
personally I'd prefer to replace all the Patterns.xyz.test(value) with isXYZ - here isLegalEntity just like isBPN above. Makes the code more readable.
…er error messages (eclipse-tractusx#210) * feat(company data): improved white space handling for company data Move trimming to onBlur and save. Refs: eclipse-tractusx#190 * Update error messages for identifiers. Changes to patterns introduced in PR eclipse-tractusx#196 were not yet included in error messages.
…er error messages (eclipse-tractusx#210) * feat(company data): improved white space handling for company data Move trimming to onBlur and save. Refs: eclipse-tractusx#190 * Update error messages for identifiers. Changes to patterns introduced in PR eclipse-tractusx#196 were not yet included in error messages.
Description
See: https://github.com/Cofinity-X/entry-portal-frontend-registration/blob/main/src/types/Patterns.ts#L23-L45
Why
This improves usability and stability. Handling by regex error is hard to understand when customer can't see the whitespace.
Issue
Refs: #190
Checklist