-
Notifications
You must be signed in to change notification settings - Fork 357
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
fix(dynamic-forms): removed regex for name #1155
Conversation
…eradata/covalent into fix/dynamic-form-element-regex
@@ -49,7 +49,7 @@ export interface ITdDynamicElementConfig { | |||
validators?: ITdDynamicElementValidator[]; | |||
} | |||
|
|||
export const DYNAMIC_ELEMENT_NAME_REGEX: RegExp = /^[a-zA-Z]+[a-zA-Z0-9-_]*$/; | |||
export const DYNAMIC_ELEMENT_NAME_REGEX: RegExp = /^[^0-9][^\@]*$/; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what are you trying to capture with this regex?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the original unit tests, it's testing for a name starting with a number or a name with an ampersand in it. I do realize that the \
isn't necessary to escape the @
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, so it actually is testing for what you want to be testing for, however. I just thought it was an odd pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
* fix(dynamic-forms): removed regex for name * fix(dynamic-forms): modified regex to allow japanese/chinese chars * fix(dynamic-forms): got rid of spaces
* fix(dynamic-forms): removed regex for name * fix(dynamic-forms): modified regex to allow japanese/chinese chars * fix(dynamic-forms): got rid of spaces
Description
Removed regex from name string
Test Steps
npm run serve
General Tests for Every PR
npm run serve:prod
still works.npm run tslint
passes.npm run stylelint
passes.npm test
passes and code coverage is not lower.npm run build:lib
still works.Screenshots or link to StackBlitz/Plunker
Stackblitz with regex implemented
https://stackblitz.com/edit/ajc-stackoverflow-48705028?file=app/app.component.ts