-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Display error message for symbols in legal name #15989
Display error message for symbols in legal name #15989
Conversation
@PauloGasparSv @sobitneupane One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Hey @PrashantMangukiya can we change the P.R. so it also accepts numbers for those fields? |
Also, not sure if we should mention which characters we allow or not in the error message. Since the Taking inspiration in the - hasInvalidCharacter: 'Name contains unsupported character.',
+ hasInvalidCharacter: 'Name can only include include letters and numbers', And - hasInvalidCharacter: 'El nombre contiene un carácter no admitido.',
+ hasInvalidCharacter: 'El nombre solo puede contener letras y números', Wow thks for being so fast with he regex update @PrashantMangukiya! |
I'll be out for ~2 hours top and will test on every platform when I get back : ) |
Reviewer Checklist
Screenshots/Videos |
Thanks for the quick changes @PrashantMangukiya . Reviewing it now. |
@@ -621,6 +621,7 @@ export default { | |||
error: { | |||
dateShouldBeBefore: ({dateString}) => `La fecha debe ser anterior a ${dateString}.`, | |||
dateShouldBeAfter: ({dateString}) => `La fecha debe ser posterior a ${dateString}.`, | |||
hasInvalidCharacter: 'El nombre solo puede contener letras y números.', |
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.
@PauloGasparSv Can you please verify if it is correct translation.
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.
LGTM!
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.
Screenshots/Videos
Web
Screen.Recording.2023-03-15.at.21.32.02.mov
Mobile Web - Chrome
Screen.Recording.2023-03-15.at.21.39.29.mov
Mobile Web - Safari
Screen.Recording.2023-03-15.at.21.43.44.mov
Desktop
Screen.Recording.2023-03-15.at.21.35.39.mov
iOS
Screen.Recording.2023-03-15.at.21.42.34.mov
Android
Screen.Recording.2023-03-15.at.21.41.09.mov
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.
LGTM
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/PauloGasparSv in version: 1.2.86-0 🚀
|
🚀 Deployed to production by https://github.com/Julesssss in version: 1.2.86-1 🚀
|
@sobitneupane @PauloGasparSv PR is ready for review
Details
When we use special chars and symbols for legal name at that time server side api will remove all symbols and not store legal name within db so legal name become clear if such chars used.
Also form does not show error in ui when user enter special chars or symbols within first name and last name fields for legal name. So within this pr we implemented functionality to show error to user when special chars used for legal name.
So now user can enter a-zA-Z0-9 and Space chars within legal name fields. e.g. "Paulo 1st" or "John the 2nd" etc.
Note: Within recorded video I did not show number while entering first and last legal name but user can also use 0-9 chars within it.
Fixed Issues
$ #15741
PROPOSAL: #15741 (comment)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Web
Web.mp4
Mobile Web - Chrome
Android-Web.mp4
Mobile Web - Safari
iOS-Web.mov
Desktop
Desktop.mp4
iOS
iOS.mov
Android
Android.mp4