-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Update expensify-common commit hash in package.json #33142
Update expensify-common commit hash in package.json #33142
Conversation
This follows the changes in PR Expensify#615 that fixes issue Expensify#28946.
@Santhosh-Sellavel 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] |
My commit message was meant to reference the PR from I apologize for that error. |
Reviewer Checklist
Screenshots/VideosMacOS: Chrome / Safari & MacOS: DesktopScreen.Recording.2023-12-18.at.10.56.00.PM.mov |
@Victor-Nyagudi Can you merge with master some tests are failing |
@Santhosh-Sellavel I gave an update on why the tests are failing. TLDR: It's a known issue that is currently being worked on. The PR in the comment was merged with tests failing. |
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 tests well!
Note: Unable to test on Android due to some issue, that shouldn't be a blocker here, so approving the PR!
Merging with Snyk tests failing as it's a bug and being worked on with the Snyk team |
✋ 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.4.14-0 🚀
|
🚀 Deployed to staging by https://github.com/amyevans in version: 1.4.14-0 🚀
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.4.14-6 🚀
|
Details
Removed the
+
quantifier from theuserMentions
regex because this resulted in all the '@' signs being matched when a mention had multiple preceding '@' signs.For example, the match looked like this
@@user@expensify.com
, and clicking it showed a user with@user@expensify.com
as their email in the RHP. Trying to message this user then resulted in an error because their email address was invalid.With this change, the match now looks like this: @
@user@expensify.com
, which, upon clicking/tapping, shows the valid emailuser@expensify.com
in the RHP, so the user can now be messaged without errors.Fixed Issues
$ #28946
PROPOSAL: #28946 (comment)
Tests
@user@expensify.com
.Offline tests
Same as above in "Tests" section.
QA Steps
Repeat steps 2-6 in the "Tests" section above in a reply thread, workspace chat, group chat, and task thread.
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.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)StyleUtils.getBackgroundAndBorderStyle(theme.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
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop