-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix/61550b - Viewport does not return to highlighted message after returning from thread #70534
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
Conversation
|
@dominictb 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] |
|
Screen.Recording.2025-09-18.at.00.17.16-compressed.mov |
@dominictb Thank you. This bug has been resolved. I've found that implementing the highlight item feature is significantly more complex than I initially anticipated, so I've been conducting parallel testing throughout. Hopefully, we’ll be able to arrive at the most stable version soon. |
|
Screen.Recording.2025-09-23.at.01.51.12.mov |
|
Screen.Recording.2025-09-23.at.01.55.00-compressed.movThis does not happen for inverted list so I expect that for non-inverted either. |
I actually noticed this issue before fixing the previous bug, but I assumed it would be reverted soon. And indeed, it was reverted in this pull request: #71010 |
I don't think this is a flaky issue, but rather that the scroll behavior is intended to address the problem where the list doesn't automatically scroll to fill the empty space at the bottom on iOS. I believe we've discussed this issue before — or did I miss something?
In practice, we don't use the list component within the |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
Thank you for pointing this out — the issue has now been resolved. |
|
Screen.Recording.2025-09-29.at.22.18.51.movAlso only the last 30 items are rendered, the rest would not show until I started scrolling the list. |
Yeah but I think it's just too noticeable and cause an unpleasant UX. Could you fix it? Maybe we can defer rendering the list until the list header component height is known? |
This is still reproducible for me #70534 (comment)
|
|
@dominictb Thanks for your patience. This PR is ready for review again. |
|
#70367 still reproducible on web: Screen.Recording.2025-10-26.at.23.16.11.mov |
|
Screen.Recording.2025-10-26.at.23.19.23-compressed.mov |
What is the section above your transaction list? I tested on the latest main branch and didn’t see those fields. Also, the scrolling happens because the added item is outside the visible area, so it gets scrolled into view. You can try testing on the latest main. So this is no longer caused by our PR. |
Resolved: Videosanroid.chrome.test.mp4ios.safari.test.mp4mac.safari.test.mp4mac.desktop.test.mp4 |
|
@dominictb When you have a moment, could you please take a look? Thanks! |
|
When list has many messages (38 in this case), returning from thread does not render all messages until you scroll to the top: Screen.Recording.2025-11-13.at.13.12.40.movNotice that at 0:10, the list stops at |
This bug has been fixed. Tests on all platforms: PASS |
joekaufmanexpensify
left a comment
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.
Good from a product perspective 👍
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-11-28.at.16.13.57.movAndroid: mWeb ChromeScreen.Recording.2025-11-28.at.16.16.55.moviOS: HybridAppScreen.Recording.2025-11-28.at.16.04.31.moviOS: mWeb SafariScreen.Recording.2025-11-28.at.16.07.13-compressed.movMacOS: Chrome / SafariScreen.Recording.2025-11-28.at.16.08.16.mov |
dominictb
left a comment
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.
🎀
|
✋ 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/carlosmiceli in version: 9.2.66-0 🚀
|
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.2.66-3 🚀
|


Explanation of Change
Fixed Issues
$ #61550
$ #70367
PROPOSAL: #61550 (comment)
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
Test 1:
1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Submit at least two expenses.
4. Go to expense report.
5. Send many messages until the chat is scrollable
6. Right click on the newest message > Reply in thread.
7. Click on the link under header subtitle.
8. The viewport will return to the highlighted message (the same way it behaves when returning from thread).
Test 2:
1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Create two non-reimbursable expenses.
4. Open the expense report.
5. Click on the header subtitle to return to workspace chat.
6. Click Submit.
7. Click Approve.
8. Verify that app will not crash.
Test 3:
1. Launch Expensify app on iOS.
2. Go to workspace chat.
3. Submit two expenses to the workspace chat.
4. After submitting two expenses, quickly open the expense report and open any transaction row.
5. Tap on the header subtitle.
6. Verify that expense report will not turn blank.
Test 4:
1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Submit two expenses to the workspace chat.
4. Open one of the transaction threads and send a message.
5. Open the transaction thread message on LHN/
6. Click on the header subtitle.
7. Verify that expense report will not turn blank.
Test 5:
1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Create two expenses in the workspace chat.
4. Open the expense report.
5. Send 20 messages (so that the report is scrollable).
6. Mark the 20th message as unread.
7. Scroll to the top.
8. Click + > Create expense > Manual.
9. Create another expense.
10. Verify that after creating expense, the report view should not scroll down.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)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.native.mp4
Android: mWeb Chrome
android.mweb.chrome.mp4
iOS: Native
ios.native.mp4
iOS: mWeb Safari
ios.safari.mp4
MacOS: Chrome / Safari
mac.safari.mp4
MacOS: Desktop
mac.desktop.mp4
Test 2:
2025-09-15.10-41-48.mp4
Test 3
test3.mp4
Test 4
2025-09-15.10-43-39.mp4
Test 5
2025-09-15.10-44-31.mp4