Fix text based mobile search on InviteMembers page#23366
Conversation
Signed-off-by: Anjana Mendiratta <expertsquarrel@gmail.com>
|
@abdulrahuman5196 Please 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] |
|
@GItGudRatio Can you fix the lint failure? |
Signed-off-by: Anjana Mendiratta <expertsquarrel@gmail.com>
|
@abdulrahuman5196 updated! |
|
@GItGudRatio Unit tests is failing. Kindly fix the same. Kindly make sure checks are passing. |
|
@abdulrahuman5196 updated! The test was failing because we were checking directly against the search value for digits and Also, added a unit test to account for this issue. |
Signed-off-by: Anjana Mendiratta <expertsquarrel@gmail.com>
|
@GItGudRatio Lint is failing agian. @GItGudRatio Kindly make sure the auto checks are passing before a review. |
Signed-off-by: Anjana Mendiratta <expertsquarrel@gmail.com>
|
@abdulrahuman5196 Sorry for wasting your time having to point out these trivial things, I'll take more care to do so in the future! Please review this now :) |
Reviewer Checklist
Screenshots/VideosWebScreen.Recording.2023-07-23.at.9.26.07.PM.movMobile Web - Chromeaz_recorder_20230723_213608.mp4Mobile Web - SafariUntitled.movDesktopScreen.Recording.2023-07-23.at.9.27.55.PM.moviOSScreen.Recording.2023-07-23.at.9.30.47.PM.movAndroidaz_recorder_20230723_213651.mp4 |
No worries. |
abdulrahuman5196
left a comment
There was a problem hiding this comment.
Changes looks good and works well. Reviewers checklist is also complete.
All yours. @amyevans
🎀 👀 🎀
C+ Reviewed
Signed-off-by: Anjana Mendiratta <expertsquarrel@gmail.com>
Signed-off-by: Anjana Mendiratta <expertsquarrel@gmail.com>
|
✋ 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/amyevans in version: 1.3.45-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 1.3.45-7 🚀
|
Details
We fix the issue where we can see that when we try to search for a user using just text, a phone number shows up, which can be confusing.
Fixed Issues
$ #22521
PROPOSAL: #22521 (comment)
Tests
998243aaaa. Make sure that no user is shown.Offline tests
Same as above
QA Steps
Same as above.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Web
web.mp4
Mobile Web - Chrome
android-mWeb.mp4
Mobile Web - Safari
ios-mWeb.mp4
Desktop
desktop.mp4
iOS
ios.mp4
Android
android.mp4