-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Show short mentions and current user mention in MarkdownTextInput with styling #54037
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
Show short mentions and current user mention in MarkdownTextInput with styling #54037
Conversation
b85d761
to
a140a7f
Compare
f179eee
to
4ddb91e
Compare
4ddb91e
to
8a8e5cd
Compare
d88cb60
to
43cf3f3
Compare
This PR is currently awaiting merge and release of: Expensify/expensify-common#824 |
43cf3f3
to
eca0668
Compare
eca0668
to
508c4a3
Compare
508c4a3
to
d09c805
Compare
Update with description how to test this here: #38025 (comment) |
Screen.Recording.2025-02-18.at.1.46.45.PM.mov@Kicu Can you please check this and verify if we need to change something in clipboard? |
@shubham1206agra So in this task I focused only on the part of adding styling to short-mentions in the Live markdown input. So after they're back from backend, we get full emails and that is what we see on your video. I believe everything related to copy-to-clipboard from this point would be working same as before. |
Sending as full mention to the API seems correct to me. That's what the back-end expects. The short mention is just a rendering thing on the client, not something the back-end is aware of. |
There seems to be some problems with publishing to npm: https://github.com/Expensify/expensify-common/actions/runs/13502875291 I've pinged people on slack and will be waiting till this gets solved. Once the version is published I will make this PR reviewable (probably next week). |
Ah gotcha. Were you able to get the help you needed on the npm publishing issue? Would you mind linking the Slack thread here? |
@puneetlath yeah the newest version of |
d09c805
to
6801386
Compare
|
Great! |
@shubham1206agra I pused a commit fixing the green long mention highlighting. Screen.Recording.2025-03-14.at.09.43.32.movAs for the rest minor bugs:
same as on prod
About reanimated warning: I'm not sure what causes it. I asked @tomekzaw who works with reanimated to take a quick look at the code and he didn't see anything wrong with I think these are small enough that we could merge this PR. I have a lot of work with ReportsView in here: #57508 - so if you find any other bugs then I can only work on them once I'm done with the other task. |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2025-03-13.at.7.20.45.PM.movAndroid: mWeb ChromeScreen.Recording.2025-03-13.at.6.23.51.PM.moviOS: NativeScreen.Recording.2025-03-13.at.6.51.53.PM.moviOS: mWeb SafariScreen.Recording.2025-03-13.at.6.14.48.PM.movMacOS: Chrome / SafariScreen.Recording.2025-03-13.at.6.07.07.PM.movMacOS: DesktopScreen.Recording.2025-03-13.at.6.39.54.PM.mov |
Co-authored-by: Shubham Agrawal <[email protected]>
|
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.
@puneetlath Will leave final approve / merge to you
|
|
|
Sorry, wrong PR |
|
✋ 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/puneetlath in version: 9.1.16-0 🚀
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.1.16-4 🚀
|
Explanation of Change
This PR introduces short mentions to the App. These are mentions of style
@
+ first part of user email (or login), ex:@john.doe
,@user123
.We are only highlighting a login (short-mention) for users that we have already interacted with in the app.
The reason is that there is no place other than local Onyx data to get knowledge whether a string like:
@some.body
is an actual login of Expensify user or just a random string. So a user types in a login of someone he has never interacted with it will not be highlighted.This PR builds upon (and uses):
live-markdown
change: Add support for short mentions from ExpensiMark react-native-live-markdown#592expensify-common
change: Add handling short mentions tags to ExpensiMark expensify-common#824parser
prop) react-native-live-markdown#439Details:
mention-short
tag from ExpensiMark and inside worklet we compare the mention with the actual list of user loginsmention-user
style in LiveMarkdown)sharedValue
is used to pass the list to a worklet.IMPORTANT:this PR should be merged together with this Hybrid bump: https://github.com/Expensify/Mobile-Expensify/pull/13451no Hybrid changes required
Fixed Issues
$ #38025
PROPOSAL:
Tests
Offline tests
QA Steps
Same as Tests
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodSTYLE.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)Design
label and/or tagged@Expensify/design
so the design team can review the changes.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
rec-mention-andr.mp4
Android: mWeb Chrome
iOS: Native
rec-mention-ios.mp4
iOS: mWeb Safari
rec-mention-ios-web.mp4
MacOS: Chrome / Safari
rec-mention-web.mp4
MacOS: Desktop