fix: Content before dot separator is empty#60192
Conversation
|
@allgandalf 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] |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2025-04-15.at.10.24.26.AM.movAndroid: mWeb ChromeScreen.Recording.2025-04-15.at.10.23.34.AM.moviOS: NativeScreen.Recording.2025-04-15.at.10.20.55.AM.moviOS: mWeb SafariScreen.Recording.2025-04-15.at.10.20.22.AM.movMacOS: Chrome / SafariScreen.Recording.2025-04-15.at.10.17.48.AM.movMacOS: DesktopScreen.Recording.2025-04-15.at.10.19.46.AM.mov |
|
@shawnborton i'm lost here now, weren't these solutions preApproved (on other PR's as well)? |
|
Looks like design wasn't tagged in that PR, so sorry for catching this one late! |
|
discussing the optimistic delete report preview look here #60104 (comment) |
|
From @shawnborton: Yeah, my thinking is that we keep the general shape and styles, we just use strikethrough for any text that's visible in the report preview and expense previews. Basically this as a rough idea: |
|
This is for the report preview but for the expense preview the idea would be similar @daledah would you be able to work on that for $250 bounty in the issue? |
|
bump @daledah |
Yes, i can take a look |
|
Ping us in slack if you need anything, can you please prioritize this over new issues? thanks! |
|
@mountiny I'll try to give an update today |
|
@daledah any luck? |
|
@mountiny @allgandalf @shawnborton i updated, is this expected? result.mp4 |
|
I think in general yeas I am just not sure about the colour of the preview when its in the pending state cc @Expensify/design |
|
@allgandalf Updated 👍 |
|
On my to-do list for tomorrow |
|
@daledah I am still able to click on the expense and open it: Screen.Recording.2025-05-09.at.11.25.42.AM.movAlso jest is failing |
|
@daledah did you address the comments? Is it ready for another review? |
|
I'm checking now |
|
@allgandalf I updated. I might've missed a style when merging main 😅 |
allgandalf
left a comment
There was a problem hiding this comment.
@shawnborton can you give approval from design perspective please , this LGTM!
|
Can we see a video of it? I believe a couple of the recent bugs for us to confirm were:
|
|
🚧 @mountiny has triggered a test app build. You can view the workflow run here. |
|
@daledah @allgandalf Could you provide that please? |
|
this is looking good to me, there is still no offline pattern on the table view, but I think that should be out of scope so we can wrap this one up |
mountiny
left a comment
There was a problem hiding this comment.
Changes look good to me, just waiting for the final design confirmation
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
|
Mhm yeah, we should for'sure add the offline pattern to the table view. Who's going to take that follow-up? |
|
I think we should just create a new issue for SWM for that #60087 this pr already moved quite a bit from the original issue |
|
All good on my end 👍 |
|
✋ 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.45-0 🚀
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.1.45-0 🚀
|
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.1.45-21 🚀
|



Explanation of Change
Fixed Issues
$ #60087
PROPOSAL: #60087 (comment)
Tests
Offline tests
Same as tests
QA Steps
Same as tests
PR 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
Screen.Recording.2025-05-08.at.23.05.11.mov
Android: mWeb Chrome
Screen.Recording.2025-05-08.at.23.05.55.mov
iOS: Native
Screen.Recording.2025-05-08.at.23.06.51.mov
iOS: mWeb Safari
Screen.Recording.2025-05-08.at.23.07.38.mov
MacOS: Chrome / Safari
Screen.Recording.2025-05-08.at.23.08.39.mp4
MacOS: Desktop
Screen.Recording.2025-05-08.at.23.09.21.mov