fix: add sender name#58271
Conversation
|
@allroundexperts 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] |
|
@shawnborton this PR is opened |
|
Thanks, cc @Expensify/design for viz. Going to run a test build now. |
|
🚧 @shawnborton has triggered a test app build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
cc @Expensify/design @JmillsExpensify @trjExpensify I was thinking that we wanted to add the "You" for DMs, but I didn't realize this change would also add the sender name for a DM preview if the message comes from the other person. For example, this: |
|
That feels a little overkill, right? I think maybe we don't need that for the DM sender if it isn't yourself... but I can also see the argument for consistency and that we should show the sender name everywhere. |
|
Don't feel super strongly here but would agree that it feels a bit overkill |
|
If WhatsApp is the example, they don't include |
|
Yeah, that's kinda what I was thinking we'd follow too. Okay, @nkdengineer can you update please? No need to use the sender name in DMs unless it is |
This works for me, but I personally don't really hate it how it currently is. Y'all are probably right that it's a bit overkill though. |
|
@shawnborton i updated |
|
Thanks! Running a new test build now. |
|
🚧 @shawnborton has triggered a test app build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
|
Feels pretty good to me, @allroundexperts please continue with getting this into final review - thanks! |
Reviewer Checklist
Screenshots/VideosAndroid: NativeUnable to build android Android: mWeb ChromeScreen.Recording.2025-03-21.at.1.51.06.PM.moviOS: NativeScreen.Recording.2025-03-21.at.1.40.28.PM.moviOS: mWeb SafariScreen.Recording.2025-03-21.at.1.39.38.PM.movMacOS: Chrome / SafariScreen.Recording.2025-03-21.at.1.30.29.PM.movMacOS: DesktopScreen.Recording.2025-03-21.at.1.35.45.PM.mov |
|
@nkdengineer can you please merge main? |
|
✋ 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/mountiny in version: 9.1.17-0 🚀
|
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.1.17-1 🚀
|

Explanation of Change
Fixed Issues
$ #57038
PROPOSAL: #57038 (comment)
Tests
Youalso appears in LHNOffline tests
Same
QA Steps
Youalso appears in LHNPR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.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
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop